Builder.setSlaveForTesting is now a liability

Bug #888010 reported by Jeroen T. Vermeulen
6
This bug affects 1 person
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.patch(BuilderSlave, 'makeBuilderSlave', FakeMethod(test_slave))

Related branches

Changed in launchpad:
assignee: nobody → Guilherme Salgado (salgado)
status: Triaged → In Progress
Revision history for this message
Launchpad QA Bot (lpqabot) wrote :

Fixed in stable r14777 (http://bazaar.launchpad.net/~launchpad-pqm/launchpad/stable/revision/14777) by a commit, but not testable.

tags: added: qa-untestable
Changed in launchpad:
status: In Progress → Fix Committed
Revision history for this message
Robert Collins (lifeless) wrote :

(test only changes -> it is done-done now)

Changed in launchpad:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.