From 45ef2483a0dc27f047bd85387d685eb3664f7d78 Mon Sep 17 00:00:00 2001 From: Evan Bradley <11745660+evan-bradley@users.noreply.github.com> Date: Wed, 24 Apr 2024 16:38:53 -0400 Subject: [PATCH 1/7] [chore][cmd/builder] Test for unreleased versions --- cmd/builder/internal/builder/main_test.go | 30 +++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/cmd/builder/internal/builder/main_test.go b/cmd/builder/internal/builder/main_test.go index 6178c2b2f89..b891c28cb79 100644 --- a/cmd/builder/internal/builder/main_test.go +++ b/cmd/builder/internal/builder/main_test.go @@ -130,6 +130,36 @@ func TestVersioning(t *testing.T) { }, expectedErr: nil, }, + { + // Check that these tests won't break during a release, + // where the local version of these packages will not + // yet exist on the Go package repository. + description: "unreleased otel version", + cfgBuilder: func() Config { + cfg := NewDefaultConfig() + cfg.Verbose = true + cfg.Distribution.Go = "go" + cfg.Distribution.OtelColVersion = "1.9999.9999" + var err error + cfg.Exporters, err = parseModules([]Module{ + { + GoMod: "go.opentelemetry.io/collector/exporter/otlpexporter v1.9999.9999", + }, + }) + require.NoError(t, err) + cfg.Receivers, err = parseModules([]Module{ + { + GoMod: "go.opentelemetry.io/collector/receiver/otlpreceiver v1.9999.9999", + }, + }) + require.NoError(t, err) + + cfg.Replaces = append(cfg.Replaces, replaces...) + + return cfg + }, + expectedErr: nil, + }, { description: "old component version", cfgBuilder: func() Config { From e1a1ab85293eb58af0a55a40bd1ddd494514b5ad Mon Sep 17 00:00:00 2001 From: Evan Bradley Date: Fri, 26 Apr 2024 11:49:39 -0400 Subject: [PATCH 2/7] Make test more comprehensive --- cmd/builder/internal/builder/main_test.go | 223 +++++++++++++++------- 1 file changed, 151 insertions(+), 72 deletions(-) diff --git a/cmd/builder/internal/builder/main_test.go b/cmd/builder/internal/builder/main_test.go index b891c28cb79..1c798fc4985 100644 --- a/cmd/builder/internal/builder/main_test.go +++ b/cmd/builder/internal/builder/main_test.go @@ -7,13 +7,16 @@ import ( "fmt" "io" "os" + "path" "path/filepath" "runtime" + "strings" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.uber.org/zap" + "golang.org/x/mod/modfile" ) var ( @@ -31,6 +34,7 @@ require ( go.opentelemetry.io/collector/receiver v0.94.1 go.opentelemetry.io/collector v0.96.0 )`) + modulePrefix = "go.opentelemetry.io/collector" ) func newInitializedConfig(t *testing.T) Config { @@ -130,36 +134,6 @@ func TestVersioning(t *testing.T) { }, expectedErr: nil, }, - { - // Check that these tests won't break during a release, - // where the local version of these packages will not - // yet exist on the Go package repository. - description: "unreleased otel version", - cfgBuilder: func() Config { - cfg := NewDefaultConfig() - cfg.Verbose = true - cfg.Distribution.Go = "go" - cfg.Distribution.OtelColVersion = "1.9999.9999" - var err error - cfg.Exporters, err = parseModules([]Module{ - { - GoMod: "go.opentelemetry.io/collector/exporter/otlpexporter v1.9999.9999", - }, - }) - require.NoError(t, err) - cfg.Receivers, err = parseModules([]Module{ - { - GoMod: "go.opentelemetry.io/collector/receiver/otlpreceiver v1.9999.9999", - }, - }) - require.NoError(t, err) - - cfg.Replaces = append(cfg.Replaces, replaces...) - - return cfg - }, - expectedErr: nil, - }, { description: "old component version", cfgBuilder: func() Config { @@ -302,6 +276,101 @@ func TestGenerateAndCompile(t *testing.T) { } } +// An incomplete set of replace statements in these tests +// may cause them to fail during the release process, when +// the local version of modules in the release branch is +// not yet available on the Go package repository. +// Unless the replace statements route all modules to the +// local copy, `go get` will try to fetch the unreleased +// version remotely and some tests will fail. +func TestReplaceStatementsAreComplete(t *testing.T) { + var err error + dir := t.TempDir() + cfg := NewDefaultConfig() + cfg.Distribution.Go = "go" + cfg.Distribution.OutputPath = dir + // Use a deliberately nonexistent version to simulate an unreleased + // version of the package. Not strictly necessary since this test + // will catch gaps in the replace statements before a relase is in + // progres. + cfg.Distribution.OtelColVersion = "1.9999.9999" + cfg.Replaces = append(cfg.Replaces, generateReplaces()...) + cfg.Providers = &[]Module{} + cfg.Exporters, err = parseModules([]Module{ + { + GoMod: "go.opentelemetry.io/collector/exporter/debugexporter v1.9999.9999", + }, + { + GoMod: "go.opentelemetry.io/collector/exporter/nopexporter v1.9999.9999", + }, + { + GoMod: "go.opentelemetry.io/collector/exporter/otlpexporter v1.9999.9999", + }, + { + GoMod: "go.opentelemetry.io/collector/exporter/otlphttpexporter v1.9999.9999", + }, + }) + require.NoError(t, err) + cfg.Receivers, err = parseModules([]Module{ + { + GoMod: "go.opentelemetry.io/collector/receiver/nopreceiver v1.9999.9999", + }, + { + GoMod: "go.opentelemetry.io/collector/receiver/otlpreceiver v1.9999.9999", + }, + }) + require.NoError(t, err) + cfg.Extensions, err = parseModules([]Module{ + { + GoMod: "go.opentelemetry.io/collector/extension/zpagesextension v1.9999.9999", + }, + }) + require.NoError(t, err) + cfg.Processors, err = parseModules([]Module{ + { + GoMod: "go.opentelemetry.io/collector/processor/batchprocessor v1.9999.9999", + }, + { + GoMod: "go.opentelemetry.io/collector/processor/memorylimiterprocessor v1.9999.9999", + }, + }) + require.NoError(t, err) + + require.NoError(t, cfg.SetBackwardsCompatibility()) + require.NoError(t, cfg.Validate()) + require.NoError(t, cfg.ParseModules()) + err = GenerateAndCompile(cfg) + require.NoError(t, err) + + gomodname := path.Join(dir, "go.mod") + gomod, err := os.ReadFile(gomodname) + require.NoError(t, err) + + mod, err := modfile.Parse(path.Join(dir, "go.mod"), 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 + } + + _, ok := replaceMods[req.Mod.Path] + require.True(t, ok, fmt.Sprintf("Module missing from replace statements list: %s", req.Mod.Path)) + + replaceMods[req.Mod.Path] = true + } + + for k, v := range replaceMods { + require.True(t, v, fmt.Sprintf("Module not used: %s", k)) + } +} + func makeModule(dir string, fileContents []byte) error { // if the file does not exist, try to create it if _, err := os.Stat(dir); os.IsNotExist(err) { @@ -319,52 +388,62 @@ func makeModule(dir string, fileContents []byte) error { return nil } +func replaceModules() []string { + return []string{ + "", + "/component", + "/config/configauth", + "/config/configcompression", + "/config/configgrpc", + "/config/confighttp", + "/config/confignet", + "/config/configopaque", + "/config/configretry", + "/config/configtelemetry", + "/config/configtls", + "/config/internal", + "/confmap", + "/confmap/converter/expandconverter", + "/confmap/provider/envprovider", + "/confmap/provider/fileprovider", + "/confmap/provider/httpprovider", + "/confmap/provider/httpsprovider", + "/confmap/provider/yamlprovider", + "/consumer", + "/connector", + "/exporter", + "/exporter/debugexporter", + "/exporter/nopexporter", + "/exporter/otlpexporter", + "/exporter/otlphttpexporter", + "/extension", + "/extension/auth", + "/extension/zpagesextension", + "/featuregate", + "/processor", + "/processor/batchprocessor", + "/processor/memorylimiterprocessor", + "/receiver", + "/receiver/nopreceiver", + "/receiver/otlpreceiver", + "/otelcol", + "/pdata", + "/semconv", + "/service", + } +} + 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))))) - return []string{fmt.Sprintf("go.opentelemetry.io/collector => %s", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/component => %s/component", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/config/configauth => %s/config/configauth", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/config/configcompression => %s/config/configcompression", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/config/configgrpc => %s/config/configgrpc", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/config/confignet => %s/config/confignet", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/config/configopaque => %s/config/configopaque", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/config/configretry => %s/config/configretry", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/config/configtelemetry => %s/config/configtelemetry", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/config/configtls => %s/config/configtls", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/config/internal => %s/config/internal", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/confmap => %s/confmap", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/confmap/converter/expandconverter => %s/confmap/converter/expandconverter", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/confmap/provider/envprovider => %s/confmap/provider/envprovider", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/confmap/provider/fileprovider => %s/confmap/provider/fileprovider", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/confmap/provider/httpprovider => %s/confmap/provider/httpprovider", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/confmap/provider/httpsprovider => %s/confmap/provider/httpsprovider", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/confmap/provider/yamlprovider => %s/confmap/provider/yamlprovider", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/consumer => %s/consumer", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/connector => %s/connector", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/exporter => %s/exporter", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/exporter/debugexporter => %s/exporter/debugexporter", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/exporter/loggingexporter => %s/exporter/loggingexporter", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/exporter/nopexporter => %s/exporter/nopexporter", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/exporter/otlpexporter => %s/exporter/otlpexporter", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/exporter/otlphttpexporter => %s/exporter/otlphttpexporter", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/extension => %s/extension", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/extension/auth => %s/extension/auth", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/extension/ballastextension => %s/extension/ballastextension", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/extension/zpagesextension => %s/extension/zpagesextension", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/featuregate => %s/featuregate", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/processor => %s/processor", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/processor/batchprocessor => %s/processor/batchprocessor", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/processor/memorylimiterprocessor => %s/processor/memorylimiterprocessor", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/receiver => %s/receiver", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/receiver/nopreceiver => %s/receiver/nopreceiver", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/receiver/otlpreceiver => %s/receiver/otlpreceiver", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/otelcol => %s/otelcol", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/pdata => %s/pdata", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/pdata/testdata => %s/pdata/testdata", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/semconv => %s/semconv", workspaceDir), - fmt.Sprintf("go.opentelemetry.io/collector/service => %s/service", workspaceDir), + modules := replaceModules() + replaces := make([]string, len(modules)) + + for i, mod := range modules { + replaces[i] = fmt.Sprintf("%s%s => %s%s", modulePrefix, mod, workspaceDir, mod) } + + return replaces } From 49ecdf68e898de5cf363c17a4eb65fc0abcdca12 Mon Sep 17 00:00:00 2001 From: Evan Bradley Date: Fri, 26 Apr 2024 12:11:22 -0400 Subject: [PATCH 3/7] Fix comment typos --- cmd/builder/internal/builder/main_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/builder/internal/builder/main_test.go b/cmd/builder/internal/builder/main_test.go index 1c798fc4985..76d4163c768 100644 --- a/cmd/builder/internal/builder/main_test.go +++ b/cmd/builder/internal/builder/main_test.go @@ -291,8 +291,8 @@ func TestReplaceStatementsAreComplete(t *testing.T) { cfg.Distribution.OutputPath = dir // Use a deliberately nonexistent version to simulate an unreleased // version of the package. Not strictly necessary since this test - // will catch gaps in the replace statements before a relase is in - // progres. + // will catch gaps in the replace statements before a release is in + // progress. cfg.Distribution.OtelColVersion = "1.9999.9999" cfg.Replaces = append(cfg.Replaces, generateReplaces()...) cfg.Providers = &[]Module{} From 5485ac4275870d318fb58f46bdde54d4c2c17ca6 Mon Sep 17 00:00:00 2001 From: Evan Bradley <11745660+evan-bradley@users.noreply.github.com> Date: Mon, 29 Apr 2024 08:33:18 -0400 Subject: [PATCH 4/7] Update cmd/builder/internal/builder/main_test.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Juraci Paixão Kröhling --- cmd/builder/internal/builder/main_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/builder/internal/builder/main_test.go b/cmd/builder/internal/builder/main_test.go index 76d4163c768..67862049b92 100644 --- a/cmd/builder/internal/builder/main_test.go +++ b/cmd/builder/internal/builder/main_test.go @@ -367,7 +367,7 @@ func TestReplaceStatementsAreComplete(t *testing.T) { } for k, v := range replaceMods { - require.True(t, v, fmt.Sprintf("Module not used: %s", k)) + require.Truef(t, v, "Module not used: %s", k) } } From 61958ba38e04ffe91cbe4a259651acbcb986c27c Mon Sep 17 00:00:00 2001 From: Evan Bradley Date: Mon, 29 Apr 2024 09:06:15 -0400 Subject: [PATCH 5/7] Update comments, touch up test --- cmd/builder/internal/builder/main_test.go | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/cmd/builder/internal/builder/main_test.go b/cmd/builder/internal/builder/main_test.go index 67862049b92..ed9e5540894 100644 --- a/cmd/builder/internal/builder/main_test.go +++ b/cmd/builder/internal/builder/main_test.go @@ -276,6 +276,10 @@ func TestGenerateAndCompile(t *testing.T) { } } +// Test that the go.mod files that other tests in this file +// may generate have all their modules covered by our +// "replace" statements created in `generateReplaces`. +// // An incomplete set of replace statements in these tests // may cause them to fail during the release process, when // the local version of modules in the release branch is @@ -295,7 +299,9 @@ func TestReplaceStatementsAreComplete(t *testing.T) { // progress. cfg.Distribution.OtelColVersion = "1.9999.9999" cfg.Replaces = append(cfg.Replaces, generateReplaces()...) - cfg.Providers = &[]Module{} + // Configure all components that we want to use elsewhere in these tests. + // This ensures the resulting go.mod file has maximum coverage of modules + // that exist in the Core repository. cfg.Exporters, err = parseModules([]Module{ { GoMod: "go.opentelemetry.io/collector/exporter/debugexporter v1.9999.9999", @@ -361,13 +367,13 @@ func TestReplaceStatementsAreComplete(t *testing.T) { } _, ok := replaceMods[req.Mod.Path] - require.True(t, ok, fmt.Sprintf("Module missing from replace statements list: %s", req.Mod.Path)) + assert.Truef(t, ok, "Module missing from replace statements list: %s", req.Mod.Path) replaceMods[req.Mod.Path] = true } for k, v := range replaceMods { - require.Truef(t, v, "Module not used: %s", k) + assert.Truef(t, v, "Module not used: %s", k) } } From f6f61d02f7090ace0e5b6cbd9be9022d03785777 Mon Sep 17 00:00:00 2001 From: Evan Bradley Date: Mon, 29 Apr 2024 14:09:06 -0400 Subject: [PATCH 6/7] Fix lint --- cmd/builder/internal/builder/main_test.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/cmd/builder/internal/builder/main_test.go b/cmd/builder/internal/builder/main_test.go index ed9e5540894..942ae9e5f0b 100644 --- a/cmd/builder/internal/builder/main_test.go +++ b/cmd/builder/internal/builder/main_test.go @@ -348,11 +348,12 @@ func TestReplaceStatementsAreComplete(t *testing.T) { err = GenerateAndCompile(cfg) require.NoError(t, err) - gomodname := path.Join(dir, "go.mod") - gomod, err := os.ReadFile(gomodname) + 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) require.NoError(t, err) - mod, err := modfile.Parse(path.Join(dir, "go.mod"), gomod, nil) + mod, err := modfile.Parse(gomodpath, gomod, nil) require.NoError(t, err) replaceMods := map[string]bool{} From a25b690d9c922af8c070e320ba5befae57b0991a Mon Sep 17 00:00:00 2001 From: Evan Bradley <11745660+evan-bradley@users.noreply.github.com> Date: Thu, 2 May 2024 16:06:11 -0400 Subject: [PATCH 7/7] Update variables to address PR feedback --- cmd/builder/internal/builder/main_test.go | 102 +++++++++++----------- 1 file changed, 51 insertions(+), 51 deletions(-) diff --git a/cmd/builder/internal/builder/main_test.go b/cmd/builder/internal/builder/main_test.go index 942ae9e5f0b..e8dcaed099a 100644 --- a/cmd/builder/internal/builder/main_test.go +++ b/cmd/builder/internal/builder/main_test.go @@ -19,8 +19,8 @@ import ( "golang.org/x/mod/modfile" ) -var ( - goModTestFile = []byte(`// Copyright The OpenTelemetry Authors +const ( + goModTestFile = `// Copyright The OpenTelemetry Authors // SPDX-License-Identifier: Apache-2.0 module go.opentelemetry.io/collector/cmd/builder/internal/tester go 1.20 @@ -33,10 +33,55 @@ require ( go.opentelemetry.io/collector/processor v0.94.1 go.opentelemetry.io/collector/receiver v0.94.1 go.opentelemetry.io/collector v0.96.0 -)`) +)` modulePrefix = "go.opentelemetry.io/collector" ) +var ( + replaceModules = []string{ + "", + "/component", + "/config/configauth", + "/config/configcompression", + "/config/configgrpc", + "/config/confighttp", + "/config/confignet", + "/config/configopaque", + "/config/configretry", + "/config/configtelemetry", + "/config/configtls", + "/config/internal", + "/confmap", + "/confmap/converter/expandconverter", + "/confmap/provider/envprovider", + "/confmap/provider/fileprovider", + "/confmap/provider/httpprovider", + "/confmap/provider/httpsprovider", + "/confmap/provider/yamlprovider", + "/consumer", + "/connector", + "/exporter", + "/exporter/debugexporter", + "/exporter/nopexporter", + "/exporter/otlpexporter", + "/exporter/otlphttpexporter", + "/extension", + "/extension/auth", + "/extension/zpagesextension", + "/featuregate", + "/processor", + "/processor/batchprocessor", + "/processor/memorylimiterprocessor", + "/receiver", + "/receiver/nopreceiver", + "/receiver/otlpreceiver", + "/otelcol", + "/pdata", + "/semconv", + "/service", + } +) + func newInitializedConfig(t *testing.T) Config { cfg := NewDefaultConfig() // Validate and ParseModules will be called before the config is @@ -93,7 +138,7 @@ func TestVersioning(t *testing.T) { cfgBuilder: func() Config { cfg := NewDefaultConfig() tempDir := t.TempDir() - err := makeModule(tempDir, goModTestFile) + err := makeModule(tempDir, []byte(goModTestFile)) require.NoError(t, err) cfg.Distribution.OutputPath = tempDir cfg.SkipGenerate = true @@ -358,7 +403,7 @@ func TestReplaceStatementsAreComplete(t *testing.T) { replaceMods := map[string]bool{} - for _, suffix := range replaceModules() { + for _, suffix := range replaceModules { replaceMods[modulePrefix+suffix] = false } @@ -395,57 +440,12 @@ func makeModule(dir string, fileContents []byte) error { return nil } -func replaceModules() []string { - return []string{ - "", - "/component", - "/config/configauth", - "/config/configcompression", - "/config/configgrpc", - "/config/confighttp", - "/config/confignet", - "/config/configopaque", - "/config/configretry", - "/config/configtelemetry", - "/config/configtls", - "/config/internal", - "/confmap", - "/confmap/converter/expandconverter", - "/confmap/provider/envprovider", - "/confmap/provider/fileprovider", - "/confmap/provider/httpprovider", - "/confmap/provider/httpsprovider", - "/confmap/provider/yamlprovider", - "/consumer", - "/connector", - "/exporter", - "/exporter/debugexporter", - "/exporter/nopexporter", - "/exporter/otlpexporter", - "/exporter/otlphttpexporter", - "/extension", - "/extension/auth", - "/extension/zpagesextension", - "/featuregate", - "/processor", - "/processor/batchprocessor", - "/processor/memorylimiterprocessor", - "/receiver", - "/receiver/nopreceiver", - "/receiver/otlpreceiver", - "/otelcol", - "/pdata", - "/semconv", - "/service", - } -} - 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))))) - modules := replaceModules() + modules := replaceModules replaces := make([]string, len(modules)) for i, mod := range modules {