-
Notifications
You must be signed in to change notification settings - Fork 149
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
bug_1447315 - Fix warnings #798
bug_1447315 - Fix warnings #798
Conversation
9265fbd
to
0884d50
Compare
tox.ini
Outdated
@@ -15,7 +15,7 @@ commands= | |||
basepython=python2.7 | |||
deps = -rrequirements-test.txt | |||
commands= | |||
py.test -n2 --cov=auslib --doctest-modules {posargs:auslib} | |||
py.test -W default::DeprecationWarning -W default::PendingDeprecationWarning -n2 --cov=auslib --doctest-modules {posargs:auslib} |
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.
Can you adjust agent/ and client/ for this, too?
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.
Sure
0884d50
to
231fdc6
Compare
@mozbhearsum, in my first commit I completely forgot py3 task, the warnings were fixed for our code, but a bunch of warnings that come from sqlalchemy-migrate and other packages still printed in the warning summary: I do not touched in client test config, once it does not use py.test, also, I do not found an explicity option to enable deprecation warnings in nosetests. Let me know if you want to change test suite of cliente to py.test. |
I'll take over the review here. Planning to take a look first thing next week. |
The balrog changes all look good to me, there's just the py36 warnings from the deps to suppress if we can. It's somewhat confusing that py27 does not have this behaviour so it might have been a change in py36 itself. Probably we need to set up Of note is that pytest 3.8 has a changelog entry |
Edit - nope! It was a little while ago. Anyway, definitely worth merging from master. |
9becfed
to
d576690
Compare
@nthomas-mozilla, The only way I found to hide warnings from other libs is using the regex filter: Here the test log, in which I forced a warning in balrog code: |
tox.ini
Outdated
default::DeprecationWarning | ||
default::PendingDeprecationWarning | ||
ignore:.*inspect.getargspec.* is deprecated.* | ||
ignore:invalid escape sequence.* |
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.
@allan-silva sorry for the delay in getting back to this. We've taken the pytest 3.9 upgrade via pyup so it would be good to rerun the tests and make sure the filters are still set correctly.
I also wondered if this approach might be helpful in ./tox.ini:
filterwarnings =
ignore
error:::auslib
default::DeprecationWarning:auslib
default::PendingDeprecationWarning:auslib
The idea is to ignore all warnings in deps, while in auslib warn about deprecations, and upgrade to error anything else. It would avoid suppressing the named errors in Balrog code, I think.
The other tox.ini would need equivalent changes too ?
d576690
to
8109c10
Compare
8109c10
to
f08d082
Compare
@nthomas-mozilla I updated the tox files to use |
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.
self.assertEqual('this pull request', 'extremely awesome').
So fantastic to have tidy test logs again.
Big part of the warnings from https://taskcluster-artifacts.net/BW9QlMpERfC91iaIEvimmg/0/public/logs/live_backing.log, was fixed in #621.