Opened at 2015-01-21T18:43:54Z
Closed at 2016-03-27T18:48:56Z
#2366 closed task (fixed)
set up buildslave to find deprecation warnings
Reported by: | warner | Owned by: | |
---|---|---|---|
Priority: | normal | Milestone: | soon (release n/a) |
Component: | dev-infrastructure | Version: | 1.10.0 |
Keywords: | DeprecationWarning tests buildbot travis-ci | Cc: | |
Launchpad Bug: |
Description
#2312 was a DeprecationWarning that resulted from Tahoe code misusing an interface (twisted.web.Request.setHeader) and the Twisted folks eventually reminding people to use it correctly by signalling a warning unless you did it right. The warning was suppressed on most of our buildslaves (running py2.7, where DeprecationWarnings are not enabled by default), but a few py2.6 builders noticed it.
It'd be nice to notice these things better. I'd like to set up a builder that adds PYTHONWARNINGS=default::DeprecationWarning, to re-enable these warnings (even under python2.7). I'd also like to add some tooling which gathers the warnings, removes ones that are known problems in other people's code, and goes red if Tahoe itself triggers anything.
It'd probably be most useful to run this builder with the latest version of Twisted, maybe even Twisted trunk, to find problems early. Ideally we'd run this with the full (13-dimensional?) matrix of dependency versions, but I think Twisted is the one place that's most likely to produce actionable deprecation warnings.
Change History (10)
comment:1 Changed at 2015-01-21T20:17:11Z by daira
comment:2 Changed at 2015-01-21T20:56:05Z by warner
Well, I think it will produce a lot of noise, which is why I'm inclined to stick it on a dedicated buildbot column and have some scripts to analyze the results. My quick pass shows about 200 lines of DeprecationWarnings, most of which are twisted's fault (twisted.spread.jelly imports "sets", nevow.appserver uses http.Request.received_headers, there's a IStreamClientEndpointStringParser which may or may not be something we can fix). I'd like the tooling to have a file with "known and unlikely to be fixed" problems, and have it emit errors for anything else. To do that on travis.. I guess that'd mean running the tests a second time, with warnings turned on, writing the results to a tempfile, running a script over the tempfile, and exiting with non-zero if the script identified a problem. (as you pointed out, turning on warnings also causes some tests to fail because they're expecting silent output of a child process, which we'd need to find a way to deal with).
So I guess no :)
comment:3 Changed at 2015-01-22T02:59:27Z by daira
Oh sorry, I pushed a change to do that without noticing your comment. But note that this only makes the Python 2.7 Travis-CI test run behave like Python 2.6, it doesn't "unsuppress" the warnings that are explicitly suppressed at src/allmydata/_auto_deps.py#L224.
comment:4 Changed at 2015-02-07T00:21:11Z by daira
- Keywords DeprecationWarning tests buildbot travis-ci added
comment:5 Changed at 2015-03-20T08:40:08Z by warner
I've added this to the ubuntu-trusty builder, because it was convenient and runs fairly quickly. There are three warnings, all in other project's code (two in nevow, one in twisted).
Here's a sample of the filtered output: https://tahoe-lafs.org/buildbot-tahoe-lafs/builders/Ubuntu%20trusty%2014.04/builds/45/steps/deprecations/logs/warnings
The next steps are:
- filter out the DeprecationWarnings that are embedded in test failure messages (failures caused by subcommands emitting noise when they should be quiet)
- determine which warnings are in tahoe's code, and which are elsewhere (at present, all are elsewhere)
- return status of ERROR if any are found in tahoe's code, or WARNINGS for ones found in third-party code
(this assumes that the third-party warnings will get cleaned up: they might WONTFIX those, in which case we'll want to suppress them instead)
comment:6 Changed at 2015-03-20T17:30:38Z by daira
An alternative approach that enables DeprecationWarnings unconditionally in tests is at https://github.com/tahoe-lafs/tahoe-lafs/commits/2312.enable-deprecation-warnings-for-tests.0 .
I tried to enable DeprecationWarnings only for allmydata.* by using PYTHONWARNINGS=default::DeprecationWarning:allmydata.*, but that didn't work (it didn't correctly match submodules).
comment:7 Changed at 2015-03-20T19:54:16Z by warner
it looks like that patch only changes three test pathways (make quicktest, make test-coverage, and setup.py trial), but not the one that travis uses (bin/tahoe debug trial), which is why travis is passing even though a local make quicktest would probably fail.
I love the idea of enabling the warnings only for our own code.. I'd be ok with that going in unconditionally. Let's see if there's a way to make the submodules work, and try to turn them on in the test-running commands, instead of changing the buildbot to scan/categorize everything.
comment:8 Changed at 2015-04-28T17:12:30Z by daira
There are currently three warnings shown by that buildslave:
We should suppress these so that any failure of the buildslave is telling us something new.
comment:9 Changed at 2015-04-28T17:13:04Z by daira
- Milestone changed from eventually to soon (release n/a)
comment:10 Changed at 2016-03-27T18:48:56Z by warner
- Resolution set to fixed
- Status changed from new to closed
We have one of these now (it's done as part of the "Ubuntu wily 15.10" builder, on our main linux buildslave).
If you want to check for deprecations yourself (locally), use tox -e deprecations.
We can enable deprecation warnings for the Travis-CI Python 2.7 build fairly easily -- do you want to do that?