Code review comment for lp:~julian-edwards/maas/stop-dhcp-server-bug-1283114

Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

> > I see no test to establish that NoReceivers actually works,
>
> What?! I added one, it's in the diff?:
>
> +class TestNoReceivers(MAASServerTestCase):
>
> If you are implying that I should test that no signal receivers are
> actually called - then you'd be asking for a test that tests Django
> signals functionality.

You wrote tests that match the code in NoReceivers, but none that verify that the thing actually works as intended. Your initial implementation would have passed that kind of test, but didn't actually work.

> > nor that it only affects the signals you pass it.
>
> That would be impossible unless there was a registry of all known
> signals, which AFAIK there isn't. They can be created at any time.

No, it wouldn't — all you'd need was _an example_ of a signal that would otherwise have been affected. Remember that your initial implementation operated on a global signals list.

« Back to merge proposal