Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adopting pytest #199

Merged
merged 11 commits into from Apr 27, 2015
Merged

Adopting pytest #199

merged 11 commits into from Apr 27, 2015

Conversation

bsravanin
Copy link

  1. Switching from unittest assertions to basic assert statements.
  2. Removing boilerplate code related to test suites.
  3. Adding a TestCommand to setup.py, to run tests using pytest by default.
  4. Fixing the fabfile changes -- print in dodo.py needs to be imported from future.
  5. Adding a pytest.ini that runs doctests as well.

file=sys.stderr option of print command needs the print function to be
imported from future.
Tests still run using pytest, by using TestCommand in setup.py.
By adding a pytest.ini so that tox and any other test runners can make
use of this.
@coveralls
Copy link

Coverage Status

Coverage remained the same at 91.62% when pulling 2c4dc5a on bsravanin:master into 1fd7c59 on wackou:master.

@bsravanin
Copy link
Author

The build failure in Python 2.6 is because of the rounding bug that was fixed in Python 2.7.

@wackou, how would you like me to handle this?

  1. I could change the example to something that passes in Python 2.6 (the way other examples do), or remove it altogether. This, I think, is plainly wrong way of doing things.
  2. I could modify change_string to use Decimal instead of floating point. I don't mind this, though the downside is that I will be changing the behavior of a function for backwards compatibility.

Also, what are your expectations for the pull request to be eligible for a merge? Do you expect all commits to pass, or just the final commit? If you expect all commits to pass, I can either squash all the commits into one large commit, or edit the chain of commits into a different set and send as a separate pull request.

@wackou
Copy link
Member

wackou commented Apr 25, 2015

Sorry I haven't had time to look at the pull request in details yet, will try to go over it during the weekend. As for python 2.6, I'm thinking about dropping support for it altogether, I feel at this point it is only holding us back, quality-wise, and doesn't bring us any benefits. Python 2.7 was released 5 years ago already!

Other than that, don't bother with squashing the commits or otherwise rewriting the history, as long as the final result works and looks nice that's all I care about 😄

@wackou
Copy link
Member

wackou commented Apr 26, 2015

looks good!

A few questions:

  • how can I enable stdout/stderr output for the tests while running them with py.test? eg: running the tests in test_matchtree.py should show:
test_base_tree (test_matchtree.TestMatchTree) ... ok
test_match (test_matchtree.TestMatchTree) ... WARNING  [guessit.matcher:checkFields] -- ---- PROPER-Xvid ----
WARNING  [guessit.matcher:checkFields] -- ADDITIONAL: 'properCount': '1'
WARNING  [guessit.matcher:checkFields] -- ---- Xvid PROPER ----
WARNING  [guessit.matcher:checkFields] -- ADDITIONAL: 'properCount': '1'

@wackou
Copy link
Member

wackou commented Apr 26, 2015

oh, and please ignore the issues with python 2.6, I have decided I will drop support for it in the next release of guessit

@bsravanin
Copy link
Author

For any additional options, you can either:

  1. Use that option with the pytest command inside an activated virtualenv.
  2. Add the option to pytest.ini if you want it to be used by default.
  3. Give it as part of the -a of setup.py (see tox.ini, e.g.)

To show stdout/stderr, use the "-s" option.

You can use -k to filter in/out any tests. To run only doctests, use -k"not test_", so that it will exclude any test with "test_" in its name (which is all of the non-doctests).

Thanks for pointing out. Fixed tabs in setup.py and re-reviewed them in the rest of the changed files.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 91.62% when pulling d1dff83 on bsravanin:master into 1fd7c59 on wackou:master.

wackou added a commit that referenced this pull request Apr 27, 2015
@wackou wackou merged commit f637f05 into guessit-io:master Apr 27, 2015
@wackou
Copy link
Member

wackou commented Apr 27, 2015

awesome, thanks for your answers!
I will remove support for python 2.6 over the coming days, so that all unittests pass.
Thanks for all the work 😄

@bsravanin
Copy link
Author

Thank you, @wackou.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants