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

Use single mock when backing bean is the same instance #15427

Merged
merged 1 commit into from
Mar 4, 2021
Merged

Use single mock when backing bean is the same instance #15427

merged 1 commit into from
Mar 4, 2021

Conversation

geoand
Copy link
Contributor

@geoand geoand commented Mar 3, 2021

Fixes: #15411

@geoand geoand mentioned this pull request Mar 3, 2021
Closed
@geoand
Copy link
Contributor Author

geoand commented Mar 3, 2021 •
edited

Keeping in draft as master is broken for the moment. Once it is fixed, I'll move the PR out of draft

@geoand geoand marked this pull request as ready for review March 3, 2021 08:04
@geoand geoand requested a review from famod March 3, 2021 08:05
mkouba
mkouba reviewed Mar 3, 2021
View reviewed changes
Copy link
Contributor

@mkouba mkouba left a comment

Choose a reason for hiding this comment

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

Looks good. But beware that there might be use cases where a shared mock does not help. For example, if both I2 and I3 extend some InterfaceX and a user would attempt to mock InterfaceX#foo() for each of the @InjectMock (it's a corner case but still ;-).

...mockito/src/main/java/io/quarkus/test/junit/mockito/internal/CreateMockitoMocksCallback.java Outdated Show resolved Hide resolved
integration-tests/injectmock/src/test/java/io/quarkus/it/mockbean/SameBeanInstanceTest.java Outdated Show resolved Hide resolved
@geoand geoand force-pushed the #15411 branch 2 times, most recently from 7f5f38e to 4df5102 Compare March 3, 2021 12:04
@geoand
Copy link
Contributor Author

geoand commented Mar 3, 2021

Looks good. But beware that there might be use cases where a shared mock does not help. For example, if both I2 and I3 extend some InterfaceX and a user would attempt to mock InterfaceX#foo() for each of the @InjectMock (it's a corner case but still ;-).

Yeah true.

But at least this PR is better (or what both OP and @famod expected) than what we currently have :)

@geoand geoand added triage/waiting-for-ci Ready to merge when CI successfully finishes triage/backport? labels Mar 3, 2021
famod
famod approved these changes Mar 3, 2021
View reviewed changes
Copy link
Member

@famod famod left a comment

Choose a reason for hiding this comment

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

I was only able to review this briefly but LGTM!

@geoand geoand merged commit d7a08cf into quarkusio:master Mar 4, 2021
@quarkus-bot quarkus-bot bot added this to the 1.13 - master milestone Mar 4, 2021
@geoand geoand deleted the #15411 branch March 4, 2021 10:23
@gsmet
Copy link
Member

gsmet commented Mar 8, 2021

Won't backport to 1.11 for now, we need some community baking for the PR.

@gsmet gsmet modified the milestones: 1.13 - master, 1.12.2.Final Mar 8, 2021
This was referenced Mar 12, 2021
Merged
Closed
Merged
Closed
Merged
This was referenced Mar 12, 2021
Merged
Merged
Merged
Closed
Merged
Closed
Merged
Merged
Closed
Closed
Merged
Merged
Merged
Closed
Merged
Closed
Merged
Closed
Closed
Closed
Closed
Merged
Merged
Closed
Merged
Merged
Merged
Merged
Merged
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mkouba mkouba mkouba approved these changes

@famod famod famod approved these changes

Assignees
No one assigned
Labels
area/testing triage/waiting-for-ci Ready to merge when CI successfully finishes
Projects
None yet
Milestone
1.12.2.Final
Development

Successfully merging this pull request may close these issues.

@InjectMock replaces existing mocks
4 participants
@geoand @gsmet @mkouba @famod