Skip to content

TST Use assert_raises "match" parameter instead of the "message" parameter.#9264

Merged
ev-br merged 3 commits into
scipy:masterfrom
rth:use-assert-raises
Sep 16, 2018
Merged

TST Use assert_raises "match" parameter instead of the "message" parameter.#9264
ev-br merged 3 commits into
scipy:masterfrom
rth:use-assert-raises

Conversation

@rth

@rth rth commented Sep 13, 2018

Copy link
Copy Markdown
Contributor

It appears there are several occurrences in tests where the match parameter should have been used in pytest.raises instead of the message parameter.

The former checks the obtained exception message against a pattern, while the latter is the message printed on failure.

The consequence is that these tests could have been passing while they shouldn't have. Will open an issue about this confusing terminology in pytest.

This also refactors a few tests to use assert_raises instead of more complicated constructions and removes one outdated call to unittest.TestCase.fail which is no longer used as a base class for tests.

@larsoner larsoner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor gripe otherwise LGTM assuming tests pass. Thanks for fixing these.

Comment thread scipy/stats/tests/test_distributions.py Outdated
pass
else:
raise AssertionError('TypeError not raised.')
assert_raises(TypeError, _distr3_gen, **dict(name='dummy'))

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be the same as (more simply) passing name='dummy' directly rather than wrapping in a **dict(name='dummy').

@rth rth force-pushed the use-assert-raises branch from 08a1091 to 803bc52 Compare September 13, 2018 18:41
@rth

rth commented Sep 13, 2018

Copy link
Copy Markdown
Contributor Author

Thanks for the review! I addressed your comment.

There is a failure in one Travis CI build in scipy/sparse/linalg/isolve/tests/test_iterative.py which is strange as I have not changed that file. Resrtated the build several times with the same result. Doesn't look like it's an issue happening on master?

@ev-br

ev-br commented Sep 16, 2018

Copy link
Copy Markdown
Member

CI failure is unrelated (gh-9277), merging. Thanks @rth @larsoner

@ev-br ev-br merged commit 819d0e3 into scipy:master Sep 16, 2018
@ev-br ev-br added the maintenance Items related to regular maintenance tasks label Sep 16, 2018
@ev-br ev-br added this to the 1.2.0 milestone Sep 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

maintenance Items related to regular maintenance tasks

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants