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

Switch to pytest #51

Merged
merged 8 commits into from
Jun 28, 2015
Merged

Switch to pytest #51

merged 8 commits into from
Jun 28, 2015

Conversation

sontek
Copy link
Contributor

@sontek sontek commented Jun 20, 2015

This switches you to use pytest instead of nose which is a much more active project and has a much better reporting mechanism. This also enables displaying code coverage by default when running tests.
#55 should be merged first.

@cgordon
Copy link
Collaborator

cgordon commented Jun 20, 2015

Can you give me more context for this change? I'm not familiar with pytest, and have used nose at both jobs for which I used Python. Can you point me at some comparisons somewhere, or just give me a quick synopsis of why pytest is better?

@sontek
Copy link
Contributor Author

sontek commented Jun 20, 2015

@cgordon Yeah, sorry I should have marked this one as a work in progress. You will see the benefit once I have the integration tests ported over as well. I'll show some before/after output as well.

@sontek
Copy link
Contributor Author

sontek commented Jun 20, 2015

@cgordon ok, check out the integration tests now

@sontek
Copy link
Contributor Author

sontek commented Jun 20, 2015

So the primary reasons you would want to switch:

  1. More pythonic, you can use assert foo == bar instead of tools.assert_equal(foo, bar)

  2. py.test is more actively maintained and by an extremely strong maintainer (guy who also created tox and devpi)

  3. Better error report, for example look at this:

    def test_dict_difference():
        assert {'foo': 'bar'} == {'bar': 'foo'}

    In nosetests:

    Traceback (most recent call last):
      File "/usr/lib/python2.7/site-packages/nose/case.py", line 197, in runTest
        self.test(*self.arg)
      File "/home/sontek/venvs/pymemcache/src/test_foo.py", line 2, in test_dict_difference
        assert {'foo': 'bar'} == {'bar': 'foo'}
    AssertionError
    

    in pytest:

        def test_dict_difference():
    >       assert {'foo': 'bar'} == {'bar': 'foo'}
    E       assert {'foo': 'bar'} == {'bar': 'foo'}
    E         Left contains more items:
    E         {'foo': 'bar'}
    E         Right contains more items:
    E         {'bar': 'foo'}
    E         Full diff:
    E         - {'foo': 'bar'}
    E         + {'bar': 'foo'}
    
  4. For setting up fixtures (dependencies of your tests) you have the ability to use dependency injection to send many different parameters to your test functions (check the integration test file to see this in action)

@sontek sontek mentioned this pull request Jun 20, 2015
6 tasks
@cgordon
Copy link
Collaborator

cgordon commented Jun 22, 2015

Any reason to keep pymemcache/test/integration.py around? Other than that, I'm ready to merge here. Pytest looks nicer, and I really like the output on failed tests.

@sontek
Copy link
Contributor Author

sontek commented Jun 22, 2015

@cgordon oh, nope. Just missed the delete on my commit. I'll fix

cgordon added a commit that referenced this pull request Jun 28, 2015
@cgordon cgordon merged commit 5c480dc into pinterest:master Jun 28, 2015
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

2 participants