-
-
Notifications
You must be signed in to change notification settings - Fork 18.2k
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
TST: Use pytest #13856
TST: Use pytest #13856
Conversation
factoring asserts can be in a later PR pytest will work with nose tools (though we mostly use our own tm.* ones) |
pandas/io/tests/test_packers.py
Outdated
""" | ||
How to add msgpack tests: | ||
|
||
1. Install pandas version intended to output the msgpack. |
There was a problem hiding this comment.
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:
My understanding is that the only way to use pytest fixtures within a class is to not inherit from And pytest discovers tests in Let me know if that's unclear. |
Agreed, the only asserts I've manually rewritten were in @MaximilianR thanks for the heads about |
6985526
to
2d1d91f
Compare
pytest 3 is out now. Do we want to make the switch just after 0.19 is released? |
2d1d91f
to
ae2e146
Compare
cccc82c
to
cd9bf8f
Compare
0e63f0f
to
ab3d85e
Compare
Codecov Report
@@ Coverage Diff @@
## master #13856 +/- ##
=========================================
Coverage ? 87.88%
=========================================
Files ? 142
Lines ? 51106
Branches ? 0
=========================================
Hits ? 44913
Misses ? 6193
Partials ? 0
Continue to review full report at Codecov.
|
f95208f
to
312f4e5
Compare
.travis.yml
Outdated
@@ -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" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could just rename these
ci/install_appveyor.ps1
Outdated
@@ -126,8 +126,8 @@ function UpdateConda ($python_home) { | |||
function main () { | |||
InstallMiniconda $env:PYTHON_VERSION $env:PYTHON_ARCH $env:PYTHON | |||
UpdateConda $env:PYTHON | |||
InstallCondaPackages $env:PYTHON "pip setuptools nose" | |||
InstallCondaPackages $env:PYTHON "pip setuptools nose pytest" |
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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).
.travis.yml
Outdated
@@ -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" |
There was a problem hiding this comment.
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
pandas/io/tests/test_pickle.py
Outdated
@@ -32,6 +33,7 @@ class TestPickle(): | |||
""" | |||
_multiprocess_can_split_ = True | |||
|
|||
@pytest.fixture(autouse=True) |
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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
)
There was a problem hiding this comment.
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.
doc/source/contributing.rst
Outdated
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] |
There was a problem hiding this comment.
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.
@TomAugspurger all merged. rebase and let's get this in! |
|
||
For more, see the `pytest<http://doc.pytest.org/en/latest/>`_ documentation. | ||
|
||
.. versionadded:: 0.18.0 |
There was a problem hiding this comment.
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"): |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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 1. The base class inherits from `TestCase`, and 2. The base class doesn't start with `Test` (e.g. PandasSQLTest) 3. 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.
1be6ea5
to
59e2be9
Compare
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. |
thanks @TomAugspurger awesome job! |
I made a couple of minor changes: 7713f29 and added a bit to the 'list' of TODOs, but otherwise merged very nicely! |
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
xref pandas-dev#13856 (comment) Author: Jeff Reback <jeff@reback.net> Closes pandas-dev#15333 from jreback/mcs and squashes the following commits: 2edc842 [Jeff Reback] TST/STYLE: remove multiprocess nose flags and slight PEP fixes
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
git diff upstream/master | flake8 --diff
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
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.
See the sql tests.
py.test
discovers_Test*
class (if they inherit fromTestCase
?), while unittest does not.Solution: move TestCase inheritance down a level
See e.g. pandas/src/testing.py
assert cond, msg
asif not cond; raise AssertionError(msg)