From 4bd4e2b970ba670e9703e86449f12ccca6149831 Mon Sep 17 00:00:00 2001 From: Evan Bradley <11745660+evan-bradley@users.noreply.github.com> Date: Tue, 21 May 2024 14:33:43 -0400 Subject: [PATCH] [chore][cmd/builder] Improve missing replace statements test (#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 #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 #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 ``` --- cmd/builder/internal/builder/main_test.go | 46 +++++++++++++---------- 1 file changed, 27 insertions(+), 19 deletions(-) diff --git a/cmd/builder/internal/builder/main_test.go b/cmd/builder/internal/builder/main_test.go index 79ed8ab2751..6c862c70721 100644 --- a/cmd/builder/internal/builder/main_test.go +++ b/cmd/builder/internal/builder/main_test.go @@ -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() @@ -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) @@ -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 @@ -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 { @@ -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)) @@ -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))))) +}