mlinhard / mockito

Automatically exported from code.google.com/p/mockito
0 stars 0 forks source link

@InjectMock try to inject to final or static fields #262

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Say I have an object with a list
public class ObjectWithList {
    private List<String> logs = new ArrayList<String>();

    public List<String> getLogs() {
        return logs;
    }

    public void setLogs(List<String> logs) {
        this.logs = logs;
    }
}
2. In my service, I have a public static final list
public class ExampleService {
    public static final List<String> CONSTANTS = Arrays.asList("c1", "c1");

    private ObjectWithList objectWithList;

    protected void doSomething() {
        // logic
        objectWithList.getLogs().add("logic completed");
    }

    @Autowired
    public void setObjectWithList(ObjectWithList objectWithList) {
        this.objectWithList = objectWithList;
    }
}
3. I tried to use @InjectMock with @Spy on the list for verifications.
@RunWith(MockitoJUnitRunner.class)
public class ExampleServiceTest {
    @Mock
    private ObjectWithList objectWithList;

    @Spy
    private List<String> logs = new ArrayList<String>();

    @InjectMocks
    private ExampleService exampleService = new ExampleService();

    @Before
    public void setUp() throws Exception {
        when(objectWithList.getLogs()).thenReturn(logs);
    }

    @Test
    public void testDoSomething() {
        exampleService.doSomething();
        verify(logs).add("logic completed");
    }
}

What is the expected output? 
Test to pass

What do you see instead?
org.mockito.exceptions.base.MockitoException: Problems injecting dependency in 
CONSTANTS

What version of the product are you using? On what operating system?
mockito version 1.8.5
OS: windows 7

Please provide any additional information below.

I am able to get around the error by manually setting the object and then use 
initMocks.

@RunWith(MockitoJUnitRunner.class)
public class ExampleServiceWorkAroundTest {
    @Mock
    private ObjectWithList objectWithList;

    @Spy
    private List<String> logs = new ArrayList<String>();

    private ExampleService exampleService = new ExampleService();

    @Before
    public void setUp() throws Exception {
        when(objectWithList.getLogs()).thenReturn(logs);
        exampleService.setObjectWithList(objectWithList);

        MockitoAnnotations.initMocks(this);
    }

    @Test
    public void testDoSomething() {
        exampleService.doSomething();
        verify(logs).add("logic completed");
    }
}

Please let me know if I'm doing something wrong.

Thanks

Original issue reported on code.google.com by dodozhan...@gmail.com on 17 May 2011 at 11:24

GoogleCodeExporter commented 9 years ago
Hi,

There is no problem with the way you are using Mockito, final static fields 
should not be targetted for injection i'll fix it.

However, you should mind the Demeter law, instead of that : 
objectWithList.getLogs().add("logic completed");

You should write :
objectWithList.addLog("logic completed");

So it shall be a better solution, and it shall work for your problem too (with 
InjectMocks).

Hope that helps.

Original comment by brice.du...@gmail.com on 18 May 2011 at 9:27

GoogleCodeExporter commented 9 years ago
Thank you for your quick feedback. I appreciate it!

Original comment by dodozhan...@gmail.com on 18 May 2011 at 9:50

GoogleCodeExporter commented 9 years ago
You are welcome :)

Fixed in 
http://code.google.com/p/mockito/source/detail?r=a91c45ecabf30253fa12dfd3176451c
867bf2b9d

Original comment by brice.du...@gmail.com on 18 May 2011 at 11:37

GoogleCodeExporter commented 9 years ago

Original comment by szcze...@gmail.com on 3 Jul 2011 at 12:43

GoogleCodeExporter commented 9 years ago

Original comment by brice.du...@gmail.com on 3 Sep 2012 at 10:00

GoogleCodeExporter commented 9 years ago
Hey guys, why have you disabled injecting final fields ? I understand why 
static finals are disabled, but finals should be (in my opinion) injectable.

Original comment by pio...@gmail.com on 26 Oct 2012 at 10:37

GoogleCodeExporter commented 9 years ago
Because InjectMocks should respect object privacy, as Mockito injects 
automatically, if final fields are candidate to injection it can cause unwanted 
injection for too many people, so we had to make this injection behavior less 
agressive in this regard.

Maybe you can use the following workaround, you can design your object to be 
configurable just as SpringSource guys are designing some of their objects. For 
example one could write.

class TheObject {
  private final SomeStrategy DEFAULT_STRATEGY = new DefaultStrategy();

  private SomeStrategy strategy = DEFAULT_STRATEGY;

  // using a setter, but this might as well be injected via a constructor, which is even more respectfull of the OO pradigm
  public void setStrategy(SomeStrategy) { this.strategy = strategy; }
}

This has the benefit to make your object configurable and then ready for reuse.

Hope that helps
Brice

Original comment by brice.du...@gmail.com on 26 Oct 2012 at 10:56

GoogleCodeExporter commented 9 years ago
It's true that it can cause unwanted injections, but why cannot this be a 
default behaviour with possibility of changing injection strategy to allow 
final injections ?

Original comment by pio...@gmail.com on 26 Oct 2012 at 1:35

GoogleCodeExporter commented 9 years ago
Injecting won't be made default again, this was a bogus injection, plus we 
won't force 80% users to modify their tests to disable final injection.

However we could add an option to enable final injection. However we might 
avoid that and instead propose users to provide their own custom strategy.

Original comment by brice.du...@gmail.com on 26 Oct 2012 at 2:15

GoogleCodeExporter commented 9 years ago
You got me wrong. I agree that injecting finals should be disabled by default. 
Customizable injection strategy would be nice :-) Thanks for your replies.

Original comment by pio...@gmail.com on 26 Oct 2012 at 2:24

GoogleCodeExporter commented 9 years ago
Oh ok :)
You are welcome, thank you for the feedback too :)

Original comment by brice.du...@gmail.com on 26 Oct 2012 at 2:28