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

Mockito testing - manually activate req. context when creating Mockito spies #35606

Merged
merged 1 commit into from
Aug 30, 2023

Conversation

manovotn
Copy link
Contributor

Fixes #35581

Allows to use req. scoped Mockito spies.
ATM we initialize all spies right after creation of test instance - this is fine except there is no req. scope active because we active that for each test method. This fix activates/deactivates req. context when creating spies.

@quarkus-bot

This comment has been minimized.

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, but I confess I didn't look elsewhere to see what we do with the request context. (Is it properly destroyed somewhere?)

@quarkus-bot

This comment has been minimized.

@manovotn
Copy link
Contributor Author

LGTM, but I confess I didn't look elsewhere to see what we do with the request context. (Is it properly destroyed somewhere?)

Good point @Ladicek; I have now made use of QuarkusTestAfterAllCallback to destroy any context states that we create along the way. This should make sure we destroy everything we created.

@quarkus-bot

This comment has been minimized.

Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

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

I added a comment that should be very easy to address

Copy link
Contributor

@geoand geoand left a comment

Choose a reason for hiding this comment

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

👌🏼

@geoand geoand added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Aug 30, 2023
@manovotn manovotn removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Aug 30, 2023
@quarkus-bot
Copy link

quarkus-bot bot commented Aug 30, 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.

@geoand geoand merged commit 1c7152b into quarkusio:main Aug 30, 2023
@quarkus-bot quarkus-bot bot added kind/bugfix kind/enhancement New feature or request labels Aug 30, 2023
@quarkus-bot quarkus-bot bot added this to the 3.4 - main milestone Aug 30, 2023
@manovotn
Copy link
Contributor Author

Hm, this might not have been right - I am just discussing it with Martin.
I will open a new PR if needed.

@manovotn manovotn deleted the issue35581 branch August 30, 2023 08:59
@geoand
Copy link
Contributor

geoand commented Aug 30, 2023

👌🏼

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.

@InjectSpy in combination with @RequestScope beans in @Nested test classes
3 participants