-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Fix for Integration Tests with test profiles: dev services config overrides #48627
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
base: main
Are you sure you want to change the base?
Fix for Integration Tests with test profiles: dev services config overrides #48627
Conversation
This comment has been minimized.
This comment has been minimized.
14e2a5d
to
04e7f63
Compare
Rebasing after #48614 |
@alesj @brunobat The failure in For context, before With this change config overrides are also "active" on custom augmentation to start dev services. |
We enabled it for our Gradle tests but since then we have merged several PRs that were failing. The last occurrence is this PR: - quarkusio#48614 And it caused failures as in: - quarkusio#48627 (comment)
This comment has been minimized.
This comment has been minimized.
…ermining the test profile and overridden properties
…arted at the runtime
04e7f63
to
770d2d8
Compare
This comment has been minimized.
This comment has been minimized.
Failing opentelemetry tests are flaky, and I cannot reproduce the failing For the It did seemed like working before because the profile config wasn't read. So observability dev service was active, not the dev resource mechanism. |
I agree.
Ohhh, that's so subtle! I wonder how often that happens. Often, I suspect. I had to work quite hard in the dev service tests to check the test was connected to the right one, and normal tests wouldn't necessarily do that. That might explain why this test is known to be flaky, though. |
This is only for LGTM Dev Resource, as it is the only dev resource implementation we've. |
I'll let @alesj comment on this. |
//@Disabled | ||
// TODO |
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.
I think we can remove these two comments, right?
<executions> | ||
<execution> | ||
<goals> | ||
<goal>integration-test</goal> | ||
<goal>verify</goal> | ||
</goals> | ||
</execution> | ||
</executions> |
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.
I'd like to see a more generic solution for this, so that we have consistent behaviour in integration tests rather than per-project adjustments to try and ensure integration tests run. But we may not be able to achieve that.
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.
Confirmed that this isn't the right way to get integration tests running; we only want them to run in native CIs. I've fixed and updated the native matrix instead.
We enabled it for our Gradle tests but since then we have merged several PRs that were failing. The last occurrence is this PR: - quarkusio#48614 And it caused failures as in: - quarkusio#48627 (comment) (cherry picked from commit b4ce66a)
QuarkusIntegrationTest
andQuarkusMainIntegrationTest
prepare dev services after determining the test profile and applying config overrides (using system properties).Added QuarkusIntegrationTest to redis-devservices IT module and enabled integration test execution.