Skip to content

Commit

Permalink
[chore][cmd/builder] Improve missing replace statements test (open-te…
Browse files Browse the repository at this point in the history
…lemetry#10196)

Updates `TestReplaceStatementsAreComplete` to check the modules used in
replace statements to ensure that their dependencies also have replace
statements. This will catch the error that happened in open-telemetry#10188 before a
release is started.

The one caveat here is that the test may need to be run multiple times
if there are modules deep in the dependency tree that haven't been added
to the list of replace statement modules. In essence, the user has to do
a BFS walk of the dependency tree themselves by running the tests until
all missing modules are caught. We could automate this process with
additional code to report all missing modules at once regardless of
depth, but I figure it's not worth the extra complexity in the test for
such a small gain.

#### Testing

I tested this on the open-telemetry#10188 branch by removing the `pdata/testdata`
module from the replace statements list and seeing that the failure is
easier to understand:

```
--- FAIL: TestReplaceStatementsAreComplete (0.60s)
        	Error:      	Should be true
        	Test:       	TestReplaceStatementsAreComplete
        	Messages:   	Module missing from replace statements list: go.opentelemetry.io/collector/pdata/testdata
```
  • Loading branch information
evan-bradley authored and steves-canva committed Jun 13, 2024
1 parent fad3864 commit 4bd4e2b
Showing 1 changed file with 27 additions and 19 deletions.
46 changes: 27 additions & 19 deletions cmd/builder/internal/builder/main_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,17 @@ func TestGenerateAndCompile(t *testing.T) {
// local copy, `go get` will try to fetch the unreleased
// version remotely and some tests will fail.
func TestReplaceStatementsAreComplete(t *testing.T) {
workspaceDir := getWorkspaceDir()
replaceMods := map[string]bool{}

for _, suffix := range replaceModules {
replaceMods[modulePrefix+suffix] = false
}

for _, mod := range replaceModules {
verifyGoMod(t, workspaceDir+mod, replaceMods)
}

var err error
dir := t.TempDir()
cfg := NewDefaultConfig()
Expand Down Expand Up @@ -401,6 +412,14 @@ func TestReplaceStatementsAreComplete(t *testing.T) {
err = GenerateAndCompile(cfg)
require.NoError(t, err)

verifyGoMod(t, dir, replaceMods)

for k, v := range replaceMods {
assert.Truef(t, v, "Module not used: %s", k)
}
}

func verifyGoMod(t *testing.T, dir string, replaceMods map[string]bool) {
gomodpath := path.Join(dir, "go.mod")
// #nosec G304 We control this path and generate the file inside, so we can assume it is safe.
gomod, err := os.ReadFile(gomodpath)
Expand All @@ -409,12 +428,6 @@ func TestReplaceStatementsAreComplete(t *testing.T) {
mod, err := modfile.Parse(gomodpath, gomod, nil)
require.NoError(t, err)

replaceMods := map[string]bool{}

for _, suffix := range replaceModules {
replaceMods[modulePrefix+suffix] = false
}

for _, req := range mod.Require {
if !strings.HasPrefix(req.Mod.Path, modulePrefix) {
continue
Expand All @@ -425,15 +438,6 @@ func TestReplaceStatementsAreComplete(t *testing.T) {

replaceMods[req.Mod.Path] = true
}

// those are modules that should be part of the replaces, but are not components
for _, unused := range []string{"/pdata/testdata"} {
replaceMods[modulePrefix+unused] = true
}

for k, v := range replaceMods {
assert.Truef(t, v, "Module not used: %s", k)
}
}

func makeModule(dir string, fileContents []byte) error {
Expand All @@ -454,10 +458,7 @@ func makeModule(dir string, fileContents []byte) error {
}

func generateReplaces() []string {
// This test is dependent on the current file structure.
// The goal is find the root of the repo so we can replace the root module.
_, thisFile, _, _ := runtime.Caller(0)
workspaceDir := filepath.Dir(filepath.Dir(filepath.Dir(filepath.Dir(filepath.Dir(thisFile)))))
workspaceDir := getWorkspaceDir()
modules := replaceModules
replaces := make([]string, len(modules))

Expand All @@ -467,3 +468,10 @@ func generateReplaces() []string {

return replaces
}

func getWorkspaceDir() string {
// This is dependent on the current file structure.
// The goal is find the root of the repo so we can replace the root module.
_, thisFile, _, _ := runtime.Caller(0)
return filepath.Dir(filepath.Dir(filepath.Dir(filepath.Dir(filepath.Dir(thisFile)))))
}

0 comments on commit 4bd4e2b

Please sign in to comment.