-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
TST: GH30999 Add match=msg to all but two pytest.raises in tests/io #38724
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: GH30999 Add match=msg to all but two pytest.raises in tests/io #38724
Conversation
except test_sql, which has two I can't test on this machine
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.
looks good. one comment. pls merge master and ping on green.
if quoting == csv.QUOTE_NONE: | ||
# We expect no match, so there should be an assertion | ||
# error out of the inner context manager. | ||
with pytest.raises(AssertionError): |
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.
thanks, this was some really old testing code
@@ -223,11 +221,12 @@ def test_s3_parquet(s3_resource, s3so): | |||
tm.assert_equal(df1, df2) | |||
|
|||
|
|||
@td.skip_if_installed("fsspec") |
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.
extra skip here
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 fixed this, merged master, green except for Travis CI
yep your changes are fine. general restructuring of tests should be a dedicated PR, but moderinzation like this is ok. |
thanks @moink |
This pull request partially addresses xref #30999 to remove bare
pytest.raises
by addingmatch=msg
. It doesn't close that issue as I have only addressed test modules in the pandas/tests/io/ directory.I thought this was going to be relatively small because there were only 25 such instances, but it turned out some of them were in methods that were reused several times, so this ended up larger than I expected.
There were a couple of cases where I found the structure of the tests quite confusing, with an internal function definition and two nested asserts. This structure also made it difficult to add the assertion about the error message. I restructured the tests to be, in my opinion, more straightforward.
There's one file, pandas/tests/io/test_sql.py, with two instances of
pytest.raises
in tests which were skipped on my machine and which I couldn't figure out how to run, so those are left as is.black pandas
git diff upstream/master -u -- "*.py" | flake8 --diff