/juju

Fix intermittent test bug #1552589. #4614

Merged
merged 1 commit into from Mar 7, 2016

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@jameinel @jujubot
@jameinel
Owner

jameinel commented Mar 3, 2016

It was possible to change the configuration before the provisioner task
was started, and thus there was never any change to observe.
Now we always observer the initial config. It means the test itself
is slightly weaker, because sometimes we never observer the change,
but at least the test doesn't accidentally fail.
A better fix would be to wait to change the config until you know
that the config has already been seen. However, we can't do that
via the Observer interface, because there is a race that we
could have already started the Task before we get a chance to
set the observer.

(Review request: http://reviews.vapour.ws/r/4054/)
@jameinel
Fix intermittent test bug #1552589. 
It was possible to change the configuration before the provisioner task
was started, and thus there was never any change to observe.
Now we always observer the initial config. It means the test itself
is slightly weaker, because sometimes we never observer the *change*,
but at least the test doesn't accidentally fail.
A better fix would be to wait to change the config until you know
that the config has already been seen. However, we can't do that
via the Observer interface, because there is a race that we
could have already started the Task before we get a chance to
set the observer.
6c74ec1
@jameinel
Owner

jameinel commented Mar 3, 2016

$$merge$$
@jujubot
Contributor

jujubot commented Mar 3, 2016

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

jujubot commented Mar 3, 2016

Build failed: Does not match ['fixes-1551276']
build url: http://juju-ci.vapour.ws:8080/job/github-merge-juju/6664
@jameinel
Owner

jameinel commented Mar 7, 2016

$$merge$$ JFDI
as a test fix, I figured this would only help the release process go smooth
@jujubot
Contributor

jujubot commented Mar 7, 2016

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

jujubot added a commit that referenced this pull request Mar 7, 2016

@jujubot
Merge pull request #4614 from jameinel/fix-lp1552589-intermittent-Tes… 
…tProvisionerObservesMachineJobs

Fix intermittent test bug #1552589.

It was possible to change the configuration before the provisioner task
was started, and thus there was never any change to observe.
Now we always observer the initial config. It means the test itself
is slightly weaker, because sometimes we never observer the *change*,
but at least the test doesn't accidentally fail.
A better fix would be to wait to change the config until you know
that the config has already been seen. However, we can't do that
via the Observer interface, because there is a race that we
could have already started the Task before we get a chance to
set the observer.

(Review request: http://reviews.vapour.ws/r/4054/)
c17aa73

@jujubot jujubot merged commit c17aa73 into juju:master Mar 7, 2016

anastasiamac added a commit to anastasiamac/juju that referenced this pull request Mar 15, 2016

@jujubot @anastasiamac
Merge pull request #4614 from jameinel/fix-lp1552589-intermittent-Tes… 
…tProvisionerObservesMachineJobs

Fix intermittent test bug #1552589.

It was possible to change the configuration before the provisioner task
was started, and thus there was never any change to observe.
Now we always observer the initial config. It means the test itself
is slightly weaker, because sometimes we never observer the *change*,
but at least the test doesn't accidentally fail.
A better fix would be to wait to change the config until you know
that the config has already been seen. However, we can't do that
via the Observer interface, because there is a race that we
could have already started the Task before we get a chance to
set the observer.

(Review request: http://reviews.vapour.ws/r/4054/)
# Conflicts:
#	worker/provisioner/provisioner.go
abf46af

@anastasiamac anastasiamac referenced this pull request Mar 15, 2016

Merged

1.25 backport of jameinel/fix-lp1552589 #4724

jujubot added a commit that referenced this pull request Mar 22, 2016

@jujubot
Merge pull request #4724 from anastasiamac/jam-patch-1552589 
1.25 backport of jameinel/fix-lp1552589

This is a word-for-word backport except for terminology change environment (1.25) to model (master).
 
Merge pull request #4614 from jameinel/fix-lp1552589-intermittent-TestProvisionerObservesMachineJobs

Fix intermittent test bug #1552589.

It was possible to change the configuration before the provisioner task
was started, and thus there was never any change to observe.
Now we always observer the initial config. It means the test itself
is slightly weaker, because sometimes we never observer the *change*,
but at least the test doesn't accidentally fail.
A better fix would be to wait to change the config until you know
that the config has already been seen. However, we can't do that
via the Observer interface, because there is a race that we
could have already started the Task before we get a chance to
set the observer.

(Review request: http://reviews.vapour.ws/r/4054/)
# Conflicts:
#	worker/provisioner/provisioner.go

(Review request: http://reviews.vapour.ws/r/4162/)
be39a0a

@jameinel jameinel deleted the jameinel:fix-lp1552589-intermittent-TestProvisionerObservesMachineJobs branch Dec 13, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment