Skip to content

Commit

Permalink
[chore] Make config validation tests more resilient (#37579)
Browse files Browse the repository at this point in the history
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

Fixes some errors introduced by
open-telemetry/opentelemetry-collector#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
open-telemetry/opentelemetry-collector#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.
  • Loading branch information
evan-bradley authored Jan 30, 2025
1 parent 0c8e0fd commit 79a2527
Show file tree
Hide file tree
Showing 15 changed files with 45 additions and 30 deletions.
2 changes: 1 addition & 1 deletion connector/failoverconnector/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
})
})
}
Expand Down
2 changes: 1 addition & 1 deletion exporter/awscloudwatchlogsexporter/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
2 changes: 1 addition & 1 deletion exporter/awsemfexporter/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
6 changes: 5 additions & 1 deletion exporter/datasetexporter/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}
}
Expand Down
4 changes: 2 additions & 2 deletions exporter/elasticsearchexporter/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
}
Expand All @@ -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`)
})
}

Expand Down
10 changes: 5 additions & 5 deletions extension/observer/dockerobserver/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand All @@ -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))
Expand Down
2 changes: 1 addition & 1 deletion extension/observer/k8sobserver/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
2 changes: 1 addition & 1 deletion processor/resourcedetectionprocessor/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
25 changes: 15 additions & 10 deletions processor/transformprocessor/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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, ""),
Expand Down Expand Up @@ -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"),
Expand Down Expand Up @@ -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)
})
}
}
Expand Down
4 changes: 2 additions & 2 deletions processor/transformprocessor/testdata/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion receiver/apachesparkreceiver/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion receiver/collectdreceiver/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
Expand Down
3 changes: 2 additions & 1 deletion receiver/k8sclusterreceiver/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
2 changes: 1 addition & 1 deletion receiver/podmanreceiver/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
7 changes: 6 additions & 1 deletion receiver/saphanareceiver/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}
}
Expand Down

0 comments on commit 79a2527

Please sign in to comment.