TST: Use pytest by TomAugspurger · Pull Request #13856 · pandas-dev/pandas (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation105 Commits11 Checks0 Files changed
Conversation
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]({{ revealButtonHref }})
- closes TST: Start using py.test #13097
- tests added / passed
- passes
git diff upstream/master | flake8 --diff - whatsnew entry
Just a WIP for now, will need to iterate on the CI stuff to ensure that's all good.
What can I provide to make this easier to review? Refactoring tests is scary.
I'll give verbose outputs of nose on master and pytest on this branch.
junit.xml output too. Anything else?
Here are some notes I took along the way
Kinds of Changes
- Nosetests Generators to fixtures
See TestPickle, TestMsgPack and this SO post. We switched to using parametrized fixtures (yay) and dependency injection instead of setUp + a for loop inside the test.
- Different Discover Rules
See the sql tests. py.test discovers _Test* class (if they inherit from TestCase?), while unittest does not.
Solution: move TestCase inheritance down a level
- Assert Rewriting
See e.g. pandas/src/testing.py
- solution: rewrite
assert cond, msgasif not cond; raise AssertionError(msg) - looking into just excluding these files... They aren't actual tests, just test helpers.
- CI work:
- updating nose to pytest
factoring asserts can be in a later PR
pytest will work with nose tools (though we mostly use our own tm.* ones)
| """ |
|---|
| How to add msgpack tests: |
| 1. Install pandas version intended to output the msgpack. |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this procedure remain?
Thanks a lot @TomAugspurger !
One note on something above:
py.test discovers _Test* class (if they inherit from TestCase?), while unittest does not.
My understanding is that the only way to use pytest fixtures within a class is to not inherit from TestCase. This can make it a bit more difficult to gradually transition, but you can use autouse=True on existing setUp methods, and the inheritance then isn't necessary.
And pytest discovers tests in Test* named classes, not *Test, unless they inherit from TestCase.
Let me know if that's unclear.
factoring asserts can be in a later PR
Agreed, the only asserts I've manually rewritten were in src/testing.pyx to get the tests to pass. I believe this is due to pytest-dev/pytest#645, which has been fixed and will be included in pytest 3.0 which is coming out soonish. They were targeting Europython, but pushed it back.
I'll revert those changes and wait for pytest3 I think.
@MaximilianR thanks for the heads about autouse=True, I'll check that out. My initial goal was to have the test-suite runnable by either nose or pytest runners. Then followup PRs could make use of pytest features.
Doesn't look like I'll be able to achieve that though due to the nose generator tests.
pytest 3 is out now. Do we want to make the switch just after 0.19 is released?
| @@ -44,7 +44,7 @@ matrix: |
|---|
| - python: 2.7 |
| env: |
| - JOB_NAME: "27_slow_nnet_LOCALE" |
| - NOSE_ARGS="slow and not network and not disabled" |
| - NOSE_ARGS="--skip-network" |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could just rename these
| @@ -126,8 +126,8 @@ function UpdateConda ($python_home) { |
|---|
| function main () { |
| InstallMiniconda env:PYTHONVERSIONenv:PYTHON_VERSION env:PYTHONVERSIONenv:PYTHON_ARCH $env:PYTHON |
| UpdateConda $env:PYTHON |
| InstallCondaPackages $env:PYTHON "pip setuptools nose" |
| InstallCondaPackages $env:PYTHON "pip setuptools nose pytest" |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would remove nose (so that it doesn't accidently run)
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jreback I think we should keep nose for now for things like SkipTest. I'll make a followup issue to clean all this up though (changing skips, removing generator-based tests, and using more fixtures are the big ones).
| @@ -82,7 +82,7 @@ matrix: |
|---|
| - python: 3.5 |
| env: |
| - JOB_NAME: "35_nslow" |
| - NOSE_ARGS="not slow and not network and not disabled" |
| - NOSE_ARGS="--skip-slow --skip-network" |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
need to remove nose from any ci/requirements* files
| @@ -32,6 +33,7 @@ class TestPickle(): |
|---|
| """ |
| _multiprocess_can_split_ = True |
| @pytest.fixture(autouse=True) |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't use autouse its a bit magical, create explict fixtures (we can come back and fix this later though if easier)
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this might have been my suggestion.
Without this, I'm not sure setUp will run unless the class inherits from unittest.TestCase.
I think we could change setUp to [setup_class](http://doc.pytest.org/en/latest/xunit_setup.html) and it would run pytest. (Or setup_method if we need access to self rather than class)
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MaximilianR thanks, changed to use setup_class and all the tests run / pass. Made the same change in test_packers to remove autouse.
| nosetests pandas/tests/[test-module].py:[TestClass] |
|---|
| nosetests pandas/tests/[test-module].py:[TestClass].[test_method] |
| pytest pandas/tests/[test-module].py |
| pytest pandas/tests/[test-module].py::[TestClass] |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would instead recommend using -k, in fact this is one of the main reasons I really like pytest, e.g.
pytest pandas/[tests-module].py -k cool_test IMHO is much easiest then finding class names and the exact names of the methods.
[WIP] TST: Use pytest TST: Use pytest
| For more, see the `pytesthttp://doc.pytest.org/en/latest/`_ documentation. |
| .. versionadded:: 0.18.0 |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
prob should take out this tag
| if 'slow' not in item.keywords and item.config.getoption("--only-slow"): |
|---|
| pytest.skip("skipping due to --only-slow") |
| if 'skip' in item.keywords and item.config.getoption("--skip-network"): |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
on the todo list, let's just make @network an actual marker then this becomes trival
| @@ -2549,9 +2550,7 @@ def assert_produces_warning(expected_warning=Warning, filter_level="always", |
|---|
| % extra_warnings) |
| def disabled(t): |
| t.disabled = True |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you might need to fix TestCase, IOW the setUpClass and such.
can you rebase and we will try for tomorrow for merging?
Pytest wouldn't discover the tests in the same way as nose when
- The base class inherits from
TestCase, and - The base class doesn't start with
Test(e.g. PandasSQLTest) - The parent class with test methods doesn't start with
Test( e.g. _TestSQLAPI)
We solved this by moving the class inheriting from TestCase down a
level from PandasSQLTest to the many TestSQL* methods.
Pytest won't use setUp unless you inherit from TestCase. These classes don't because they use nose-style generator tests. We change to setup_class becuase that is called by the pytest runner.
Replaced a init method with a setUp method; Makes the XML generated by nose more standard.
TST: Move staticmethod to top-level Makes the XML generated by nose more standard.
update ci
DOC
Fixup runners
added only-slow
appveyor
Move conftest
Removes all pandas.util.nosetester as interactive's no longer needed
On some travis runs, locale.getlocale() returned (None, None).
When the tm.set_locale context manager exists, we set it
to locale.getlocale(locale.LC_ALL, None), which defauls to utf-8,
so when we compare afterwards, we get a failure.
Rebased and travis is green. I'm under a deadline for Wednesday, so won't be able to start on followup issues till later next week.
ok thanks @TomAugspurger let me play a bit with this. I think we can just merge and fix anything else later.
I made a couple of minor changes: 7713f29 and added a bit to the 'list' of TODOs, but otherwise merged very nicely!
AnkurDedania pushed a commit to AnkurDedania/pandas that referenced this pull request
Prevent locale from affecting Stata file date creation, which must be en_US.
xref pandas-dev#13856
Author: Kevin Sheppard kevin.k.sheppard@gmail.com
Closes pandas-dev#15208 from bashtage/stata-locale-date-fix and squashes the following commits:
a55bfd8 [Kevin Sheppard] BUG: Remove locale conversion from Stata file date
AnkurDedania pushed a commit to AnkurDedania/pandas that referenced this pull request
AnkurDedania pushed a commit to AnkurDedania/pandas that referenced this pull request
closes pandas-dev#13097
Author: Tom Augspurger tom.augspurger88@gmail.com
Closes pandas-dev#13856 from TomAugspurger/pytest and squashes the following commits:
59e2be9 [Tom Augspurger] NOSE_ARGS -> TEST_ARGS 03695aa [Tom Augspurger] TST: Remove disabled marks 42790ae [Tom Augspurger] TST: Remove test_multi.sh 40d7336 [Tom Augspurger] PKG: redo pd.test import 9ba1f12 [Tom Augspurger] TST: Skip if getlocale is None 14c447c [Tom Augspurger] TST: pd.test uses pytest c4f6008 [Tom Augspurger] TST/CI: Use pytest b268d89 [Tom Augspurger] TST: Change method to make reporting more standard a638390 [Tom Augspurger] TST: Test consistency change c8dc927 [Tom Augspurger] TST: Refactor to use setup_class 9b5f2b2 [Tom Augspurger] TST: Refactor sql test inheritance
Labels
Continuous Integration
pandas testing functions or related to the test suite