Fix variable unintentionally shared by multiple goroutines in test.

[arvados.git] / lib / dispatchcloud / container / queue_test.go

diff --git a/lib/dispatchcloud/container/queue_test.go b/lib/dispatchcloud/container/queue_test.go
index 91d65359e884a91955f47523a7d11836a52767df..bb8226118cb4dc91a36d5c5382fa5b3e8e5a1487 100644 (file)
--- a/lib/dispatchcloud/container/queue_test.go
+++ b/lib/dispatchcloud/container/queue_test.go
@@ -11,8 +11,8 @@ import (
        "testing"
        "time"
 
-       "git.curoverse.com/arvados.git/sdk/go/arvados"
-       "git.curoverse.com/arvados.git/sdk/go/arvadostest"
+       "git.arvados.org/arvados.git/sdk/go/arvados"
+       "git.arvados.org/arvados.git/sdk/go/arvadostest"
        "github.com/sirupsen/logrus"
        check "gopkg.in/check.v1"
 )
@@ -70,10 +70,12 @@ func (suite *IntegrationSuite) TestGetLockUnlockCancel(c *check.C) {
                c.Check(ctr.UUID, check.Equals, uuid)
 
                wg.Add(1)
-               go func() {
+               go func(uuid string) {
                        defer wg.Done()
                        err := cq.Unlock(uuid)
                        c.Check(err, check.NotNil)
+                       c.Check(err, check.ErrorMatches, ".*cannot unlock when Queued.*")
+
                        err = cq.Lock(uuid)
                        c.Check(err, check.IsNil)
                        ctr, ok := cq.Get(uuid)
@@ -81,6 +83,7 @@ func (suite *IntegrationSuite) TestGetLockUnlockCancel(c *check.C) {
                        c.Check(ctr.State, check.Equals, arvados.ContainerStateLocked)
                        err = cq.Lock(uuid)
                        c.Check(err, check.NotNil)
+
                        err = cq.Unlock(uuid)
                        c.Check(err, check.IsNil)
                        ctr, ok = cq.Get(uuid)
@@ -88,12 +91,17 @@ func (suite *IntegrationSuite) TestGetLockUnlockCancel(c *check.C) {
                        c.Check(ctr.State, check.Equals, arvados.ContainerStateQueued)
                        err = cq.Unlock(uuid)
                        c.Check(err, check.NotNil)
-               }()
+
+                       err = cq.Cancel(uuid)
+                       c.Check(err, check.IsNil)
+                       ctr, ok = cq.Get(uuid)
+                       c.Check(ok, check.Equals, true)
+                       c.Check(ctr.State, check.Equals, arvados.ContainerStateCancelled)
+                       err = cq.Lock(uuid)
+                       c.Check(err, check.NotNil)
+               }(uuid)
        }
        wg.Wait()
-
-       err = cq.Cancel(arvadostest.CompletedContainerUUID)
-       c.Check(err, check.ErrorMatches, `.*State cannot change from Complete to Cancelled.*`)
 }
 
 func (suite *IntegrationSuite) TestCancelIfNoInstanceType(c *check.C) {
@@ -104,16 +112,32 @@ func (suite *IntegrationSuite) TestCancelIfNoInstanceType(c *check.C) {
        client := arvados.NewClientFromEnv()
        cq := NewQueue(logger(), nil, errorTypeChooser, client)
 
+       ch := cq.Subscribe()
+       go func() {
+               defer cq.Unsubscribe(ch)
+               for range ch {
+                       // Container should never be added to
+                       // queue. Note that polling the queue this way
+                       // doesn't guarantee a bug (container being
+                       // incorrectly added to the queue) will cause
+                       // a test failure.
+                       _, ok := cq.Get(arvadostest.QueuedContainerUUID)
+                       if !c.Check(ok, check.Equals, false) {
+                               // Don't spam the log with more failures
+                               break
+                       }
+               }
+       }()
+
        var ctr arvados.Container
        err := client.RequestAndDecode(&ctr, "GET", "arvados/v1/containers/"+arvadostest.QueuedContainerUUID, nil, nil)
        c.Check(err, check.IsNil)
        c.Check(ctr.State, check.Equals, arvados.ContainerStateQueued)
 
-       cq.Update()
-
        // Wait for the cancel operation to take effect. Container
        // will have state=Cancelled or just disappear from the queue.
        suite.waitfor(c, time.Second, func() bool {
+               cq.Update()
                err := client.RequestAndDecode(&ctr, "GET", "arvados/v1/containers/"+arvadostest.QueuedContainerUUID, nil, nil)
                return err == nil && ctr.State == arvados.ContainerStateCancelled
        })