-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Fix e2e test #6167
Fix e2e test #6167
Conversation
bbf8444
to
c417dd2
Compare
c417dd2
to
5928181
Compare
Codecov Report
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great 👍
@@ -40,12 +40,12 @@ instances: | |||
## @param host - string - required | |||
## JMX hostname to connect to. | |||
# | |||
- host: <HOST> | |||
- host: localhost |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, we don't need to override this anymore don't we? host and port are overriden in conftest
.
The placeholder are our usual way to proceed for required parameters, any reason to change them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, we don't need to override this anymore don't we?
Overriding in the test makes the test more independent to the sample config.
The placeholder are our usual way to proceed for required parameters, any reason to change them?
The sample files had these values before applying specs. I'm happy to revert this override but I thought is probably useful to keep these default values
The e2e test was relying on the sample configuration default values that were changed to a placeholder when introducing specs here https://github.com/DataDog/integrations-core/pull/6115/files
This PR: