Builder.setSlaveForTesting is now a liability
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
Launchpad itself |
Fix Released
|
High
|
Guilherme Salgado |
Bug Description
The Builder class has a method setSlaveForTesting, which only exists to let tests inject test doubles for build slaves. The ability is important because without a test double, tests will trigger attempts to contact nonexistent builder slaves over the network.
But it no longer works properly. The buildmaster code needs to commit its database changes regularly (whenever there's any chance of other builders committing or aborting on the same store that has changes pending) and every commit/abort resets the builder slave reference to a real, proper BuilderSlave.
This leaves us with a situation where it's very easy to break tests. The failures are obscure and contain no tracebacks. A trap for unwary engineers.
Instead, tests should now patch the slave factory method underlying the BuilderSlave.slave cachedproperty:
self.
Related branches
- Ian Booth (community): Approve
-
Diff: 512 lines (+67/-59)10 files modifiedlib/lp/buildmaster/interfaces/builder.py (+1/-6)
lib/lp/buildmaster/model/builder.py (+1/-18)
lib/lp/buildmaster/tests/test_builder.py (+8/-5)
lib/lp/buildmaster/tests/test_manager.py (+15/-10)
lib/lp/buildmaster/tests/test_packagebuild.py (+2/-1)
lib/lp/code/model/tests/test_sourcepackagerecipebuild.py (+2/-1)
lib/lp/soyuz/tests/test_binarypackagebuild.py (+4/-1)
lib/lp/soyuz/tests/test_binarypackagebuildbehavior.py (+29/-15)
lib/lp/translations/stories/buildfarm/xx-build-summary.txt (+3/-1)
lib/lp/translations/tests/test_translationtemplatesbuildbehavior.py (+2/-1)
Changed in launchpad: | |
assignee: | nobody → Guilherme Salgado (salgado) |
status: | Triaged → In Progress |
Fixed in stable r14777 (http:// bazaar. launchpad. net/~launchpad- pqm/launchpad/ stable/ revision/ 14777) by a commit, but not testable.