Code review comment for lp:~viswesn/juju-ci-tools/ensure_provider_cleanup

Revision history for this message
Aaron Bentley (abentley) wrote :

attempt_terminate_instances does not work on AWS.

The tests mock out the direct callees, even though there are alternatives. Once consequence is that you didn't notice that attempt_terminate_instances does not work on AWS.

The tests use assertEqual(0, mock.call_count) where they could use mock.assert_called_once_with.

Also, I don't think it makes sense to reimplement set.issubset as contains_only_known_instances.

There are some formatting suggestions, too. See below.

Also, please merge trunk more frequently. I got merge conflicts with one algorithm.

review: Needs Fixing

« Back to merge proposal