Code review comment for lp:~jk0/nova/diagnostics-per-instance

Revision history for this message
Josh Kearney (jk0) wrote :

> I don't understand the changes to test_run_terminate. Why change
>
> self.assertEqual(len(instances), 1)
>
> to
>
> self.assertNotEqual(instance_count, 0)
>
> ? The former is more specific and is a tighter test.

I had to change that because the test was failing when more than one instance was returned. By doing it this way, as long as the instance count is >0, we know there is one or more running. The restriction comes from Python 2.6's unittest. There just isn't much to work with - it's either Equal or Not Equal.

> Also, the assertion should probably compare against a constant value that you
> are expecting, 0 or 1 in this case, rather than len(instances) + 1.
>
> You may have a good reason for these changes and I'm just not aware of it. If
> so, let me know. :)

The thing is we don't know that it will *always* be 1 or 0. We could start off with 15 running instances. Terminate one. Assert that the original instance_count is equal to the current instance count + 1. If we were running Python 2.7's unittest, there is assertGreater, assertGreaterEqual, etc, but unfortunately those aren't available in 2.6.

« Back to merge proposal