Jukito should give same instances to requested injection and tests

[dougmill]

To demonstrate the problem, I made a toy project with these classes:

@Singleton
public class InjectedClass {
    private static int instanceCount = 0;

    public InjectedClass() {
        instanceCount++;
    }
}

public class InjectionReceiver {
    private static int instanceCount = 0;
    private static int injectCount = 0;

    public InjectionReceiver() {
        instanceCount++;
    }

    @Inject
    public void inject(InjectedClass injectedObject) {
        injectCount++;
    }
}

public class InjectingModule extends AbstractModule {
    @Override
    protected void configure() {
        requestInjection(new InjectionReceiver());
    }
}

and this test:

@RunWith(JukitoRunner.class)
public class InjectTest {
    public static class Module extends JukitoModule {
        @Override
        protected void configureTest() {
            install(new InjectingModule());
        }
    }

    @Test
    public void test(InjectedClass injectedObject) {
        int a = 1;
    }
}

When I run the test with break points to follow the execution flow, InjectTest runs its module configuration twice (annoying, but mostly ignorable), which creates two InjectionReceiver objects and requests that each of them get injected. The first never gets injected, I suspect because it gets thrown away and garbage collected.

After this, one instance of InjectedClass is created and passed in to the second InjectionReceiver instance's inject method. Then another instance of InjectedClass is created and that instance is passed in to InjectTest.test. This is a bug.

In my production code, Guice creates only a single instance of the InjectedClass analog, and that same instance is passed in to both the InjectionReceiver.inject analog and every other place the class is called for. This is working correctly, and my code depends on this behavior to function.

In order for my tests to work, they have to have the same instance that is passed in to InjectionReceiver.inject, and they aren't getting it.

[christiangoudreau]

@PhilBeaudoin Before investigating, do you have an idea about why this happens?

[mmadson]

I've tracked down the source of this bug. It seems to be a side effect of the BindingsCollector here:

https://github.com/ArcBees/Jukito/blob/master/jukito/src/main/java/org/jukito/JukitoRunner.java#L112

In order for the collector to collect the bindings it needs to call the configure method of the module, which will create the first InjectionReceiver in the above example.

Later when the test module is used to create the injector

https://github.com/ArcBees/Jukito/blob/master/jukito/src/main/java/org/jukito/JukitoRunner.java#L116

the configure method of the test module will be called a second time causing another InjectionReceiver to be instantiated.

If @dougmill can do without automocking, then extending TestModule instead of JuktioModule may be a sufficient workaround, since only JukitoModules need to have their bindings inspected using the BindingsCollector.

[dougmill]

@mmadson Two InjectionReceivers is not ideal, but not really a problem. The important part of this issue is getting two distinct instances of InjectedClass, which you don't mention.

And automocking is a large part of why I use Jukito, giving that up is a very poor option.

[mmadson]

@dougmill I guess I didn't illustrate the problem clearly enough. Since the configure method of your test module gets called twice, once to collect the binding definitions (so that juktio can determine what it needs to mock) and again when the module is actually installed to the injector that will be used to provide instances to the test class, the requestInjection will also be called twice on 2 different instances of InjectionReceiver, which is what causes the injection count to be incorrect.

I agree that using TestModule and sacrificing the automocking is a poor alternative, but I was really just trying to bring the soruce of the problem to the Arcbees maintainer's attention so that your problem might get resolved.

As for actually fixing the problem... I'd need to spend quite a bit more time with the codebase to come up with ideas. My first idea would be to investigate JIT bindings to see if there is a way to JIT bind the mocks without having to use the extension SPI to collect the bindings in advance.