Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[chore] Make additional config validation tests more resilient #37594

Merged
merged 2 commits into from
Jan 31, 2025
Merged

[chore] Make additional config validation tests more resilient #37594

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
447c00d
[chore] Make additional config validation tests more resilient
evan-bradley Jan 30, 2025
2942f8b
Undo local testing change
evan-bradley Jan 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions receiver/dockerstatsreceiver/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ func TestValidateErrors(t *testing.T) {
cfg := &Config{ControllerConfig: scraperhelper.NewDefaultControllerConfig(), Config: docker.Config{
DockerAPIVersion: "1.25",
}}
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{
Expand All @@ -110,7 +110,7 @@ func TestValidateErrors(t *testing.T) {
},
ControllerConfig: scraperhelper.ControllerConfig{CollectionInterval: 1 * time.Second},
}
assert.Equal(t, `"api_version" 1.21 must be at least 1.25`, component.ValidateConfig(cfg).Error())
assert.ErrorContains(t, component.ValidateConfig(cfg), `"api_version" 1.21 must be at least 1.25`)

cfg = &Config{
Config: docker.Config{
Expand All @@ -119,7 +119,7 @@ func TestValidateErrors(t *testing.T) {
},
ControllerConfig: scraperhelper.ControllerConfig{},
}
assert.Equal(t, `"collection_interval": requires positive value`, component.ValidateConfig(cfg).Error())
assert.ErrorContains(t, component.ValidateConfig(cfg), `"collection_interval": requires positive value`)
}

func TestApiVersionCustomError(t *testing.T) {
Expand Down
1 change: 1 addition & 0 deletions receiver/hostmetricsreceiver/internal/scraper/filesystemscraper/filesystem_scraper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
)

func TestScrape(t *testing.T) {
t.Skip("")
type testCase struct {
name string
config Config
Expand Down
2 changes: 1 addition & 1 deletion receiver/k8sclusterreceiver/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func TestInvalidConfig(t *testing.T) {
}
err := component.ValidateConfig(cfg)
assert.Error(t, err)
assert.Equal(t, "invalid authType for kubernetes: ", err.Error())
assert.ErrorContains(t, err, "invalid authType for kubernetes: ")

// Wrong distro
cfg = &Config{
Expand Down
20 changes: 11 additions & 9 deletions receiver/podmanreceiver/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ func TestLoadConfig(t *testing.T) {
require.NoError(t, err)

tests := []struct {
id component.ID
expected component.Config
expectedErrMsg string
id component.ID
expected component.Config
expectedErrMsgs []string
}{
{
id: component.NewIDWithName(metadata.Type, ""),
Expand Down Expand Up @@ -55,12 +55,12 @@ func TestLoadConfig(t *testing.T) {
},
},
{
id: component.NewIDWithName(metadata.Type, "empty_endpoint"),
expectedErrMsg: "config.Endpoint must be specified",
id: component.NewIDWithName(metadata.Type, "empty_endpoint"),
expectedErrMsgs: []string{"config.Endpoint must be specified"},
},
{
id: component.NewIDWithName(metadata.Type, "invalid_collection_interval"),
expectedErrMsg: `config.CollectionInterval must be specified; "collection_interval": requires positive value`,
id: component.NewIDWithName(metadata.Type, "invalid_collection_interval"),
expectedErrMsgs: []string{`config.CollectionInterval must be specified`, `"collection_interval": requires positive value`},
},
}

Expand All @@ -73,8 +73,10 @@ func TestLoadConfig(t *testing.T) {
require.NoError(t, err)
require.NoError(t, sub.Unmarshal(cfg))

if tt.expectedErrMsg != "" {
assert.ErrorContains(t, component.ValidateConfig(cfg), tt.expectedErrMsg)
if len(tt.expectedErrMsgs) > 0 {
for _, msg := range tt.expectedErrMsgs {
assert.ErrorContains(t, component.ValidateConfig(cfg), msg)
}
return
}

Expand Down
33 changes: 18 additions & 15 deletions receiver/postgresqlreceiver/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"go.opentelemetry.io/collector/config/confignet"
"go.opentelemetry.io/collector/config/configtls"
"go.opentelemetry.io/collector/confmap/confmaptest"
"go.uber.org/multierr"

"github.com/open-telemetry/opentelemetry-collector-contrib/receiver/postgresqlreceiver/internal/metadata"
)
Expand All @@ -24,33 +23,33 @@ func TestValidate(t *testing.T) {
testCases := []struct {
desc string
defaultConfigModifier func(cfg *Config)
expected error
expected []error
}{
{
desc: "missing username and password",
defaultConfigModifier: func(*Config) {},
expected: multierr.Combine(
expected: []error{
errors.New(ErrNoUsername),
errors.New(ErrNoPassword),
),
},
},
{
desc: "missing password",
defaultConfigModifier: func(cfg *Config) {
cfg.Username = "otel"
},
expected: multierr.Combine(
expected: []error{
errors.New(ErrNoPassword),
),
},
},
{
desc: "missing username",
defaultConfigModifier: func(cfg *Config) {
cfg.Password = "otel"
},
expected: multierr.Combine(
expected: []error{
errors.New(ErrNoUsername),
),
},
},
{
desc: "bad endpoint",
Expand All @@ -59,9 +58,9 @@ func TestValidate(t *testing.T) {
cfg.Password = "otel"
cfg.Endpoint = "open-telemetry"
},
expected: multierr.Combine(
expected: []error{
errors.New(ErrHostPort),
),
},
},
{
desc: "bad transport",
Expand All @@ -70,9 +69,9 @@ func TestValidate(t *testing.T) {
cfg.Password = "otel"
cfg.Transport = "udp"
},
expected: multierr.Combine(
expected: []error{
errors.New(ErrTransportsSupported),
),
},
},
{
desc: "unsupported SSL params",
Expand All @@ -83,11 +82,11 @@ func TestValidate(t *testing.T) {
cfg.MinVersion = "1.0"
cfg.MaxVersion = "1.0"
},
expected: multierr.Combine(
expected: []error{
fmt.Errorf(ErrNotSupported, "ServerName"),
fmt.Errorf(ErrNotSupported, "MaxVersion"),
fmt.Errorf(ErrNotSupported, "MinVersion"),
),
},
},
{
desc: "no error",
Expand All @@ -104,7 +103,11 @@ func TestValidate(t *testing.T) {
cfg := factory.CreateDefaultConfig().(*Config)
tC.defaultConfigModifier(cfg)
actual := component.ValidateConfig(cfg)
require.Equal(t, tC.expected, actual)
if len(tC.expected) > 0 {
for _, err := range tC.expected {
require.ErrorContains(t, actual, err.Error())
}
}
})
}
}
Expand Down
2 changes: 1 addition & 1 deletion receiver/prometheusreceiver/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ func TestRejectUnsupportedPrometheusFeatures(t *testing.T) {
rule_files`

gotErrMsg := strings.ReplaceAll(err.Error(), "\t", strings.Repeat(" ", 8))
require.Equal(t, wantErrMsg, gotErrMsg)
require.Contains(t, gotErrMsg, wantErrMsg)
}

func TestNonExistentAuthCredentialsFile(t *testing.T) {
Expand Down
12 changes: 6 additions & 6 deletions receiver/solacereceiver/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func TestLoadConfig(t *testing.T) {
require.NoError(t, sub.Unmarshal(cfg))

if tt.expectedErr != nil {
assert.ErrorIs(t, component.ValidateConfig(cfg), tt.expectedErr)
assert.ErrorContains(t, component.ValidateConfig(cfg), tt.expectedErr.Error())
return
}
assert.NoError(t, component.ValidateConfig(cfg))
Expand All @@ -84,7 +84,7 @@ func TestConfigValidateMissingAuth(t *testing.T) {
cfg := createDefaultConfig().(*Config)
cfg.Queue = "someQueue"
err := component.ValidateConfig(cfg)
assert.Equal(t, errMissingAuthDetails, err)
assert.ErrorContains(t, err, errMissingAuthDetails.Error())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not "ErrorIs"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The motivation for these changes is open-telemetry/opentelemetry-collector#12108, which prints the path in the config that produced this error. I don't want to depend on the format of the error message produced by component.ValidateConfig, only the part of the error produced by this component.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am going to merge the PR to unblock the work on core, since this are tests if you want to change it after the PR is merged we can do that

}

func TestConfigValidateMultipleAuth(t *testing.T) {
Expand All @@ -93,14 +93,14 @@ func TestConfigValidateMultipleAuth(t *testing.T) {
cfg.Auth.PlainText = &SaslPlainTextConfig{"Username", "Password"}
cfg.Auth.XAuth2 = &SaslXAuth2Config{"Username", "Bearer"}
err := component.ValidateConfig(cfg)
assert.Equal(t, errTooManyAuthDetails, err)
assert.ErrorContains(t, err, errTooManyAuthDetails.Error())
}

func TestConfigValidateMissingQueue(t *testing.T) {
cfg := createDefaultConfig().(*Config)
cfg.Auth.PlainText = &SaslPlainTextConfig{"Username", "Password"}
err := component.ValidateConfig(cfg)
assert.Equal(t, errMissingQueueName, err)
assert.ErrorContains(t, err, errMissingQueueName.Error())
}

func TestConfigValidateMissingFlowControl(t *testing.T) {
Expand All @@ -110,7 +110,7 @@ func TestConfigValidateMissingFlowControl(t *testing.T) {
// this should never happen in reality, test validation anyway
cfg.Flow.DelayedRetry = nil
err := cfg.Validate()
assert.Equal(t, errMissingFlowControl, err)
assert.ErrorContains(t, err, errMissingFlowControl.Error())
}

func TestConfigValidateInvalidFlowControlDelayedRetryDelay(t *testing.T) {
Expand All @@ -121,7 +121,7 @@ func TestConfigValidateInvalidFlowControlDelayedRetryDelay(t *testing.T) {
Delay: -30 * time.Second,
}
err := cfg.Validate()
assert.Equal(t, errInvalidDelayedRetryDelay, err)
assert.ErrorContains(t, err, errInvalidDelayedRetryDelay.Error())
}

func TestConfigValidateSuccess(t *testing.T) {
Expand Down
38 changes: 20 additions & 18 deletions receiver/windowsperfcountersreceiver/config_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@ func TestLoadConfig(t *testing.T) {
}

tests := []struct {
id component.ID
expected component.Config
expectedErr string
id component.ID
expected component.Config
expectedErrs []string
}{
{
id: component.NewIDWithName(metadata.Type, ""),
Expand Down Expand Up @@ -180,34 +180,34 @@ func TestLoadConfig(t *testing.T) {
},
},
{
id: component.NewIDWithName(metadata.Type, "negative-collection-interval"),
expectedErr: "collection_interval must be a positive duration; " + negativeCollectionIntervalErr,
id: component.NewIDWithName(metadata.Type, "negative-collection-interval"),
expectedErrs: []string{"collection_interval must be a positive duration", negativeCollectionIntervalErr},
},
{
id: component.NewIDWithName(metadata.Type, "noperfcounters"),
expectedErr: noPerfCountersErr,
id: component.NewIDWithName(metadata.Type, "noperfcounters"),
expectedErrs: []string{noPerfCountersErr},
},
{
id: component.NewIDWithName(metadata.Type, "noobjectname"),
expectedErr: noObjectNameErr,
id: component.NewIDWithName(metadata.Type, "noobjectname"),
expectedErrs: []string{noObjectNameErr},
},
{
id: component.NewIDWithName(metadata.Type, "nocounters"),
expectedErr: fmt.Sprintf(noCountersErr, "object"),
id: component.NewIDWithName(metadata.Type, "nocounters"),
expectedErrs: []string{fmt.Sprintf(noCountersErr, "object")},
},
{
id: component.NewIDWithName(metadata.Type, "allerrors"),
expectedErr: fmt.Sprintf(
"collection_interval must be a positive duration; %s; %s; %s; %s",
expectedErrs: []string{
"collection_interval must be a positive duration",
fmt.Sprintf(noCountersErr, "object"),
fmt.Sprintf(emptyInstanceErr, "object"),
noObjectNameErr,
negativeCollectionIntervalErr,
),
},
},
{
id: component.NewIDWithName(metadata.Type, "emptyinstance"),
expectedErr: fmt.Sprintf(emptyInstanceErr, "object"),
id: component.NewIDWithName(metadata.Type, "emptyinstance"),
expectedErrs: []string{fmt.Sprintf(emptyInstanceErr, "object")},
},
}

Expand All @@ -220,8 +220,10 @@ func TestLoadConfig(t *testing.T) {
require.NoError(t, err)
require.NoError(t, sub.Unmarshal(cfg))

if tt.expectedErr != "" {
assert.Equal(t, tt.expectedErr, component.ValidateConfig(cfg).Error())
if len(tt.expectedErrs) > 0 {
for _, err := range tt.expectedErrs {
assert.ErrorContains(t, component.ValidateConfig(cfg), err)
}
return
}
assert.NoError(t, component.ValidateConfig(cfg))
Expand Down