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

@InjectMock replaces existing mocks #15411

Closed
blazmrak opened this issue Mar 2, 2021 · 9 comments · Fixed by #15427
Closed

@InjectMock replaces existing mocks #15411

blazmrak opened this issue Mar 2, 2021 · 9 comments · Fixed by #15427
Assignees
Labels
area/testing kind/bug Something isn't working
Milestone

Comments

@blazmrak
Copy link
Contributor

blazmrak commented Mar 2, 2021

Describe the bug
Basically what I have is class C1, which implements 2 interfaces I1 and I2. Class C2 then injects those 2 interfaces. The actual (non-test) code runs well, but in testing one of the mocks is not called.

It will probably be the simplest to understand with some pseudo code, as I have a hard time explaining the problem with words.

class C1 implements I1, I2 {...}

class C2 {
  @Inject I1
  @Inject I2

  method() {
    I1.m1()
    I2.m2()
  }
}

@QuarkusTest
class Test {
  @Inject C2
  @MockInject I1
  @MockInject I2

  @Test
  test() {
    when(I1)...
    when(I2)...

    C2.method()

    verify(I1) - NOT CALLED
    verify(I2) - CALLED
  }
}

From what I understand what happens is that the mocks replace the actual implementation, but that does not seem correct to me, because the implementation is not what I really want to mock or worry about.

Now... Does this qualify as a bug, or is it a "feature"? Is there some workaround except just creating interfaces that are the "same" as implementations (one interface per class)?

Expected behavior
Both mock get called.

Actual behavior
One mock gets called.

** To reproduce **

git clone https://github.com/blazmrak/inject-mock-issue.git
mvn clean test

Environment (please complete the following information):

  • Output of uname -a or ver: 10.0.19042.804
  • Output of java -version: 11.0.10
  • GraalVM version (if different from Java):
  • Quarkus version or git rev: 1.12
  • Build tool (ie. output of mvnw --version or gradlew --version): 3.6.3
@blazmrak blazmrak added the kind/bug Something isn't working label Mar 2, 2021
@famod
Copy link
Member

famod commented Mar 2, 2021

TBH, I'd expect the same as you do. There might be some limitation related to ArC but that's pure speculation.
Someone needs to take a closer look.

/cc @geoand @mkouba

@mkouba
Copy link
Contributor

mkouba commented Mar 2, 2021

Hm, what's the CDI scope of the I1 bean implementation? AFAIK the @InjectMock and friends only work for normal scoped CDI bean (e.g. @ApplicationScoped, @RequestScoped, etc).

@famod
Copy link
Member

famod commented Mar 2, 2021

As long as class C1 implements I1, I2 {...} and @MockInject I2 works, C1 seems to be a normal scoped bean. 🤔
But it certainly doesn't hurt if @blazmrak would confirm. 🙂

@blazmrak a small sample project reproducing the problem would come in handy.

@mkouba
Copy link
Contributor

mkouba commented Mar 2, 2021

As long as class C1 implements I1, I2...

@famod You're right, except that there might be various CDI constructs that can change the game ;-), e.g. qualifers, @Typed, @Alternative, ...

@blazmrak
Copy link
Contributor Author

blazmrak commented Mar 2, 2021

Sorry, I didn't bother to setup a repo, as I didn't know if it is a problem at all. I will get on it and update the issue.

@mkouba The scope is @ApplicationScoped.

@blazmrak
Copy link
Contributor Author

blazmrak commented Mar 2, 2021

I have added a minimal sample. Notice if you switch the order of fields in test

@InjectMock I3
@InjectMock I2

instead of

@InjectMock I2
@InjectMock I3

there is different mock which does not get called.

@blazmrak
Copy link
Contributor Author

blazmrak commented Mar 2, 2021

@mkouba @famod The problem probably lies with MockitoMocksTracker, as it is beanInstance to mock map.

Basically what happens is that the second mock that is read gets mapped to the same bean instance. Instead of a bean instance, would it be more appropriate to map the type of field + bean instance to the mock? (I have too little experience with quarkus to actually know if there would be a problem with that approach)

@geoand
Copy link
Contributor

geoand commented Mar 2, 2021

I'll take a look sometime this week.

Thanks for the reproducer and the details

@geoand
Copy link
Contributor

geoand commented Mar 3, 2021

#15427 fixes the issue

geoand added a commit to geoand/quarkus that referenced this issue Mar 3, 2021
geoand added a commit to geoand/quarkus that referenced this issue Mar 3, 2021
geoand added a commit to geoand/quarkus that referenced this issue Mar 3, 2021
geoand added a commit to geoand/quarkus that referenced this issue Mar 4, 2021
geoand added a commit that referenced this issue Mar 4, 2021
Use single mock when backing bean is the same instance
@quarkus-bot quarkus-bot bot added this to the 1.13 - master milestone Mar 4, 2021
@gsmet gsmet modified the milestones: 1.13 - master, 1.12.2.Final Mar 8, 2021
gsmet pushed a commit to gsmet/quarkus that referenced this issue Mar 8, 2021
This was referenced Mar 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/testing kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants