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

Small follow-up for @WithTestResource change #43004

Merged
merged 2 commits into from
Sep 4, 2024

Conversation

gsmet
Copy link
Member

@gsmet gsmet commented Sep 4, 2024

No description provided.

The semantic is that all the test resources have to match so let's use
the plural form.

This comment was marked as resolved.

Copy link
Member Author

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

The comments I added in the review are probably more important than the changes themselves.

* Note that when a restart is needed, all the resources will be restarted.
* This includes the global test resources and the Dev Services.
* <p>
* Quarkus groups the tests by test resources to reduce the number of restarts.
Copy link
Member Author

Choose a reason for hiding this comment

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

This is not true yet but I think we need to have it before we release 3.16.

/cc @famod - FWIW, I think a hash code of the sorted concatenation of resource classes would be enough. But we need to deal with multiple @WithTestResource and also with meta annotations.

Comment on lines +15 to +16
* Restarting Quarkus for this test means that the test resources will be restarted.
* This includes the global test resources and the Dev Services.
Copy link
Member Author

Choose a reason for hiding this comment

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

That's what makes me a bit unsure about the semantic.

The semantic is actually more about the test itself than about the attached test resources. This is even more obvious when you have global and non global test resources attached.

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed

@geoand geoand added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Sep 4, 2024
Copy link

quarkus-bot bot commented Sep 4, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 6afca63.

Failing Jobs

Status Name Step Failures Logs Raw logs Build scan
Quickstarts Compilation - JDK 17 Compile Quickstarts Failures Logs Raw logs 🚧

You can consult the Develocity build scans.

Failures

⚙️ Quickstarts Compilation - JDK 17 #

- Failing: optaplanner-quickstart 

📦 optaplanner-quickstart

Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.11.0:testCompile (default-testCompile) on project optaplanner-quickstart: Compilation failure /home/runner/work/quarkus/quarkus/quarkus-quickstarts/optaplanner-quickstart/src/test/java/org/acme/optaplanner/TestResources.java:[6,57] cannot find symbol symbol: method restrictToAnnotatedClass() location: @interface io.quarkus.test.common.WithTestResource


Flaky tests - Develocity

⚙️ JVM Tests - JDK 17

📦 extensions/smallrye-reactive-messaging/deployment

io.quarkus.smallrye.reactivemessaging.hotreload.ConnectorChangeTest.testUpdatingConnector - History

  • Expecting actual: ["-4","-5","-6","-7","-8","-9","-10","-11"] to start with: ["-3", "-4", "-5", "-6"] - java.lang.AssertionError
java.lang.AssertionError: 

Expecting actual:
  ["-4","-5","-6","-7","-8","-9","-10","-11"]
to start with:
  ["-3", "-4", "-5", "-6"]

	at io.quarkus.smallrye.reactivemessaging.hotreload.ConnectorChangeTest.testUpdatingConnector(ConnectorChangeTest.java:36)

@geoand geoand merged commit 547ffd3 into quarkusio:main Sep 4, 2024
51 of 52 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.16 - main milestone Sep 4, 2024
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Sep 4, 2024
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.

2 participants