Skip to content

Use unittest-style tests#440

Merged
jvkersch merged 22 commits into
masterfrom
tst/unittest-framework
May 28, 2019
Merged

Use unittest-style tests#440
jvkersch merged 22 commits into
masterfrom
tst/unittest-framework

Conversation

@jvkersch
Copy link
Copy Markdown
Contributor

fixes #439

The first few commits are concerned with turning nose-style tests into unittest test cases (this was done in a semi-automatic way, with lots of Emacs macros). The remaining commits do the following:

  • Make each tests folder into a package
  • Rename test module to start with test_ so that it can be picked up by the unittest discovery runner
  • Make each test have at least one assert (some tests would check something and then print a pass/fail message to stdout, others would just print things and not even do any checking).

Along the way, I did some minor cleanup (removing unused print statements, rewriting overly complicated six constructs, etc). I also made it so that the backend tests are run as part of the test suite: now that PyQt is easily installed, there is no need to keep these tests separate.

@jvkersch jvkersch requested a review from corranwebster May 19, 2019 11:11
Comment thread chaco/tests/test_component.py Outdated
Comment thread chaco/tests/test_image_plot.py Outdated
Comment thread ci/edmtool.py Outdated
@corranwebster
Copy link
Copy Markdown
Contributor

Generally looks good. A couple of minor questions, and the major issue that it isn't actually running any tests in CI right now.

@jvkersch
Copy link
Copy Markdown
Contributor Author

@corranwebster Tests pass (and are actually running this time). I think I've addressed your comments, would you like to give this another look?

@jvkersch
Copy link
Copy Markdown
Contributor Author

Actually, I marked one test as an expectedFailure that requires a bit more looking into (it fails because of a warning raised in Traits).

@jvkersch
Copy link
Copy Markdown
Contributor Author

@corranwebster I've fixed up the issue with the expected failure, and I think this is good to go. Would you mind giving this a last look?

tool = RangeSelection(renderer)
with warnings.catch_warnings(record=True) as w:
# Ignore warnings coming from Traits
warnings.filterwarnings(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Being a bit picky here, but I think this is changing the global state of the warning filters but not resetting it, and so could affect the outcome of other tests which mess with warnings. Unfortunately this looks awkward to do in the warnings module withot messing with warnings.filter directly.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't think this is an issue here, since it's wrapped in a catch_warnings block which will reset the filters upon exit:

In [1]: import warnings

In [2]: print(warnings.filters)
[('default', re.compile('', re.IGNORECASE), <class 'DeprecationWarning'>, re.compile('__main__'), 0), ('ignore', None, <class 'DeprecationWarning'>, None, 0), ('ignore', None, <class 'PendingDeprecationWarning'>, None, 0), ('ignore', None, <class 'ImportWarning'>, None, 0), ('ignore', None, <class 'BytesWarning'>, None, 0), ('ignore', None, <class 'ResourceWarning'>, None, 0)]

In [3]: with warnings.catch_warnings(record=True) as w:
   ...:     warnings.filterwarnings("ignore", 'elementwise == comparison failed')
   ...:     print(warnings.filters)
   ...:     
[('ignore', re.compile('elementwise == comparison failed', re.IGNORECASE), <class 'Warning'>, re.compile(''), 0), ('default', re.compile('', re.IGNORECASE), <class 'DeprecationWarning'>, re.compile('__main__'), 0), ('ignore', None, <class 'DeprecationWarning'>, None, 0), ('ignore', None, <class 'PendingDeprecationWarning'>, None, 0), ('ignore', None, <class 'ImportWarning'>, None, 0), ('ignore', None, <class 'BytesWarning'>, None, 0), ('ignore', None, <class 'ResourceWarning'>, None, 0)]

In [4]: print(warnings.filters)
[('default', re.compile('', re.IGNORECASE), <class 'DeprecationWarning'>, re.compile('__main__'), 0), ('ignore', None, <class 'DeprecationWarning'>, None, 0), ('ignore', None, <class 'PendingDeprecationWarning'>, None, 0), ('ignore', None, <class 'ImportWarning'>, None, 0), ('ignore', None, <class 'BytesWarning'>, None, 0), ('ignore', None, <class 'ResourceWarning'>, None, 0)]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Cool, I didn't realise that.

Copy link
Copy Markdown
Contributor

@corranwebster corranwebster left a comment

Choose a reason for hiding this comment

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

LGTM

@jvkersch
Copy link
Copy Markdown
Contributor Author

Thanks @corranwebster !

@jvkersch jvkersch merged commit 1d538d4 into master May 28, 2019
@jvkersch jvkersch deleted the tst/unittest-framework branch May 28, 2019 14:15
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.

Move away from nose style tests

2 participants