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

Jump to bottom

Export application name and version in tests #24666

Merged
merged 1 commit into from
Apr 20, 2022
Merged

Export application name and version in tests #24666

merged 1 commit into from
Apr 20, 2022

Conversation

glefloch
Copy link
Member

This export quarkus.application.name and quarkus.application.version properties. This used to resolved model to extract data in the boostrap phase.

close #15013

@glefloch glefloch requested a review from aloubyansky March 31, 2022 11:58
@quarkus-bot quarkus-bot bot added area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/gradle Gradle labels Mar 31, 2022
@quarkus-bot

This comment has been minimized.

Sign in to view
@glefloch
Copy link
Member Author

glefloch commented Apr 5, 2022

@aloubyansky could you have a look? I will fix the build, but I would like to be sure I'm heading in the right direction.

aloubyansky
aloubyansky reviewed Apr 5, 2022
View reviewed changes
...pendent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/app/QuarkusBootstrap.java Outdated
return new CuratedApplication(this, appModelFactory.resolveAppModel(), classLoadingConfig);
CurationResult curationResult = appModelFactory.resolveAppModel();

if (mode == Mode.TEST || test) {
Copy link
Member

Choose a reason for hiding this comment

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

We could probably do this in any mode.

@aloubyansky
Copy link
Member

I think it is, thanks @glefloch

@quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot quarkus-bot bot added the area/spring Issues relating to the Spring integration label Apr 12, 2022
@quarkus-bot

This comment has been minimized.

Sign in to view
@glefloch
Copy link
Member Author

@cescoffier the kafka test keeps failing due to a serialization issue after the devmode reload, I don't see how the failure can be related to my branch, do you have an idea?

@cescoffier
Copy link
Member

Hard to say, but having two failures is highly suspicious.

@glefloch
Copy link
Member Author

The failure is definitely related, without my code, the test pass.
It looks like setting quarkus.application.name set a group.id in the consumer (this is the only difference I can see in logs when running both version of the code).
Without my code, each app restart starts with:


2022-04-13 13:38:01,130 WARN  [io.sma.rea.mes.kafka] (Quarkus Main Thread) SRMSG18216: No `group.id` set in the configuration, generate a random id: deb71522-99d3-4132-91e2-a93469232c7c

Could this be related? (the test is testing devmode). Is there something to reset in the consumer?

@cescoffier
Copy link
Member

cescoffier commented Apr 13, 2022 •
edited
Loading

Oh yeah! The kafka connector uses the application name as consumer group is.

If not set it use a random group id. There are way to reset but it's tricky (need an admin client, the consumer must not be connected...)

@glefloch
Copy link
Member Author

Setting it to an empty value fix the test

@glefloch
Copy link
Member Author

Finally got a green CI :), @aloubyansky @cescoffier could you review it ?

@glefloch glefloch requested a review from cescoffier April 15, 2022 09:30
aloubyansky
aloubyansky reviewed Apr 15, 2022
View reviewed changes
...pendent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/app/QuarkusBootstrap.java Outdated
@@ -159,7 +159,20 @@ public CuratedApplication bootstrap() throws BootstrapException {
appModelFactory.setEnableClasspathCache(true);
}
}
return new CuratedApplication(this, appModelFactory.resolveAppModel(), classLoadingConfig);
CurationResult curationResult = appModelFactory.resolveAppModel();
if (curationResult.getApplicationModel().getApplicationModule() != null
Copy link
Member

Choose a reason for hiding this comment

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

curationResult.getApplicationModel().getAppArtifact() would be simpler and a bit more reliable here.

Copy link
Member Author

Choose a reason for hiding this comment

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

right, I pushed a fix.

@aloubyansky
Copy link
Member

@phillip-kruger could you please check the Open API test changes? Should be ok but just in case.

@quarkus-bot
Copy link

quarkus-bot bot commented Apr 16, 2022

Failing Jobs - Building 4b79122

Status Name Step Failures Logs Raw logs
✖ Gradle Tests - JDK 11 Windows Build Failures Logs Raw logs

Full information is available in the Build summary check run.

Failures

⚙️ Gradle Tests - JDK 11 Windows #

- Failing: integration-tests/gradle

📦 integration-tests/gradle

✖ io.quarkus.gradle.devmode.QuarkusDevDependencyDevModeTest.main line 14 - More details - Source on GitHub

org.awaitility.core.ConditionTimeoutException: Condition with lambda expression in io.quarkus.test.devmode.util.DevModeTestUtils that uses java.util.function.Supplier, java.util.function.Supplierjava.util.concurrent.atomic.AtomicReference, java.util.concurrent.atomic.AtomicReferencejava.lang.String, java.lang.Stringboolean was not fulfilled within 1 minutes.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)

@glefloch
Copy link
Member Author

@phillip-kruger could you have a look at Open api test change to make sure I haven't introduced any regression ?

phillip-kruger
phillip-kruger approved these changes Apr 20, 2022
View reviewed changes
Copy link
Member

@phillip-kruger phillip-kruger left a comment

Choose a reason for hiding this comment

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

Lgtm

@phillip-kruger
Copy link
Member

Sorry, I was meaning to look at this and forgot. All fine. Before when the name was to available in test profile it fell back to the old static name, this is much better

@glefloch
Copy link
Member Author

No problem, thanks for review.
@aloubyansky can we merge this one? the gradle failure does not seem to be related.

@aloubyansky
Copy link
Member

Yes, thanks @glefloch

@aloubyansky aloubyansky merged commit 30b369c into quarkusio:main Apr 20, 2022
@quarkus-bot quarkus-bot bot added this to the 2.9 - main milestone Apr 20, 2022
@quarkus-bot quarkus-bot bot added the kind/enhancement New feature or request label Apr 20, 2022
@glefloch glefloch deleted the fix/15013 branch April 20, 2022 14:55
This was referenced Apr 22, 2022
Merged
Closed
Closed
@gsmet gsmet modified the milestones: 2.9 - main, 2.8.2.Final Apr 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@phillip-kruger phillip-kruger phillip-kruger approved these changes

@aloubyansky aloubyansky aloubyansky approved these changes

@cescoffier cescoffier Awaiting requested review from cescoffier

Assignees
No one assigned
Labels
area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/gradle Gradle area/openapi area/reactive-messaging area/smallrye area/spring Issues relating to the Spring integration kind/enhancement New feature or request
Projects
None yet
Milestone
2.8.2.Final
Development

Successfully merging this pull request may close these issues.

Application name and version are not available in test
5 participants
@glefloch @aloubyansky @cescoffier @phillip-kruger @gsmet