Fix an assert that wasn't testing anything. #7624

Merged
merged 1 commit into from Jul 11, 2017

Conversation

Projects
None yet
4 participants
Owner

jameinel commented Jul 11, 2017

Description of change

In one of the recent refactorings, I pulled out a helper
function to test whether an entity was alive. I turned out
that the helper was broken in many ways.
It hard coded the 'key', but that didn't end up mattering because
it ended up doing:
c.Assert(alive, gc.Equals, alive)
which will always be true (it reused a parameter as a local variable).

The test happened to be correct anyway, this just fixes it to actually be
testing what we thought it was.

QA steps

Internal test change. You could change one of the values from "assertAlive" calls and see that it actually fails when it is supposed to instead of universally succeeding.

Documentation changes

No.

Bug reference

None (drive by while fixing a bug, but not directly related)

Fix an assert that wasn't testing anything.
In one of the recent refactorings, I pulled out a helper
function to test whether an entity was alive. I turned out
that the helper was broken in many ways.
It hard coded the 'key', but that didn't end up mattering because
it ended up doing:
   c.Assert(alive, gc.Equals, alive)
which will always be true (it reused a parameter as a local variable).

The test happened to be correct anyway, this just fixes it to actually be
testing what we thought it was.

axw approved these changes Jul 11, 2017

Owner

jameinel commented Jul 11, 2017

$$merge$$

Contributor

jujubot commented Jul 11, 2017

Status: merge request accepted. Url: http://juju-ci.vapour.ws:8080/job/github-merge-juju

@jujubot jujubot merged commit a323d3c into juju:2.2 Jul 11, 2017

1 check passed

github-check-merge-juju Ran tests against PR. Use !!.*!! to request another build. IE, !!build!!, !!retry!!
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment