have a better-defined approach to mocking

[dustin]

This year's testing track at PyCon had a lot of content about hitting the right approach with mocks -- and the pains of not doing so.

One major point was to make your mocks authoritative (one or zero mocks for any system), narrow (minimal implementation), and isolated (don't depend on the real system).

Most of the mocks used in Buildbot are already authoritative -- the stuff in master/buidlbot/test/fake in particular, but even some of the mocks in individual test files are unique. There are a lot of status and process class mocks, thought, that are not authoritative.

The DB mocks, at least, are narrow - they are pretty poor implementations of the API, in fact. And they are quite well isolated.

The path from here is:

• state this as policy
  • check patches against it
  • fix existing violations
• test the mocks
  • mocks should pass the same tests that the system they're mocking pass
    • especially for DB API
  • where this isn't practical, using introspection to compare function signatures gets 80% of the win

[tom.prince]

• We also have at least one place where we mock out part of twisted (we mock out out a request in the webhook tests
• the mock library has some support for creating a mock that matches the signature of a given object or class. This could probably get us the introspection bit.
• The scheduler tests have various implementations of addBuildset* that they use to fake out methods on the base class.

[dustin]

There's no problem with mocking third-party objects -- in fact, that's the "normal" use of mocks. Synchronizing the mock and the API there has not been problematic, and should be tested with integration tests.

Mock can do some introspection (and Foord announced that 0.8.0 has "autospec", which makes it even easier). However, I'd like to check the signatures directly, rather than relying on the tests to uncover any inconsistencies by getting an error from mock. If practical, we can do both!

[dustin]

This is coming along nicely in nine. DB methods are 100% covered: fakes and real methods both pass the same set of tests automatically, and their arg specs are compared automatically. The fake Data API update methods' arg specs are compared; their behavior is not significant enough to test. Most of the status mocks will go away along with the classes they're mocking. The scheduler tests now use the real addBuildsetFor.. methods from their base class, all of which call back to self.data.updates.addBuildset, which is mocked.

I'll leave this open as a reminder to check on progress.