-
Notifications
You must be signed in to change notification settings - Fork 344
Move integration tests to polaris-server
#3261
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?
Conversation
| implementation(enforcedPlatform(libs.quarkus.bom)) | ||
| implementation("io.quarkus:quarkus-container-image-docker") | ||
|
|
||
| testImplementation("io.quarkus:quarkus-junit5") |
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.
Two questions / doubts here:
- Why modify the
testImplementationconfiguration here? I don't think it applies to integration tests. - Do we need to add
testImplementation(enforcedPlatform(libs.quarkus.bom))?
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.
oops, my mistake. Thx for catching it... will fix.
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 had to make a build config change and move a non-QuarkusIntegrationTest back to runtime/service... but I hope the end result is better now :)
104e25c to
7b01d17
Compare
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.
Shouldn't we rename this test to something ending with Test? It is also oddly designed, the nested class seems unnecessary.
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.
BTW: the nested test class is not being executed, it is lacking the @Nested annotation. All in all, I think this test class could be deleted.
`@QuarkusIntegrationTest` uses the regular server build with only non-test dependencies. Therefore, those tests naturally belong into the `server` module. This change is as a prerequisite for apache#3252 to allow the `polaris-runtime-service` module to avoid having Polaris extensions as runtime dependencies. Extensions will be runtime dependencies only in the `polaris-server` module.
7b01d17 to
8331a24
Compare
@QuarkusIntegrationTestuses the regular server build with only non-test dependencies. Therefore, those tests naturally belong into theservermodule.This change is as a prerequisite for #3252 to allow the
polaris-runtime-servicemodule to avoid having Polaris extensions as runtime dependencies. Extensions will be runtime dependencies only in thepolaris-servermodule.Checklist
CHANGELOG.md(if needed)site/content/in-dev/unreleased(if needed)