From 79a25277c661f62b4f3f279f7908dcfbb7185a69 Mon Sep 17 00:00:00 2001 From: Evan Bradley <11745660+evan-bradley@users.noreply.github.com> Date: Thu, 30 Jan 2025 04:01:56 -0500 Subject: [PATCH] [chore] Make config validation tests more resilient (#37579) #### Description Fixes some errors introduced by https://github.com/open-telemetry/opentelemetry-collector/pull/12108. Overall this makes tests that check errors resulting from config validation more resilient by only checking that the error message contains the information relevant to the component. The tests changed in this PR make more precise assumptions about the error message that no longer hold up if `component.ValidateConfig` returns errors with different internal structures or messages. I've run this locally with the changes in https://github.com/open-telemetry/opentelemetry-collector/pull/12108, but I encountered some issues that prevented running tests locally in some modules, so not all errors may be fixed. I will play more with this tomorrow, but I also think it's safe to merge this as-is and fix the remaining errors during a `make update-otel` PR. --- connector/failoverconnector/config_test.go | 2 +- .../awscloudwatchlogsexporter/config_test.go | 2 +- exporter/awsemfexporter/config_test.go | 2 +- exporter/datasetexporter/factory_test.go | 6 ++++- exporter/elasticsearchexporter/config_test.go | 4 +-- .../observer/dockerobserver/config_test.go | 10 ++++---- extension/observer/k8sobserver/config_test.go | 2 +- .../resourcedetectionprocessor/config_test.go | 2 +- processor/transformprocessor/config_test.go | 25 +++++++++++-------- .../transformprocessor/testdata/config.yaml | 4 +-- receiver/apachesparkreceiver/config_test.go | 2 +- receiver/collectdreceiver/config_test.go | 2 +- receiver/k8sclusterreceiver/config_test.go | 3 ++- receiver/podmanreceiver/config_test.go | 2 +- receiver/saphanareceiver/config_test.go | 7 +++++- 15 files changed, 45 insertions(+), 30 deletions(-) diff --git a/connector/failoverconnector/config_test.go b/connector/failoverconnector/config_test.go index e39255763d8a..13c47851217b 100644 --- a/connector/failoverconnector/config_test.go +++ b/connector/failoverconnector/config_test.go @@ -109,7 +109,7 @@ func TestValidateConfig(t *testing.T) { require.NoError(t, err) require.NoError(t, sub.Unmarshal(cfg)) - assert.EqualError(t, component.ValidateConfig(cfg), tc.err.Error()) + assert.ErrorContains(t, component.ValidateConfig(cfg), tc.err.Error()) }) }) } diff --git a/exporter/awscloudwatchlogsexporter/config_test.go b/exporter/awscloudwatchlogsexporter/config_test.go index 749207e502f9..258ed8fadfcd 100644 --- a/exporter/awscloudwatchlogsexporter/config_test.go +++ b/exporter/awscloudwatchlogsexporter/config_test.go @@ -226,7 +226,7 @@ func TestValidateTags(t *testing.T) { }, } if tt.errorMessage != "" { - assert.EqualError(t, component.ValidateConfig(cfg), tt.errorMessage) + assert.ErrorContains(t, component.ValidateConfig(cfg), tt.errorMessage) return } assert.NoError(t, component.ValidateConfig(cfg)) diff --git a/exporter/awsemfexporter/config_test.go b/exporter/awsemfexporter/config_test.go index 0bd93d6dfc27..0be4cabab71d 100644 --- a/exporter/awsemfexporter/config_test.go +++ b/exporter/awsemfexporter/config_test.go @@ -252,7 +252,7 @@ func TestValidateTags(t *testing.T) { logger: zap.NewNop(), } if tt.errorMessage != "" { - assert.EqualError(t, component.ValidateConfig(cfg), tt.errorMessage) + assert.ErrorContains(t, component.ValidateConfig(cfg), tt.errorMessage) return } assert.NoError(t, component.ValidateConfig(cfg)) diff --git a/exporter/datasetexporter/factory_test.go b/exporter/datasetexporter/factory_test.go index dab12b484f66..c7e8dd95b0e1 100644 --- a/exporter/datasetexporter/factory_test.go +++ b/exporter/datasetexporter/factory_test.go @@ -161,7 +161,11 @@ func TestValidateConfigs(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(*testing.T) { err := component.ValidateConfig(tt.config) - assert.Equal(t, tt.expectedError, err) + if tt.expectedError != nil { + assert.ErrorContains(t, err, tt.expectedError.Error()) + } else { + assert.NoError(t, err) + } }) } } diff --git a/exporter/elasticsearchexporter/config_test.go b/exporter/elasticsearchexporter/config_test.go index 51d3955ebbd7..23dae5eab05d 100644 --- a/exporter/elasticsearchexporter/config_test.go +++ b/exporter/elasticsearchexporter/config_test.go @@ -427,7 +427,7 @@ func TestConfig_Validate(t *testing.T) { for name, tt := range tests { t.Run(name, func(t *testing.T) { - assert.EqualError(t, component.ValidateConfig(tt.config), tt.err) + assert.ErrorContains(t, component.ValidateConfig(tt.config), tt.err) }) } } @@ -443,7 +443,7 @@ func TestConfig_Validate_Environment(t *testing.T) { t.Setenv("ELASTICSEARCH_URL", "http://valid:9200, *:!") config := withDefaultConfig() err := component.ValidateConfig(config) - assert.EqualError(t, err, `invalid endpoint "*:!": parse "*:!": first path segment in URL cannot contain colon`) + assert.ErrorContains(t, err, `invalid endpoint "*:!": parse "*:!": first path segment in URL cannot contain colon`) }) } diff --git a/extension/observer/dockerobserver/config_test.go b/extension/observer/dockerobserver/config_test.go index 5a9d9d686571..9022e4139535 100644 --- a/extension/observer/dockerobserver/config_test.go +++ b/extension/observer/dockerobserver/config_test.go @@ -52,7 +52,7 @@ func TestLoadConfig(t *testing.T) { t.Run(tt.id.String(), func(t *testing.T) { cfg := loadConfig(t, tt.id) if tt.expectedError != "" { - assert.EqualError(t, component.ValidateConfig(cfg), tt.expectedError) + assert.ErrorContains(t, component.ValidateConfig(cfg), tt.expectedError) } else { assert.NoError(t, component.ValidateConfig(cfg)) } @@ -63,16 +63,16 @@ func TestLoadConfig(t *testing.T) { func TestValidateConfig(t *testing.T) { cfg := &Config{Config: docker.Config{DockerAPIVersion: "1.24", Timeout: 5 * time.Second}, CacheSyncInterval: 5 * time.Second} - assert.Equal(t, "endpoint must be specified", component.ValidateConfig(cfg).Error()) + assert.ErrorContains(t, component.ValidateConfig(cfg), "endpoint must be specified") cfg = &Config{Config: docker.Config{Endpoint: "someEndpoint", DockerAPIVersion: "1.23"}} - assert.Equal(t, `"api_version" 1.23 must be at least 1.24`, component.ValidateConfig(cfg).Error()) + assert.ErrorContains(t, component.ValidateConfig(cfg), `"api_version" 1.23 must be at least 1.24`) cfg = &Config{Config: docker.Config{Endpoint: "someEndpoint", DockerAPIVersion: version}} - assert.Equal(t, "timeout must be specified", component.ValidateConfig(cfg).Error()) + assert.ErrorContains(t, component.ValidateConfig(cfg), "timeout must be specified") cfg = &Config{Config: docker.Config{Endpoint: "someEndpoint", DockerAPIVersion: version, Timeout: 5 * time.Minute}} - assert.Equal(t, "cache_sync_interval must be specified", component.ValidateConfig(cfg).Error()) + assert.ErrorContains(t, component.ValidateConfig(cfg), "cache_sync_interval must be specified") cfg = &Config{Config: docker.Config{Endpoint: "someEndpoint", DockerAPIVersion: version, Timeout: 5 * time.Minute}, CacheSyncInterval: 5 * time.Minute} assert.NoError(t, component.ValidateConfig(cfg)) diff --git a/extension/observer/k8sobserver/config_test.go b/extension/observer/k8sobserver/config_test.go index 65148580d1dc..d8eebf7e8601 100644 --- a/extension/observer/k8sobserver/config_test.go +++ b/extension/observer/k8sobserver/config_test.go @@ -66,7 +66,7 @@ func TestLoadConfig(t *testing.T) { require.NoError(t, err) require.NoError(t, sub.Unmarshal(cfg)) if tt.expectedErr != "" { - assert.EqualError(t, component.ValidateConfig(cfg), tt.expectedErr) + assert.ErrorContains(t, component.ValidateConfig(cfg), tt.expectedErr) return } assert.NoError(t, component.ValidateConfig(cfg)) diff --git a/processor/resourcedetectionprocessor/config_test.go b/processor/resourcedetectionprocessor/config_test.go index 499474e81d17..6516711cbee0 100644 --- a/processor/resourcedetectionprocessor/config_test.go +++ b/processor/resourcedetectionprocessor/config_test.go @@ -148,7 +148,7 @@ func TestLoadConfig(t *testing.T) { require.NoError(t, sub.Unmarshal(cfg)) if tt.expected == nil { - assert.EqualError(t, component.ValidateConfig(cfg), tt.errorMessage) + assert.ErrorContains(t, component.ValidateConfig(cfg), tt.errorMessage) return } assert.NoError(t, component.ValidateConfig(cfg)) diff --git a/processor/transformprocessor/config_test.go b/processor/transformprocessor/config_test.go index e736707cd706..446998ba385b 100644 --- a/processor/transformprocessor/config_test.go +++ b/processor/transformprocessor/config_test.go @@ -4,13 +4,13 @@ package transformprocessor import ( + "errors" "path/filepath" "testing" "github.com/stretchr/testify/assert" "go.opentelemetry.io/collector/component" "go.opentelemetry.io/collector/confmap/confmaptest" - "go.uber.org/multierr" "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl" "github.com/open-telemetry/opentelemetry-collector-contrib/processor/transformprocessor/internal/common" @@ -23,7 +23,7 @@ func TestLoadConfig(t *testing.T) { tests := []struct { id component.ID expected component.Config - errorLen int + errors []error }{ { id: component.NewIDWithName(metadata.Type, ""), @@ -144,8 +144,12 @@ func TestLoadConfig(t *testing.T) { id: component.NewIDWithName(metadata.Type, "unknown_function_log"), }, { - id: component.NewIDWithName(metadata.Type, "bad_syntax_multi_signal"), - errorLen: 3, + id: component.NewIDWithName(metadata.Type, "bad_syntax_multi_signal"), + errors: []error{ + errors.New("unexpected token \"where\""), + errors.New("unexpected token \"attributes\""), + errors.New("unexpected token \"none\""), + }, }, { id: component.NewIDWithName(metadata.Type, "structured_configuration_with_path_context"), @@ -218,14 +222,15 @@ func TestLoadConfig(t *testing.T) { err = component.ValidateConfig(cfg) assert.Error(t, err) - if tt.errorLen > 0 { - assert.Len(t, multierr.Errors(err), tt.errorLen) + if len(tt.errors) > 0 { + for _, expectedErr := range tt.errors { + assert.ErrorContains(t, err, expectedErr.Error()) + } } - - return + } else { + assert.NoError(t, component.ValidateConfig(cfg)) + assert.Equal(t, tt.expected, cfg) } - assert.NoError(t, component.ValidateConfig(cfg)) - assert.Equal(t, tt.expected, cfg) }) } } diff --git a/processor/transformprocessor/testdata/config.yaml b/processor/transformprocessor/testdata/config.yaml index c327eeb3a2ef..d364fbe8623f 100644 --- a/processor/transformprocessor/testdata/config.yaml +++ b/processor/transformprocessor/testdata/config.yaml @@ -81,12 +81,12 @@ transform/bad_syntax_multi_signal: metric_statements: - context: datapoint statements: - - set(name, "bear" where attributes["http.path"] == "/animal" + - set(name, "bear" attributes["http.path"] == "/animal" - keep_keys(attributes, ["http.method", "http.path"]) log_statements: - context: log statements: - - set(body, "bear" where attributes["http.path"] == "/animal" + - set(body, "bear" none["http.path"] == "/animal" - keep_keys(attributes, ["http.method", "http.path"]) transform/unknown_function_log: diff --git a/receiver/apachesparkreceiver/config_test.go b/receiver/apachesparkreceiver/config_test.go index 9d6d8cebc89e..efdf43028a08 100644 --- a/receiver/apachesparkreceiver/config_test.go +++ b/receiver/apachesparkreceiver/config_test.go @@ -40,7 +40,7 @@ func TestValidate(t *testing.T) { t.Run(tc.desc, func(t *testing.T) { actualErr := tc.cfg.Validate() if tc.expectedErr != nil { - require.EqualError(t, actualErr, tc.expectedErr.Error()) + require.ErrorContains(t, actualErr, tc.expectedErr.Error()) } else { require.NoError(t, actualErr) } diff --git a/receiver/collectdreceiver/config_test.go b/receiver/collectdreceiver/config_test.go index f92bafcaac21..1da15f7ef077 100644 --- a/receiver/collectdreceiver/config_test.go +++ b/receiver/collectdreceiver/config_test.go @@ -59,7 +59,7 @@ func TestLoadConfig(t *testing.T) { if tt.wantErr == nil { assert.NoError(t, component.ValidateConfig(cfg)) } else { - assert.Equal(t, tt.wantErr, component.ValidateConfig(cfg)) + assert.ErrorContains(t, component.ValidateConfig(cfg), tt.wantErr.Error()) } assert.Equal(t, tt.expected, cfg) }) diff --git a/receiver/k8sclusterreceiver/config_test.go b/receiver/k8sclusterreceiver/config_test.go index 7cfbb46c6f0e..ca47838204d0 100644 --- a/receiver/k8sclusterreceiver/config_test.go +++ b/receiver/k8sclusterreceiver/config_test.go @@ -93,7 +93,8 @@ func TestInvalidConfig(t *testing.T) { Distribution: "wrong", CollectionInterval: 30 * time.Second, } + expectedErr := "\"wrong\" is not a supported distribution. Must be one of: \"openshift\", \"kubernetes\"" err = component.ValidateConfig(cfg) assert.Error(t, err) - assert.Equal(t, "\"wrong\" is not a supported distribution. Must be one of: \"openshift\", \"kubernetes\"", err.Error()) + assert.ErrorContains(t, err, expectedErr) } diff --git a/receiver/podmanreceiver/config_test.go b/receiver/podmanreceiver/config_test.go index 6b3bc0030d1d..4db4df62d5be 100644 --- a/receiver/podmanreceiver/config_test.go +++ b/receiver/podmanreceiver/config_test.go @@ -74,7 +74,7 @@ func TestLoadConfig(t *testing.T) { require.NoError(t, sub.Unmarshal(cfg)) if tt.expectedErrMsg != "" { - assert.EqualError(t, component.ValidateConfig(cfg), tt.expectedErrMsg) + assert.ErrorContains(t, component.ValidateConfig(cfg), tt.expectedErrMsg) return } diff --git a/receiver/saphanareceiver/config_test.go b/receiver/saphanareceiver/config_test.go index 104a6659b7d3..4b837c836c4c 100644 --- a/receiver/saphanareceiver/config_test.go +++ b/receiver/saphanareceiver/config_test.go @@ -66,7 +66,12 @@ func TestValidate(t *testing.T) { cfg := factory.CreateDefaultConfig().(*Config) tC.defaultConfigModifier(cfg) actual := component.ValidateConfig(cfg) - require.Equal(t, tC.expected, actual) + + if tC.expected != nil { + require.ErrorContains(t, actual, tC.expected.Error()) + } else { + require.NoError(t, actual) + } }) } }