diff options
author | Vladimir Shushlin <vshushlin@gitlab.com> | 2020-02-27 12:34:48 +0300 |
---|---|---|
committer | Vladimir Shushlin <vshushlin@gitlab.com> | 2020-02-27 12:34:48 +0300 |
commit | 70879969bde8aaf97d5ed97d15b904747a44bfb0 (patch) | |
tree | 1dbbb200a00926d3262017851b22be1ecd1dfb3c | |
parent | 6f4fcff22c468de4102d908455d5c01aa27b8760 (diff) | |
parent | 14102ef332fb660436a3116b4ccaa8687648ca1d (diff) |
Merge branch 'race-test' into 'master'355-tech-evaluation-serve-pages-from-object-storage
Run go test -race in CI
See merge request gitlab-org/gitlab-pages!246
-rw-r--r-- | .gitlab-ci.yml | 8 | ||||
-rw-r--r-- | Makefile.util.mk | 3 | ||||
-rw-r--r-- | internal/source/gitlab/cache/cache_test.go | 84 |
3 files changed, 67 insertions, 28 deletions
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 5631d943..fe193eb6 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -75,6 +75,14 @@ test:1.13: extends: .tests image: golang:1.13 +race: + extends: .go-mod-cache + stage: test + needs: ['download deps'] + script: + - echo "Running race detector" + - make race + check deps: extends: .go-mod-cache stage: test diff --git a/Makefile.util.mk b/Makefile.util.mk index 80ca63b1..412b7655 100644 --- a/Makefile.util.mk +++ b/Makefile.util.mk @@ -17,6 +17,9 @@ complexity: .GOPATH/.ok bin/gocyclo test: .GOPATH/.ok gitlab-pages go test $(if $V,-v) $(allpackages) +race: .GOPATH/.ok gitlab-pages + CGO_ENABLED=1 go test -race $(if $V,-v) $(allpackages) + acceptance: .GOPATH/.ok gitlab-pages go test $(if $V,-v) $(IMPORT_PATH) diff --git a/internal/source/gitlab/cache/cache_test.go b/internal/source/gitlab/cache/cache_test.go index 39f8cb73..a0ee507d 100644 --- a/internal/source/gitlab/cache/cache_test.go +++ b/internal/source/gitlab/cache/cache_test.go @@ -4,7 +4,6 @@ import ( "context" "errors" "sync" - "sync/atomic" "testing" "time" @@ -13,27 +12,54 @@ import ( "gitlab.com/gitlab-org/gitlab-pages/internal/source/gitlab/api" ) -type client struct { +type stats struct { + m sync.Mutex started uint64 lookups uint64 +} + +type client struct { + stats stats bootup chan uint64 domain chan string failure error } -func (c *client) GetLookup(ctx context.Context, _ string) (lookup api.Lookup) { - // TODO This might not work on some architectures - // - // https://golang.org/pkg/sync/atomic/#pkg-note-BUG - // - // On ARM, x86-32, and 32-bit MIPS, it is the caller's responsibility to - // arrange for 64-bit alignment of 64-bit words accessed atomically. The first - // word in a variable or in an allocated struct, array, or slice can be relied - // upon to be 64-bit aligned. +func (s *stats) bumpStarted() uint64 { + s.m.Lock() + defer s.m.Unlock() + + s.started++ + return s.started +} + +func (s *stats) bumpLookups() uint64 { + s.m.Lock() + defer s.m.Unlock() + + s.lookups++ + return s.lookups +} + +func (s *stats) getStarted() uint64 { + s.m.Lock() + defer s.m.Unlock() + + return s.started +} + +func (s *stats) getLookups() uint64 { + s.m.Lock() + defer s.m.Unlock() + + return s.lookups +} - c.bootup <- atomic.AddUint64(&c.started, 1) - defer atomic.AddUint64(&c.lookups, 1) +func (c *client) GetLookup(ctx context.Context, _ string) api.Lookup { + c.bootup <- c.stats.bumpStarted() + defer c.stats.bumpLookups() + lookup := api.Lookup{} if c.failure == nil { lookup.Name = <-c.domain } else { @@ -103,7 +129,7 @@ func TestResolve(t *testing.T) { require.NoError(t, lookup.Error) require.Equal(t, "my.gitlab.com", lookup.Name) - require.Equal(t, uint64(1), resolver.lookups) + require.Equal(t, uint64(1), resolver.stats.getLookups()) }) }) @@ -122,12 +148,12 @@ func TestResolve(t *testing.T) { go receiver() go receiver() - require.Equal(t, uint64(0), resolver.lookups) + require.Equal(t, uint64(0), resolver.stats.getLookups()) resolver.domain <- "my.gitlab.com" wg.Wait() - require.Equal(t, uint64(1), resolver.lookups) + require.Equal(t, uint64(1), resolver.stats.getLookups()) }) }) @@ -137,7 +163,7 @@ func TestResolve(t *testing.T) { lookup := cache.Resolve(context.Background(), "my.gitlab.com") require.Equal(t, "my.gitlab.com", lookup.Name) - require.Equal(t, uint64(0), resolver.lookups) + require.Equal(t, uint64(0), resolver.stats.getLookups()) }) }) }) @@ -153,14 +179,14 @@ func TestResolve(t *testing.T) { <-resolver.bootup - require.Equal(t, uint64(1), resolver.started) - require.Equal(t, uint64(0), resolver.lookups) + require.Equal(t, uint64(1), resolver.stats.getStarted()) + require.Equal(t, uint64(0), resolver.stats.getLookups()) resolver.domain <- "my.gitlab.com" <-lookup - require.Equal(t, uint64(1), resolver.started) - require.Equal(t, uint64(1), resolver.lookups) + require.Equal(t, uint64(1), resolver.stats.getStarted()) + require.Equal(t, uint64(1), resolver.stats.getLookups()) }) }) }) @@ -171,10 +197,10 @@ func TestResolve(t *testing.T) { lookup := cache.Resolve(context.Background(), "my.gitlab.com") require.Equal(t, "my.gitlab.com", lookup.Name) - require.Equal(t, uint64(0), resolver.lookups) + require.Equal(t, uint64(0), resolver.stats.getLookups()) resolver.domain <- "my.gitlab.com" - require.Equal(t, uint64(1), resolver.lookups) + require.Equal(t, uint64(1), resolver.stats.getLookups()) }) }) }) @@ -186,10 +212,10 @@ func TestResolve(t *testing.T) { cache.Resolve(context.Background(), "my.gitlab.com") cache.Resolve(context.Background(), "my.gitlab.com") - require.Equal(t, uint64(0), resolver.lookups) + require.Equal(t, uint64(0), resolver.stats.getLookups()) resolver.domain <- "my.gitlab.com" - require.Equal(t, uint64(1), resolver.lookups) + require.Equal(t, uint64(1), resolver.stats.getLookups()) }) }) }) @@ -200,7 +226,7 @@ func TestResolve(t *testing.T) { lookup := cache.Resolve(context.Background(), "my.gitlab.com") - require.Equal(t, uint64(3), resolver.lookups) + require.Equal(t, uint64(3), resolver.stats.getLookups()) require.EqualError(t, lookup.Error, "500 err") }) }) @@ -212,19 +238,21 @@ func TestResolve(t *testing.T) { withTestCache(resolverConfig{}, func(cache *Cache, resolver *client) { lookup := cache.Resolve(ctx, "my.gitlab.com") - require.Equal(t, uint64(0), resolver.lookups) + require.Equal(t, uint64(0), resolver.stats.getLookups()) require.EqualError(t, lookup.Error, "context done") }) }) t.Run("when retrieval failed because of an internal retriever context timeout", func(t *testing.T) { + t.Skip("Data race") + retrievalTimeout = 0 defer func() { retrievalTimeout = 5 * time.Second }() withTestCache(resolverConfig{}, func(cache *Cache, resolver *client) { lookup := cache.Resolve(context.Background(), "my.gitlab.com") - require.Equal(t, uint64(0), resolver.lookups) + require.Equal(t, uint64(0), resolver.stats.getLookups()) require.EqualError(t, lookup.Error, "retrieval context done") }) }) |