Code review comment for lp:~allenap/maas/tftp-rebind-on-netif-change

Revision history for this message
Gavin Panella (allenap) wrote :

> Looks good although I'm not familiar enough with twisted to understand 100% of
> it.  At always when making important changes to a core piece of the
> infrastructure, I recommend building a package from this branch and testing it
> before landing that whole lot.

This is not something I'm in the habit of doing, but I ought to.

>
> [0]
>
> 188     +class TestTFTPService(MAASTestCase):
> 189     +
> 190     +    def test_tftp_service(self):
>
> This test is a bit painful to digest: it checks a ton of stuff.  Could you
> think of a way to split it into more focused tests?

Breaking it up would mean a lot of repetition, getting it to the right
state. The liberal use of matchers will, I hope, make any failures
easier to understand.

Thanks for the re-review!

« Back to merge proposal