set up buildslave to find deprecation warnings #2366

New Issue

warner · 2015-01-21T18:43:54Z

warner commented

2015-01-21 18:43:54 +00:00

#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.

#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.

warner added the

labels 2015-01-21 18:43:54 +00:00

warner added this to the eventually milestone 2015-01-21 18:43:54 +00:00

daira commented

2015-01-21 20:17:11 +00:00

We can enable deprecation warnings for the Travis-CI Python 2.7 build fairly easily -- do you want to do that?

warner commented

2015-01-21 20:56:05 +00:00

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 :)

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 `DeprecationWarning`s, 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 :)

daira commented

2015-01-22 02:59:27 +00:00

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 source:src/allmydata/_auto_deps.py#L224.

warner commented

2015-03-20 08:40:08 +00:00

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)

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)

daira commented

2015-03-20 17:30:38 +00:00

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).

An alternative approach that enables `DeprecationWarning`s unconditionally in tests is at <https://github.com/tahoe-lafs/tahoe-lafs/commits/2312.enable-deprecation-warnings-for-tests.0> . I tried to enable `DeprecationWarning`s only for `allmydata.*` by using `PYTHONWARNINGS=default::DeprecationWarning:allmydata.*`, but that didn't work (it didn't correctly match submodules).

warner commented

2015-03-20 19:54:16 +00:00

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.

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.

daira commented

2015-04-28 17:12:30 +00:00

There are currently three warnings shown by that buildslave:

https://tahoe-lafs.org/buildbot-tahoe-lafs/builders/Ubuntu%20trusty%2014.04/builds/63/steps/deprecations/logs/warnings

We should suppress these so that any failure of the buildslave is telling us something new.

There are currently three warnings shown by that buildslave: <https://tahoe-lafs.org/buildbot-tahoe-lafs/builders/Ubuntu%20trusty%2014.04/builds/63/steps/deprecations/logs/warnings> We should suppress these so that any failure of the buildslave is telling us something new.

tahoe-lafs modified the milestone from eventually to soon (release n/a)

2015-04-28 17:13:04 +00:00

warner commented

2016-03-27 18:48:56 +00:00

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 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`.

warner added the

fixed

label 2016-03-27 18:48:56 +00:00

warner closed this issue

2016-03-27 18:48:56 +00:00

Sign in to join this conversation.