Some tests in pserv_services.tests.test_dhcp_probe_service:TestDHCPProbeService never fail

[Graham Binns]

To reproduce:

1. Branch trunk
2. Edit src/provisioningserver/pserv_services/tests/test_dhcp_probe_service.py
3. Add a self.fail() call into one of the tests
4. Run bin/test.pserv pserv_services.tests.test_dhcp_probe_service:TestDHCPProbeService
5. See that for all except test_probe_is_initiated_in_new_thread and test_is_called_every_interval, the test that should fail doesn't.

This led to at least one bug being missed that should never have landed.

[Gavin Panella (allenap)]

A few of the tests are assuming the presence of inlineCallbacks, but they're not decorated as such. See http://paste.ubuntu.com/8180104/ for a fix.

[Graham Binns (gmb)]

Thanks Gavin. Is there any way we can protect against this kind of stuff in
future? ISTM that this is going to happen again for people who aren't
twisted-savvy.

[Gavin Panella (allenap)]

> Is there any way we can protect against this kind of stuff in
> future? ISTM that this is going to happen again for people who
> aren't twisted-savvy.

Heh, it happens to people who *are* Twisted savvy.

When I do it, I see it as a lesson in making sure I see my tests
fail before I see them pass.

There's probably something we can do to testtools to make it fail
tests that return generators.

[Gavin Panella (allenap)]

This works, as a replacement for AsynchronousDeferredRunTest:
----

import types
from testtools import deferredruntest
from twisted.internet import defer

class InvalidTest(Exception):
    """Signifies that the test is invalid; it's not a good test."""

class TwistedRunTest(deferredruntest.AsynchronousDeferredRunTest):

    def _check_for_generator(self, result):
        if isinstance(result, types.GeneratorType):
            raise InvalidTest(
                "Test returned a generator. Should it be "
                "decorated with inlineCallbacks?")
        else:
            return result

    def _run_user(self, function, *args):
        d = defer.maybeDeferred(function, *args)
        d.addCallback(self._check_for_generator)
        d.addErrback(self._got_user_failure)
        return d

[Gavin Panella (allenap)]

I should have added that we should see to get this upstream into testtools.

[Gavin Panella (allenap)]

I put TwistedTestCase into a branch. It caught three tests in TestDHCPProbeService and one elsewhere. The three failed when decorated with inlineCallbacks, the one other passes.

[Graham Binns (gmb)]

You lovely man. I'll review it when I get chance if someone else doesn't
first.

[Julian Edwards (julian-edwards)]

Which is why we should always write tests that fail before writing code.

[Graham Binns (gmb)]

On 30 August 2014 05:51, Julian Edwards <email address hidden> wrote:

> Which is why we should always write tests that fail before writing code.
>

Agreed.

[Gavin Panella (allenap)]

This has been fixed for, I think, quite a long time.