Skip to content

Commit

Permalink
Remove var buildInfo (#2358)
Browse files Browse the repository at this point in the history
Problem: Running the tests in parallel would cause a data race, because
the tests were all trying to write the same variable.

Solution: Remove the var and type, that were only used for tests and use
the actual build info to test that the function works correctly.
  • Loading branch information
lucacome authored Aug 9, 2024
1 parent c732259 commit aedc18d
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 55 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ lint: check-golangci-lint ## Run golangci-lint against code
.PHONY: unit-test
unit-test: ## Run unit tests for the go code
# We have to run the tests in the cmd package using `go test` because of a bug with the CLI library cobra. See https://github.com/spf13/cobra/issues/2104.
go test ./cmd/... -race -shuffle=on -coverprofile=cmd-coverage.out -covermode=atomic
go test -buildvcs ./cmd/... -race -shuffle=on -coverprofile=cmd-coverage.out -covermode=atomic
go run github.com/onsi/ginkgo/v2/ginkgo --randomize-all --randomize-suites --race --keep-going --fail-on-pending --trace --covermode=atomic --coverprofile=coverage.out -r internal
go tool cover -html=coverage.out -o cover.html
go tool cover -html=cmd-coverage.out -o cmd-cover.html
Expand Down
6 changes: 1 addition & 5 deletions cmd/gateway/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -501,16 +501,12 @@ func parseFlags(flags *pflag.FlagSet) ([]string, []string) {
return flagKeys, flagValues
}

type buildInfoFunc func() (info *debug.BuildInfo, ok bool)

var buildInfo buildInfoFunc = debug.ReadBuildInfo

func getBuildInfo() (commitHash string, commitTime string, dirtyBuild string) {
commitHash = "unknown"
commitTime = "unknown"
dirtyBuild = "unknown"

info, ok := buildInfo()
info, ok := debug.ReadBuildInfo()
if !ok {
return
}
Expand Down
56 changes: 7 additions & 49 deletions cmd/gateway/commands_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package main

import (
"io"
"runtime/debug"
"testing"

. "github.com/onsi/gomega"
Expand Down Expand Up @@ -585,57 +584,16 @@ func TestParseFlags(t *testing.T) {
}

func TestGetBuildInfo(t *testing.T) {
t.Parallel()
g := NewWithT(t)
stubBuildInfo := func() (info *debug.BuildInfo, ok bool) {
return &debug.BuildInfo{
Settings: []debug.BuildSetting{
{Key: "vcs.revision", Value: "abc123"},
{Key: "vcs.time", Value: "2024-07-30T12:34:56Z"},
{Key: "vcs.modified", Value: "true"},
},
}, true
}

buildInfo = stubBuildInfo

commitHash, commitTime, dirtyBuild := getBuildInfo()

g.Expect(commitHash).To(Equal("abc123"))
g.Expect(commitTime).To(Equal("2024-07-30T12:34:56Z"))
g.Expect(dirtyBuild).To(Equal("true"))
}

func TestGetBuildInfoNoBuildInfo(t *testing.T) {
g := NewWithT(t)
stubBuildInfo := func() (info *debug.BuildInfo, ok bool) {
return nil, false
}

buildInfo = stubBuildInfo

commitHash, commitTime, dirtyBuild := getBuildInfo()

g.Expect(commitHash).To(Equal("unknown"))
g.Expect(commitTime).To(Equal("unknown"))
g.Expect(dirtyBuild).To(Equal("unknown"))
}

func TestGetBuildInfoMissingValue(t *testing.T) {
g := NewWithT(t)
stubBuildInfo := func() (info *debug.BuildInfo, ok bool) {
return &debug.BuildInfo{
Settings: []debug.BuildSetting{
{Key: "vcs.time", Value: "2024-07-30T12:34:56Z"},
{Key: "vcs.modified", Value: "true"},
},
}, true
}

buildInfo = stubBuildInfo

commitHash, commitTime, dirtyBuild := getBuildInfo()
g.Expect(commitHash).To(Not(BeEmpty()))
g.Expect(commitTime).To(Not(BeEmpty()))
g.Expect(dirtyBuild).To(Not(BeEmpty()))

g.Expect(commitHash).To(Equal("unknown"))
g.Expect(commitTime).To(Equal("2024-07-30T12:34:56Z"))
g.Expect(dirtyBuild).To(Equal("true"))
g.Expect(commitHash).To(Not(Equal("unknown")))
g.Expect(commitTime).To(Not(Equal("unknown")))
g.Expect(dirtyBuild).To(Not(Equal("unknown")))
}

0 comments on commit aedc18d

Please sign in to comment.