overlord: fix TestEnsureLoopPrune not to be so racy #3140

Merged
merged 5 commits into from Apr 5, 2017

Conversation

Projects
None yet
3 participants
Contributor

zyga commented Apr 4, 2017

We are seeing many failures that involve the TestEnsureLoopPrune test.
This test is inherently racy as it uses a actual time to wait for
something to happen. In case the machine is loaded heavily and other
processes contend for resources it may not get scheduled in time for
this to happen

This branch contains (now) a more sophisticated approach suggested
and demonstrated by pedronis.

Signed-off-by: Zygmunt Krynicki zygmunt.krynicki@canonical.com

zyga added some commits Apr 4, 2017

overlord: increase prune test wait by x10
We are seeing many failures that involve the TestEnsureLoopPrune test.
This test is inherently racy as it uses a actual time to wait for
something to happen. In case the machine is loaded heavily and other
processes contend for resources it may not get scheduled in time for
this to happen.

While the fix is not perfect I hope to at least decrease the frequency
of the failures. We may investigate a more correct fix where timing
would no longer be a factor if this is deemed insufficient.

Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>
overlord: fix TestEnsureLoopPrune with a patch from pedronis
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>

@zyga zyga changed the title from overlord: increase prune test wait by x10 to overlord: fix TestEnsureLoopPrune not to be so racy Apr 5, 2017

mvo5 approved these changes Apr 5, 2017

Nice

overlord/overlord_test.go
@@ -370,11 +370,42 @@ func (ovs *overlordSuite) TestEnsureLoopPrune(c *C) {
chg1.AddTask(t1)
chg2 := st.NewChange("prune", "...")
chg2.SetStatus(state.DoneStatus)
+ t0 := chg2.SpawnTime()
@pedronis

pedronis Apr 5, 2017

Contributor

conceptually this needs to be ReadyTime(), my fault

@zyga

zyga Apr 5, 2017

Contributor

I'll fix this in a second.

overlord: use ReadyTime rather than SpawnTime (thanks to pedronis)
Signed-off-by: Zygmunt Krynicki <zygmunt.krynicki@canonical.com>

@pedronis pedronis merged commit 59472d2 into snapcore:master Apr 5, 2017

6 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
xenial-amd64 autopkgtest finished (success)
Details
xenial-i386 autopkgtest finished (success)
Details
xenial-ppc64el autopkgtest finished (success)
Details
yakkety-amd64 autopkgtest finished (success)
Details
zesty-amd64 autopkgtest finished (success)
Details

@zyga zyga deleted the zyga:fix-prune branch Aug 22, 2017

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