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.

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

Conversation

evan-bradley
Copy link
Contributor

Description

Follow up to #37579. This should address issues with the remaining tests.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@@ -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

Copy link
Contributor

@pjanotti pjanotti left a comment

Choose a reason for hiding this comment

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

windowspercountersreceiver LGTM

@mx-psi mx-psi merged commit fb138b5 into open-telemetry:main Jan 31, 2025
163 checks passed
@github-actions github-actions bot added this to the next release milestone Jan 31, 2025
chengchuanpeng pushed a commit to chengchuanpeng/opentelemetry-collector-contrib that referenced this pull request Feb 8, 2025
…telemetry#37594)

<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue.
Ex. Adding a feature - Explain what this achieves.-->
#### Description

Follow up to
open-telemetry#37579.
This should address issues with the remaining tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants