Skip to content

Conversation

@wjbenfold
Copy link
Contributor

@wjbenfold wjbenfold commented May 24, 2022

🚀 Pull Request

Description

Currently it's hard to see your test failures for the warning summary. However, suppressing the warning summary also loses us the warnings as they happen, which may well lead to us ignoring them / losing track of them. Should we merge this?


Consult Iris pull request check list

@trexfeathers
Copy link
Contributor

Would it be too much to try something like #4384 (reply in thread) at this point?

@rcomer
Copy link
Member

rcomer commented May 24, 2022

Looking through the output, most of the warnings seem to be coming out of Iris itself. For UserWarnings, could we just suppress the Iris ones and then see what's left?

@wjbenfold
Copy link
Contributor Author

I think most of the warnings that get printed are Iris, but there are a lot of "22 warnings in..." that will get printed in pytest's warning summary even if we suppress Iris's warnings?

@wjbenfold
Copy link
Contributor Author

Also, worryingly, the last test I ran with warnings showing included this (fyi @lbdreyer):

lib/iris/tests/unit/fileformats/pp/test_PPField.py:46: 15 warnings
  /tmp/cirrus-ci-build/lib/iris/tests/unit/fileformats/pp/test_PPField.py:46: PytestCollectionWarning: cannot collect test class 'TestPPField' because it has a __init__ constructor (from: lib/iris/tests/unit/fileformats/pp/test_PPField.py)
    class TestPPField(PPField):

@rcomer
Copy link
Member

rcomer commented May 24, 2022

Also, worryingly, the last test I ran with warnings showing included this (fyi @lbdreyer):

lib/iris/tests/unit/fileformats/pp/test_PPField.py:46: 15 warnings
  /tmp/cirrus-ci-build/lib/iris/tests/unit/fileformats/pp/test_PPField.py:46: PytestCollectionWarning: cannot collect test class 'TestPPField' because it has a __init__ constructor (from: lib/iris/tests/unit/fileformats/pp/test_PPField.py)
    class TestPPField(PPField):

I spotted that too. I looked and concluded it should probably be renamed to something like DummyPPField or MockPPField as it's a subclass of the PPField class to use in the tests. Then I forgot all about it...

class TestPPField(PPField):

@rcomer
Copy link
Member

rcomer commented May 24, 2022

Also, worryingly, the last test I ran with warnings showing included this (fyi @lbdreyer):

This one is now fixed by #4764

@wjbenfold
Copy link
Contributor Author

I think reducing the warnings Iris fires rather than hiding them all in CI is a better shout

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.

3 participants