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

Fail on @MockBean being used with class #1015

Open
musketyr opened this issue Apr 25, 2024 · 10 comments
Open

Fail on @MockBean being used with class #1015

musketyr opened this issue Apr 25, 2024 · 10 comments
Labels
status: next major version The issue will be considered for the next major version status: under consideration The issue is being considered, but has not been accepted yet

Comments

@musketyr
Copy link

Feature description

At the moment, beans that are not injected by interface (e.g. they don't implement any interface) cannot be replaced using @MockBean (see #535). This is a commons source of frustration as there is no warning or failure from the compiler when someone do so. I suggest that whenever the @MockBean is used on a field that is not an interface or a method that does not return an interface then a compilation error is being thrown to inform the developer that the @MockBean annotation will have no effect.

@sdelamo
Copy link
Contributor

sdelamo commented Apr 29, 2024

@timyates can you look if we can throw an exception or inform it. @musketyr can you create a sample project?

@musketyr
Copy link
Author

@timyates
Copy link
Contributor

Oddly...this seems to pass for me 🤔

@MicronautTest
@Property(name = "spec.name", value = "MockBeanClassTest")
class MockBeanClassTest {

    @Inject
    MainService mainService;

    @Inject
    ServiceThatIsAClass service;

    @MockBean(ServiceThatIsAClass.class)
    ServiceThatIsAClass serviceThat() {
        return Mockito.mock(ServiceThatIsAClass.class);
    }

    @Test
    void testDoSomething() {
        mainService.doSomething();

        // verify that the method was called 1 time
        Mockito.verify(service, Mockito.times(1)).doSomething();
    }

    @Singleton
    @Requires(property = "spec.name", value = "MockBeanClassTest")
    public static class MainService {

        private final ServiceThatIsAClass service;

        public MainService(ServiceThatIsAClass service) {
            this.service = service;
        }

        void doSomething() {
            service.doSomething();
        }
    }

    @Singleton
    @Requires(property = "spec.name", value = "MockBeanClassTest")
    public static class ServiceThatIsAClass {

        public void doSomething() {
            System.out.println("Doing something");
        }
    }
}

I'll try moving the classes to be non-internal and see if that breaks things 🤔

@musketyr
Copy link
Author

I wouldn't complain if you actually find out a solution for #535 instead 😅

@timyates
Copy link
Contributor

I'll see if I can come up with a non-lombok reproducer for that 🤔

@timyates
Copy link
Contributor

timyates commented Apr 29, 2024

Your reproducer test passes if I make the method public 🤔

diff --git a/src/main/java/com/example/ServiceThatIsAClass.java b/src/main/java/com/example/ServiceThatIsAClass.java
index d59b498..bd923ef 100644
--- a/src/main/java/com/example/ServiceThatIsAClass.java
+++ b/src/main/java/com/example/ServiceThatIsAClass.java
@@ -5,7 +5,7 @@ import jakarta.inject.Singleton;
 @Singleton
 public class ServiceThatIsAClass {
 
-    void doSomething() {
+    public void doSomething() {
         System.out.println("Doing something");
     }
 

@musketyr
Copy link
Author

ok, I have just tired to create Spock reproducer that is more close to our codebase but it still passes. I'll try to dig deeper to figure out what is the difference between the reproducer and our codebase

@musketyr
Copy link
Author

ok, I was trying to make it fail and I think I found the common source off issues that is forgetting to add the value of the @MockBean annotation.

@MicronautTest
class ServiceThatIsAClassTest {


    @Inject OtherService otherService;
    @Inject ServiceThatIsAClass serviceThat;

    @MockBean
    ServiceThatIsAClass serviceThat() {
        return Mockito.mock(ServiceThatIsAClass.class);
    }

    @Test
    void testDoSomething() {
        otherService.doSomething();

        // verify that the method was called 1 time
        Mockito.verify(serviceThat, Mockito.times(1)).doSomething();
    }


}

if the mocked bean is an interface it fails with duplicate bean but with the bean being a class it executes silently and it never replaces the bean. Knowing this, would it be possible to make the value of @MockBean required, at least for non-interface classes?

@timyates
Copy link
Contributor

timyates commented May 1, 2024

Ahhhh... Ok, getting closer...

When we add the class to the @MockBean annotation this is an alias for a @Replaces annotation in addition

/**
* @return The bean this mock replaces
*/
@AliasFor(annotation = Replaces.class, member = "value")
Class<?> value() default void.class;

If we change your example to the following:

@MicronautTest
class ServiceThatIsAClassTest {


    @Inject OtherService otherService;
    @Inject ServiceThatIsAClass serviceThat;

    @MockBean
    @Replaces(ServiceThatIsAClass.class)
    ServiceThatIsAClass serviceThat() {
        return Mockito.mock(ServiceThatIsAClass.class);
    }

    @Test
    void testDoSomething() {
        otherService.doSomething();

        // verify that the method was called 1 time
        Mockito.verify(serviceThat, Mockito.times(1)).doSomething();
    }
}

It works again...

I think we should make the value of MockBean mandatory, and remove the convenience of leaving it blank...

However this is a breaking change that would break applications where peiople are using it successfully with interfaces...

It could be considered for Micronaut 5.0 though 🤔

@sdelamo sdelamo added status: under consideration The issue is being considered, but has not been accepted yet status: next major version The issue will be considered for the next major version labels May 2, 2024
@sdelamo sdelamo removed this from 4.5.0 Release May 2, 2024
@musketyr
Copy link
Author

musketyr commented May 2, 2024

Thanks @timyates! Under normal circumstances, the number of @MockBean annotations that has no value and still works should be minimal. For example in our codebase there's 1455 @MockBean annotations and only 4 of them are not having the value and mocks interfaces. In most of the applications the test should fail with NonUniqueBeanException like on this branch.

io.micronaut.context.exceptions.DependencyInjectionException: Failed to inject value for parameter [serviceThat] of class: com.example.OtherService

Message: Multiple possible bean candidates found: [$ServiceThatIsAClassTest$ServiceThat0$Definition$Intercepted, ServiceThatIsAClass]
Path Taken: ServiceThatIsAClassTest.otherService --> new OtherService([SomeInterface serviceThat])
	at io.micronaut.context.AbstractInitializableBeanDefinition.resolveBean(AbstractInitializableBeanDefinition.java:2177)
	at io.micronaut.context.AbstractInitializableBeanDefinition.getBeanForConstructorArgument(AbstractInitializableBeanDefinition.java:1324)
	at com.example.$OtherService$Definition.instantiate(Unknown Source)
	at io.micronaut.context.DefaultBeanContext.resolveByBeanFactory(DefaultBeanContext.java:2311)
	at io.micronaut.context.DefaultBeanContext.doCreateBean(DefaultBeanContext.java:2281)
	at io.micronaut.context.DefaultBeanContext.doCreateBean(DefaultBeanContext.java:2293)
	at io.micronaut.context.DefaultBeanContext.createRegistration(DefaultBeanContext.java:3095)
	at io.micronaut.context.SingletonScope.getOrCreate(SingletonScope.java:80)
	at io.micronaut.context.DefaultBeanContext.findOrCreateSingletonBeanRegistration(DefaultBeanContext.java:2997)
	at io.micronaut.context.DefaultBeanContext.resolveBeanRegistration(DefaultBeanContext.java:2958)
	at io.micronaut.context.DefaultBeanContext.resolveBeanRegistration(DefaultBeanContext.java:2732)
	at io.micronaut.context.DefaultBeanContext.getBean(DefaultBeanContext.java:1731)
	at io.micronaut.context.AbstractBeanResolutionContext.getBean(AbstractBeanResolutionContext.java:89)
	at io.micronaut.context.AbstractInitializableBeanDefinition.resolveBean(AbstractInitializableBeanDefinition.java:2161)
	at io.micronaut.context.AbstractInitializableBeanDefinition.getBeanForField(AbstractInitializableBeanDefinition.java:1668)
	at com.example.$ServiceThatIsAClassTest$Definition.inject(Unknown Source)
	at io.micronaut.context.DefaultBeanContext.doInjectAndInitialize(DefaultBeanContext.java:2638)
	at io.micronaut.context.DefaultBeanContext.inject(DefaultBeanContext.java:1006)
	at io.micronaut.test.extensions.AbstractMicronautExtension.beforeEach(AbstractMicronautExtension.java:459)
	at io.micronaut.test.extensions.junit5.MicronautJunit5Extension.beforeEach(MicronautJunit5Extension.java:223)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1596)
Caused by: io.micronaut.context.exceptions.NonUniqueBeanException: Multiple possible bean candidates found: [$ServiceThatIsAClassTest$ServiceThat0$Definition$Intercepted, ServiceThatIsAClass]
	at io.micronaut.context.DefaultBeanContext.findConcreteCandidate(DefaultBeanContext.java:2430)
	at io.micronaut.context.DefaultApplicationContext.findConcreteCandidate(DefaultApplicationContext.java:620)
	at io.micronaut.context.DefaultBeanContext.lastChanceResolve(DefaultBeanContext.java:3247)
	at io.micronaut.context.DefaultBeanContext.pickOneBean(DefaultBeanContext.java:3197)
	at io.micronaut.context.DefaultBeanContext.findConcreteCandidateNoCache(DefaultBeanContext.java:3153)
	at io.micronaut.context.DefaultBeanContext.findConcreteCandidate(DefaultBeanContext.java:3136)
	at io.micronaut.context.DefaultBeanContext.findBeanDefinition(DefaultBeanContext.java:2747)
	at io.micronaut.context.DefaultBeanContext.resolveBeanRegistration(DefaultBeanContext.java:2722)
	at io.micronaut.context.DefaultBeanContext.getBean(DefaultBeanContext.java:1731)
	at io.micronaut.context.AbstractBeanResolutionContext.getBean(AbstractBeanResolutionContext.java:89)
	at io.micronaut.context.AbstractInitializableBeanDefinition.resolveBean(AbstractInitializableBeanDefinition.java:2161)
	... 21 more


Multiple possible bean candidates found: [$ServiceThatIsAClassTest$ServiceThat0$Definition$Intercepted, ServiceThatIsAClass]
io.micronaut.context.exceptions.NonUniqueBeanException: Multiple possible bean candidates found: [$ServiceThatIsAClassTest$ServiceThat0$Definition$Intercepted, ServiceThatIsAClass]
	at app//io.micronaut.context.DefaultBeanContext.findConcreteCandidate(DefaultBeanContext.java:2430)

If not possible to have errors in 4.5.0, can we at least have some warnings in the compiler console that would turn into errors in 5.x?

@timyates timyates removed their assignment May 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: next major version The issue will be considered for the next major version status: under consideration The issue is being considered, but has not been accepted yet
Projects
Status: No status
Development

No branches or pull requests

3 participants