Skip to content
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

Move from nose to pytest #149

Closed
wants to merge 7 commits into from
Closed

Move from nose to pytest #149

wants to merge 7 commits into from

Conversation

kuenishi
Copy link
Member

@kuenishi kuenishi commented Dec 12, 2017

Chainer has moved to pytest since 3.0.0 and now MN follows.
An example of testing tool mismatch is: Chainer's parameterized test
started checking all Exceptions including nose SkipTest(), at #3963
and #3876 which are included in 3.2.0 and 4.0, and all unit tests that
skips is recognized as test failure. Additionally, this patch starts
supporting Chainer 3.2.0 and 2.1.0, dropping 1.x as well.

@kuenishi kuenishi changed the title Move from nose to pytest [WIP] Move from nose to pytest Dec 12, 2017
@kuenishi
Copy link
Member Author

memo: chainer/chainer#3952 is the key difference between Chainer 3.2 and 4.0 on 2534888 test failure.

- Chainer has moved to pytest since 3.0.0 and now MN follows.  An
  example of testing tool mismatch is: Chainer's parameterized test
  started checking all Exceptions including nose SkipTest(), at #3963
  and #3876 which are included in 3.2.0 and 4.0, and all unit tests
  that skips is recognized as test failure. Additionally, this patch
  starts supporting Chainer 3.2.0 and 2.1.0, dropping 1.x as well (One
  more issue to support 4.0 remains).
- Also, mixture of chainer.testing.attr.gpu and
  chainer.testing.parametrize, with passing pytest to `-m 'not gpu'
  does not work in pytest because nose's attributes are direct
  attribute to test object or function, in contrast to pytest. Thus
  this patch dissolves all parametrized tests that is using `{gpu:
  True}, {gpu: False}` or `{nccl: True}, {nccl: False}` as parameters,
  which is replaced by simple function call patters with
  `@pytest.mark.gpu` and `@pytest.mark.nccl`.
- Add Python 3.6 as test target in Travis.
- Fix test_scatter_large_dataset not working for flat communicator
@kuenishi
Copy link
Member Author

kuenishi commented Dec 15, 2017

I think its ready for review. And quote from last commit:

  • Chainer has moved to pytest since 3.0.0 and now MN follows. An
    example of testing tool mismatch is: Chainer's parameterized test
    started checking all Exceptions including nose SkipTest(), at #3963
    and #3876 which are included in 3.2.0 and 4.0, and all unit tests
    that skips is recognized as test failure. Additionally, this patch
    starts supporting Chainer 3.2.0 and 2.1.0, dropping 1.x as well (One
    more issue to support 4.0 remains).
  • Also, mixture of chainer.testing.attr.gpu and
    chainer.testing.parametrize, with passing pytest to -m 'not gpu'
    does not work in pytest because nose's attributes are direct
    attribute to test object or function, in contrast to pytest. Thus
    this patch dissolves all parametrized tests that is using {gpu: True}, {gpu: False} or {nccl: True}, {nccl: False} as parameters,
    which is replaced by simple function call patters with
    @pytest.mark.gpu and @pytest.mark.nccl.
  • Add Python 3.6 as test target in Travis.
  • Fix test_scatter_large_dataset not working for flat communicator

@kuenishi kuenishi mentioned this pull request Dec 15, 2017
@kuenishi kuenishi self-assigned this Dec 15, 2017
@kuenishi
Copy link
Member Author

kuenishi commented Dec 18, 2017

CI test failures are all against Chainer 2.1 and this is because it's chainer.testing.attr.gpu implementation works only for nose. I'd rather want to drop Chainer 2.1 support than to add hack to work on this issue.

@kuenishi
Copy link
Member Author

As it turned out we don't need any nccl notation in test selection argument with -a or -m, I'd close this complicated pull request and reopen another one.

@kuenishi kuenishi closed this Dec 18, 2017
@kuenishi kuenishi deleted the pytest branch December 18, 2017 09:04
@kuenishi
Copy link
Member Author

kuenishi commented Dec 18, 2017

Superseded by #167 .

@kuenishi kuenishi removed this from the v1.1.0 milestone Dec 19, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants