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

QuarkusComponentTest: refactorings and API changes #35768

Merged
merged 1 commit into from
Sep 7, 2023
Merged

QuarkusComponentTest: refactorings and API changes #35768

merged 1 commit into from
Sep 7, 2023

Conversation

mkouba
Copy link
Contributor

@mkouba mkouba commented Sep 6, 2023

  • determine the test phase when the container is started from the TestInstance#value()
  • introduce the QuarkusComponentTestExtensionBuilder to create immutable extension instance when programmatic API is used
  • TestConfigProperty can be declared on test methods

@mkouba
Copy link
Contributor Author

mkouba commented Sep 6, 2023

It's a DRAFT because the docs need to be updated as well.

This is a breaking change (the programmatic API only) but it should be ok since it's an experimental feature. I will add a note to the migration guide.

@Ladicek
Copy link
Contributor

Ladicek commented Sep 6, 2023 •
edited
Loading

If I understand it correctly, we will run the entire BeanProcessor for each test method with Lifecycle.PER_METHOD. I think that's a waste of time. Shouldn't it be possible to do that once, in "before all", and then for each test method just shutdown and init again (Arc.shutdown() + Arc.initialize())?

EDIT: otherwise LGTM, and I like how the test configuration is externalized to an extra class.

@mkouba mkouba force-pushed the qct-per-method-lifecycle-support branch from fae399e to 1eebd1e Compare September 6, 2023 09:52
@mkouba
Copy link
Contributor Author

mkouba commented Sep 6, 2023

If I understand it correctly, we will run the entire BeanProcessor for each test method with Lifecycle.PER_METHOD. I think that's a waste of time. Shouldn't it be possible to do that once, in "before all", and then for each test method just shutdown and init again (Arc.shutdown() + Arc.initialize())?

Well, if we allow users to change the config per test method then it might make sense to restart the whole thing. I mean we don't support things like @IfBuildProperty (yet?) but... OTOH we could always detect a per-method config and adapt the strategy. So I'll look into this ;-).

@mkouba mkouba force-pushed the qct-per-method-lifecycle-support branch from 1eebd1e to d37cc2a Compare September 6, 2023 12:16
@mkouba
Copy link
Contributor Author

mkouba commented Sep 6, 2023

If I understand it correctly, we will run the entire BeanProcessor for each test method with Lifecycle.PER_METHOD. I think that's a waste of time. Shouldn't it be possible to do that once, in "before all", and then for each test method just shutdown and init again (Arc.shutdown() + Arc.initialize())?

Well, if we allow users to change the config per test method then it might make sense to restart the whole thing. I mean we don't support things like @IfBuildProperty (yet?) but... OTOH we could always detect a per-method config and adapt the strategy. So I'll look into this ;-).

It seems to work (with some tweaks here and there ;-)...

@mkouba @Ladicek
QuarkusComponentTest: refactorings and API changes
1a2de65 
- determine the test phase in which the container should be started from the
TestInstance lifecycle
- introduce the QuarkusComponentTestExtensionBuilder to create immutable
extension instance when programmatic API is used
- TestConfigProperty can be declared on test methods

Co-authored-by: Ladislav Thon <ladicek@gmail.com>
@mkouba mkouba force-pushed the qct-per-method-lifecycle-support branch from d37cc2a to 1a2de65 Compare September 6, 2023 12:36
@mkouba mkouba marked this pull request as ready for review September 6, 2023 12:36
@quarkus-bot quarkus-bot bot added this to To do in Quarkus Documentation Sep 6, 2023
Ladicek
Ladicek approved these changes Sep 6, 2023
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.

Nice!

Quarkus Documentation automation moved this from To do to Reviewer approved Sep 6, 2023
@mkouba mkouba added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Sep 6, 2023
@github-actions
Copy link

github-actions bot commented Sep 6, 2023 •
edited
Loading

🙈 The PR is closed and the preview is expired.

@quarkus-bot

This comment has been minimized.

Sign in to view
@quarkus-bot
Copy link

quarkus-bot bot commented Sep 7, 2023

✔️ 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.

@mkouba mkouba merged commit 2c25d2f into quarkusio:main Sep 7, 2023
26 checks passed
Quarkus Documentation automation moved this from Reviewer approved to Done Sep 7, 2023
@quarkus-bot quarkus-bot bot added this to the 3.5 - main milestone Sep 7, 2023
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Sep 7, 2023
@mkouba
Copy link
Contributor Author

mkouba commented Sep 7, 2023

FTR the migration guide: https://github.com/quarkusio/quarkus/wiki/Migration-Guide-3.4#component-testing

@mkouba mkouba mentioned this pull request Sep 13, 2023
Closed
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

@holly-cummins holly-cummins holly-cummins approved these changes

@manovotn manovotn Awaiting requested review from manovotn

Assignees
No one assigned
Labels
area/documentation area/testing release/breaking-change
Projects
Quarkus Documentation
Done
Milestone
3.5.0.CR1
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@mkouba @Ladicek @holly-cummins