Code review comment for lp:~rconradharris/nova/server_progress

Revision history for this message
Rick Harris (rconradharris) wrote :

> I do not see where you are testing the new server progress feature you have added. For example, I
> see no new assertions in the tests. Is it because this is difficult to test and should be tested at
> a higher level?

Automated functional testing for this didn't feel like it would be worth the additional effort. This was a judgement call, and feel free to disagree here, but it seemed like any tests for this would be slow, difficult to write, and not very useful.

The reason I think it would not be very useful is that, unlike some areas of the code where we have complex interactions, this is very straightforward. That's not to say this code can't break, it's just that the chances of that happening inadvertently are probably small relative to the pain of testing this.

> Also, it looks like you have only added the progress feature for xen. Is this correct?

Yeah adding it to XenServer for now since that's the hypervisor I'm most familiar with. At a high level the steps should be similar, but each virt driver may have a slightly different process for builds and migrations--and I'm not familiar enough with libvirt, etc to add those myself, without considerable additional research.

« Back to merge proposal