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

QuarkusTest: veto all test classes belonging to a different test profile #45336

Merged
merged 1 commit into from
Jan 3, 2025
Merged

QuarkusTest: veto all test classes belonging to a different test profile #45336

merged 1 commit into from
Jan 3, 2025
+73 −2

Conversation

mkouba
Copy link
Contributor

@mkouba mkouba commented Jan 2, 2025 •
edited
Loading

@quarkus-bot quarkus-bot bot added the area/arc Issue related to ARC (dependency injection) label Jan 2, 2025
@mkouba mkouba requested review from Ladicek and manovotn January 2, 2025 14:32
Ladicek
Ladicek approved these changes Jan 2, 2025
View reviewed changes
Copy link
Contributor

@Ladicek Ladicek left a comment

Choose a reason for hiding this comment

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

LGTM!

manovotn
manovotn approved these changes Jan 2, 2025
View reviewed changes
Copy link
Contributor

@manovotn manovotn left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for diving into it :)

@manovotn manovotn added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Jan 2, 2025
@quarkus-bot quarkus-bot

This comment has been minimized.

Sign in to view
@manovotn
Copy link
Contributor

manovotn commented Jan 3, 2025

@mkouba the CI failures are related.
For nested test classes we automatically assume the outer class is a CDI bean and attempt to initialize it as such which fails with the changes present here (the outer class will be vetoed).

@mkouba
Copy link
Contributor Author

mkouba commented Jan 3, 2025 •
edited
Loading

@mkouba the CI failures are related. For nested test classes we automatically assume the outer class is a CDI bean and attempt to initialize it as such which fails with the changes present here (the outer class will be vetoed).

Well, that's a bit weird because in this particular test the enclosing test class has a different profile...

We could detect this case where a test class contains some @Nested tests but it's IMO wrong and it can still cause troubles.

@mkouba
QuarkusTest: veto all test classes belonging to a different test profile
7123aeb 
- note that we cannot veto test classes that contain a matching nested
  test; we should probably drop the test profile support for nested tests, see quarkusio#45349
- fixes quarkusio#45308
@quarkus-bot quarkus-bot
Copy link

quarkus-bot bot commented Jan 3, 2025 •
edited by github-actions bot
Loading

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 7123aeb.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

@mkouba mkouba merged commit 9e2523f into quarkusio:main Jan 3, 2025
52 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.18 - main milestone Jan 3, 2025
@quarkus-bot quarkus-bot bot added kind/bugfix and removed triage/waiting-for-ci Ready to merge when CI successfully finishes labels Jan 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ladicek Ladicek Ladicek approved these changes

@manovotn manovotn manovotn approved these changes

Assignees
No one assigned
Labels
area/arc Issue related to ARC (dependency injection) kind/bugfix triage/flaky-test
Projects
None yet
Milestone
3.18 - main
Development

Successfully merging this pull request may close these issues.

IfBuildProfile does not work on scheduler classes
3 participants
@mkouba @manovotn @Ladicek