Code review comment for lp://qastaging/~doanac/utah/bug1179531

Revision history for this message
Javier Collado (javier.collado) wrote :

The code looks good.

A few comments:
- socket module is no longer needed in `utah/run.py`.
- A documentation string is missing in `tests/test_run.py` for the test case
  that has been added.
- Instead of `str(e)` you can use `e.message` in the assertion that checks the
  exception message.
- The exception raised by the mock object should be an `Exception` (not a
  `UTAHException`) to ensure that the change captures all exceptions.
- I would recommend to use a context manager to check the exception contents:

       with self.assertRaises(Exception) as cm:
            is_utah_done(machine, 1)

        e = cm.exception
        self.assertEquals(e.message, 'blah')
        self.assertTrue(e.retry)

« Back to merge proposal