From c83663cc2699bf06e61747a89d3e5faa79110783 Mon Sep 17 00:00:00 2001 From: Carlos Alexandro Becker Date: Tue, 16 Aug 2022 02:05:36 -0300 Subject: [PATCH] fix: eventual race condition in artifacts (#3310) * fix: eventual race condition in artifacts Signed-off-by: Carlos A Becker * fix: locks Signed-off-by: Carlos A Becker * fix: tests Signed-off-by: Carlos A Becker Signed-off-by: Carlos A Becker --- internal/artifact/artifact.go | 10 ++++++---- internal/pipe/aur/aur_test.go | 8 ++++---- internal/pipe/brew/brew_test.go | 8 ++++---- internal/pipe/docker/docker_test.go | 6 +++--- internal/pipe/krew/krew_test.go | 8 ++++---- 5 files changed, 21 insertions(+), 19 deletions(-) diff --git a/internal/artifact/artifact.go b/internal/artifact/artifact.go index dc9a036b990..06434e02c4c 100644 --- a/internal/artifact/artifact.go +++ b/internal/artifact/artifact.go @@ -268,13 +268,15 @@ func New() Artifacts { // List return the actual list of artifacts. func (artifacts Artifacts) List() []*Artifact { + artifacts.lock.Lock() + defer artifacts.lock.Unlock() return artifacts.items } // GroupByID groups the artifacts by their ID. func (artifacts Artifacts) GroupByID() map[string][]*Artifact { result := map[string][]*Artifact{} - for _, a := range artifacts.items { + for _, a := range artifacts.List() { id := a.ID() if id == "" { continue @@ -287,7 +289,7 @@ func (artifacts Artifacts) GroupByID() map[string][]*Artifact { // GroupByPlatform groups the artifacts by their platform. func (artifacts Artifacts) GroupByPlatform() map[string][]*Artifact { result := map[string][]*Artifact{} - for _, a := range artifacts.items { + for _, a := range artifacts.List() { plat := a.Goos + a.Goarch + a.Goarm + a.Gomips + a.Goamd64 result[plat] = append(result[plat], a) } @@ -485,7 +487,7 @@ func (artifacts *Artifacts) Filter(filter Filter) Artifacts { } result := New() - for _, a := range artifacts.items { + for _, a := range artifacts.List() { if filter(a) { result.items = append(result.items, a) } @@ -507,7 +509,7 @@ type VisitFn func(a *Artifact) error // Visit executes the given function for each artifact in the list. func (artifacts Artifacts) Visit(fn VisitFn) error { - for _, artifact := range artifacts.items { + for _, artifact := range artifacts.List() { if err := fn(artifact); err != nil { return err } diff --git a/internal/pipe/aur/aur_test.go b/internal/pipe/aur/aur_test.go index b11dc4446e8..9b108fddbba 100644 --- a/internal/pipe/aur/aur_test.go +++ b/internal/pipe/aur/aur_test.go @@ -465,13 +465,13 @@ func TestRunPipe(t *testing.T) { } func TestRunPipeNoBuilds(t *testing.T) { - ctx := &context.Context{ - TokenType: context.TokenTypeGitHub, - Config: config.Project{ + ctx := context.New( + config.Project{ ProjectName: "foo", AURs: []config.AUR{{}}, }, - } + ) + ctx.TokenType = context.TokenTypeGitHub client := client.NewMock() require.NoError(t, Pipe{}.Default(ctx)) require.Equal(t, ErrNoArchivesFound, runAll(ctx, client)) diff --git a/internal/pipe/brew/brew_test.go b/internal/pipe/brew/brew_test.go index 5ceb02948bd..1056ad14a12 100644 --- a/internal/pipe/brew/brew_test.go +++ b/internal/pipe/brew/brew_test.go @@ -747,9 +747,8 @@ func TestRunPipeForMultipleArmVersions(t *testing.T) { } func TestRunPipeNoBuilds(t *testing.T) { - ctx := &context.Context{ - TokenType: context.TokenTypeGitHub, - Config: config.Project{ + ctx := context.New( + config.Project{ Brews: []config.Homebrew{ { Tap: config.RepoRef{ @@ -759,7 +758,8 @@ func TestRunPipeNoBuilds(t *testing.T) { }, }, }, - } + ) + ctx.TokenType = context.TokenTypeGitHub client := client.NewMock() require.Equal(t, ErrNoArchivesFound, runAll(ctx, client)) require.False(t, client.CreatedFile) diff --git a/internal/pipe/docker/docker_test.go b/internal/pipe/docker/docker_test.go index 3858c74a692..9b7ee7497a8 100644 --- a/internal/pipe/docker/docker_test.go +++ b/internal/pipe/docker/docker_test.go @@ -1252,13 +1252,13 @@ func TestDefaultDockerfile(t *testing.T) { } func TestDraftRelease(t *testing.T) { - ctx := &context.Context{ - Config: config.Project{ + ctx := context.New( + config.Project{ Release: config.Release{ Draft: true, }, }, - } + ) require.False(t, pipe.IsSkip(Pipe{}.Publish(ctx))) } diff --git a/internal/pipe/krew/krew_test.go b/internal/pipe/krew/krew_test.go index a8b633a4cad..203fc9a8c95 100644 --- a/internal/pipe/krew/krew_test.go +++ b/internal/pipe/krew/krew_test.go @@ -731,9 +731,8 @@ func TestRunPipeForMultipleArmVersions(t *testing.T) { } func TestRunPipeNoBuilds(t *testing.T) { - ctx := &context.Context{ - TokenType: context.TokenTypeGitHub, - Config: config.Project{ + ctx := context.New( + config.Project{ Krews: []config.Krew{ { Name: manifestName(t), @@ -746,7 +745,8 @@ func TestRunPipeNoBuilds(t *testing.T) { }, }, }, - } + ) + ctx.TokenType = context.TokenTypeGitHub client := client.NewMock() require.Equal(t, ErrNoArchivesFound, runAll(ctx, client)) require.False(t, client.CreatedFile)