FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Migrate to Pytest by vmuriart · Pull Request #368 · pythonnet/pythonnet · GitHub

/ pythonnet Public

Migrate to Pytest - #368

Merged
vmuriart merged 4 commits into
pythonnet:masterfrom
vmuriart:pytest
Feb 14, 2017
ConversationCommits (4)ChecksFiles changed
Merged

Migrate to Pytest#368
vmuriart merged 4 commits into
pythonnet:masterfrom
vmuriart:pytest

Conversation

vmuriart commented Feb 8, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

What does this implement/fix? Explain your changes.

  • Unittests module requires alot of boilerplate setup to get running. Pytest the process of unittest writing within python making it easier for new contributors to add tests.
  • Any test can be run by itself again, instead of having to run the entire test suite.

Does this close any currently open issues?

N/A

Any other comments?

This was the original meant to be part of #329 but decided to separate it out.

Checklist

Check all those that are applicable and complete.

  • Make sure to include one or more tests for your change
  • If an enhancement PR, please create docs and at best an example
  • Add yourself to AUTHORS
  • Updated the CHANGELOG

vmuriart self-assigned this Feb 8, 2017

vmuriart commented Feb 8, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Also another nice feature is that it has a nicer output

$ python -m pytest
============================= test session starts ==============================
platform linux -- Python 3.3.5, pytest-3.0.6, py-1.4.32, pluggy-0.4.0
rootdir: /home/travis/build/pythonnet/pythonnet, inifile: setup.cfg
collected 360 items 
src/tests/test_array.py ........................................
src/tests/test_callback.py ..
src/tests/test_class.py ..................
src/tests/test_compat.py ................
src/tests/test_constructors.py ....
src/tests/test_conversion.py ....................
src/tests/test_delegate.py .................
src/tests/test_docstring.py ...
src/tests/test_engine.py .ss
src/tests/test_enum.py ...............
src/tests/test_event.py ............................
src/tests/test_exceptions.py .........................
src/tests/test_field.py ..............................
src/tests/test_generic.py .................
src/tests/test_import.py .
src/tests/test_indexer.py .............................
src/tests/test_interface.py ....
src/tests/test_method.py ........................................
src/tests/test_module.py ...........................
src/tests/test_property.py ..........
src/tests/test_recursive_types.py .
src/tests/test_subclass.py sssss.
src/tests/test_sysargv.py .
src/tests/test_thread.py ...

vs

$ python src/tests/runtests.py
Load clr import hook
.................................................................................................................................s.............................................................................................................................................................................................................................s.ssssss.
----------------------------------------------------------------------
Ran 360 tests in 2.367s
OK (skipped=8)

codecov Bot commented Feb 8, 2017 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #368 into master will increase coverage by 0.09%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #368      +/-   ##
==========================================
+ Coverage   63.08%   63.18%   +0.09%     
==========================================
  Files          61       61              
  Lines        5239     5239              
  Branches      876      876              
==========================================
+ Hits         3305     3310       +5     
+ Misses       1715     1709       -6     
- Partials      219      220       +1
Impacted Files Coverage Δ
src/runtime/pythonexception.cs 64.28% <ø> (-1.79%) ❌
src/runtime/assemblymanager.cs 89.78% <ø> (+1.07%) ✅
src/runtime/moduleobject.cs 84.39% <ø> (+2.31%) ✅

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a5d325f...76aa731. Read the comment docs.

filmor approved these changes Feb 8, 2017
View reviewed changes

filmor left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This is a very good idea. We just need to fix the conda recipe, too.

Comment thread MANIFEST.in Outdated
@@ -0,0 +1,20 @@
graft docs

filmor Feb 8, 2017

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

What is this file for?

vmuriart Feb 8, 2017

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I forgot to remove that commit/file. Its supposed to replace the _get_source_files in setup.py.
I didn't get around to testing it though. Removing it fixes the conda recipe.

Let me fix that.

den-run-ai Feb 8, 2017

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Same question

vmuriart commented Feb 8, 2017

Copy link
Copy Markdown
Contributor Author

@filmor @denfromufa conda recipe fixed.
I wrote that MANIFEST.in a while back, it was referring to a docs folder we had i think.

vmuriart commented Feb 8, 2017

Copy link
Copy Markdown
Contributor Author

All builds finished, none of them had the conda.recipe build error from the manifest. Looks like we are good to merge then?

den-run-ai commented Feb 9, 2017 •
edited
Loading

Copy link
Copy Markdown
Contributor

I like the switch to pytest, can you give 1-2 days to review this and test locally? @vmuriart

vmuriart commented Feb 9, 2017

Copy link
Copy Markdown
Contributor Author

Sure. Test wise, nothing changed other than indent level.
Have you tried using tox for testing? It will let you run against all interpreters you have installed.

den-run-ai commented Feb 9, 2017 via email

Copy link
Copy Markdown
Contributor

vmuriart commented Feb 9, 2017

Copy link
Copy Markdown
Contributor Author

You can configure it for it; though idk if pythonnet would have any issues w it.

vmuriart removed the decision label Feb 13, 2017

vmuriart commented Feb 13, 2017

Copy link
Copy Markdown
Contributor Author

@denfromufa did you get a chance to finish testing?

den-run-ai commented Feb 13, 2017 via email

Copy link
Copy Markdown
Contributor

Add pytest to CI
ba7794b 
pytest on travis initially required `sudo=required` otherwise failed.
Without the sudo flag pytest wasn't being upgraded. Needed to force
upgrade.

pytest-dev/pytest#2240
Convert unittest to pytest
1ea29c7 
Needs local imports to work.
Conversion done with unittests2pytests and a couple regex
Add setup.cfg
104ef71 
Control settings for some add-ons like pytest
Simpler tox
76aa731
vmuriart merged commit 76aa731 into pythonnet:master Feb 14, 2017
vmuriart added a commit that referenced this pull request Feb 14, 2017
Merge pull request #368 from vmuriart/pytest
13e2a87
vmuriart deleted the pytest branch February 14, 2017 11:20
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

filmor filmor approved these changes
tonyroberts Awaiting requested review from tonyroberts
den-run-ai Awaiting requested review from den-run-ai

Assignees

vmuriart

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL