mlinhard / mockito

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

Create a TestNG Listener that does the same job as the MockitoJUnitRunner #304

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Since TestNG only instantiates your test class once and not for each test case 
like JUnit does, you can not use MockitoAnnotations.initMocks(this) just by 
itself with constructor injection.
http://martinfowler.com/bliki/JunitNewInstance.html
http://beust.com/weblog/2004/08/25/testsetup-and-evil-static-methods/

The issue appears to be that FieldInitializer.acquireFieldInstance() checks to 
see if the field is null or not. If it is not null then it will instantiate it 
otherwise you are left with the instance from the previous test case.
So if you have a test class that has multiple test cases, the first one will 
work as expected and the subsequent tests may get unexpected behavior.

Example with TestNG:    

package org.mockitousage.annotation;

import org.mockito.InjectMocks;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;
import org.mockitousage.examples.use.ArticleCalculator;
import org.mockitousage.examples.use.ArticleDatabase;
import org.mockitousage.examples.use.ArticleManager;
import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;

import static org.mockito.Matchers.eq;
import static org.mockito.Mockito.verify;

@Test
public class MockInjectionUsingConstructorUsingTestNGTest {

    @Mock private ArticleCalculator calculator;
    @Mock private ArticleDatabase database;

    @InjectMocks private ArticleManager articleManager;

    @BeforeMethod
    public void setup() {
        MockitoAnnotations.initMocks(this);
    }

    public void initMocks_should_create_all_new_objects_each_time_execution_1() {
        articleManager.updateArticleCounters("new");

        verify(calculator).countArticles(eq("new"));
    }

    // this test will fail because the Mocks injected into ArticleManager will be from the previous test case
    public void initMocks_should_create_all_new_objects_each_time_execution_2() {
        articleManager.updateArticleCounters("new");

        verify(calculator).countArticles(eq("new"));
    }
}

Ideally @InjectMocks would create a new instance each time 
MockitoAnnotations.initMocks is called. But this could cause problems for test 
cases that inline the field initialization.
Example:
    @InjectMocks private ArticleManager articleManager = new ArticleManager();

I can work around the issue by changing the setup to be something like the 
following but it would be nice to not have to.
    @BeforeMethod
    public void setup() {
        articleManager = null;
        MockitoAnnotations.initMocks(this);
    }

Original issue reported on code.google.com by kdomb...@gmail.com on 23 Dec 2011 at 10:11

Attachments:

GoogleCodeExporter commented 9 years ago
Hi,

Yes we are aware of the different philosophy of JUnit and TestNG, the curent 
implementation was a design choice, and is actually expected.

In JUnit this is not a problem since it is running a fresh new instance of the 
test between any test methods. The MockitoJUnitRunner helps to also manage 
better this redundant part.

However TestNG prefer to keep a unique test instance between test method runs 
in the same class, it was decided that we won't break current test code, such 
as the one you pointed out, ie : @InjectMocks private ArticleManager 
articleManager = new ArticleManager();

And TestNG doesn't have to my knowledge something like @RunWith. So users are 
unfortunately stucks to deal with this manually.

On this matter also the following code in your snippet does seem to break the 
TestNG philosophy on this matter, ie only one initialization.

    @BeforeMethod
    public void setup() {
        MockitoAnnotations.initMocks(this);
    }

It would break anyway if you mess with the state of your production code 
between your runs.

While your alternative will work it is so JUnit like, that I think the more 
correct way for TestNG (not the most user friendly though) would be initialize 
stuff before any test method are executed.

So instead of using @BeforeMethod which will be run before each test method as 
the name suggest, you should instead use @BeforeClass whose annotated method 
will be run once for the entire class, almost like the JUnit's @BeforeClass, 
but it will run on the instance rather than on the class.

So you should write :
    @BeforeClass
    public void setup() {
        MockitoAnnotations.initMocks(this);
    }

However this approach has the drawback tht you will need to reset the mocks 
after each test methods :

    @AfterMethod
    public void setup() {
        Mockito.reset(mock1, mock2);
    }

Makes sense ?

While I don't think it will be fixed, your point raise the fact that Javadoc 
could be improved on how to work with this feature and with TestNG.

Original comment by brice.du...@gmail.com on 24 Dec 2011 at 1:13

GoogleCodeExporter commented 9 years ago
If you want to avoid writing an @AfterMethod to reset Mockito's state for each 
of your test, you should consider doing so in an ITestNGListener.

Original comment by cbe...@gmail.com on 28 Dec 2011 at 4:21

GoogleCodeExporter commented 9 years ago
Hi Cedric,

So it would be possible to configure, via an annotation, an extension to make 
all the repetitive work ?

That would be great.

Original comment by brice.du...@gmail.com on 28 Dec 2011 at 9:15

GoogleCodeExporter commented 9 years ago
Yes, you could write an ITestListener and do the reset in the 
onTestFinished/Passed/Failed callbacks. You can declare this listener either as 
an annotation, in your testng.xml or via ServiceLoader (in which case you don't 
need to declare it anywhere, having it in your classpath is all that's needed).

Relevant documentation:

http://testng.org/doc/documentation-main.html#testng-listeners

Original comment by cbe...@gmail.com on 28 Dec 2011 at 3:59

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
That's good news. I didn't knew about listeners in TestNG. I will probably work 
on a better Mockito-TestNG integration then.

For now I think we could write something like :

@Listeners({MockitoTestNGListener.class})
public class TheTestNGTest {
  // @mock / @injectmocks annotated fields

  // test methods
}

Original comment by brice.du...@gmail.com on 28 Dec 2011 at 5:18

GoogleCodeExporter commented 9 years ago
Good point about my example of @BeforeMethod. I guess I was "hacking" TestNG to 
behave like JUnit.

The reason I opened this as a bug was because of the inconsistency of my test 
results between property vs. constructor injection. If ArticleManager is 
written using property injection the unit test above will work but if you use 
constructor injection you will get different behaviour.

A IInvokedMethodListener could be created using the afterTest method to reset 
all of the Mocks and Spies after each test invocation.

One nice thing that would help would be a Mockito.resetAll() method. Looks like 
it has been discussed before.
http://code.google.com/p/mockito/issues/detail?id=119

It doesn't appear that TestNG has a listner interface for the equivalent of 
@BeforeClass. 
http://groups.google.com/group/testng-dev/browse_thread/thread/d04fb64bf8c156a9/

I looked at IConfigurationListener but it only invokes one of its on* methods 
for each @Before* annotated method defined in the test class. So if you don't 
have any @Before* methods define in the test class this listener will not be 
invoked. So it does not appear to be a good canidate for initializing all of 
the mocks.
IObjectFactory could be used to call MockitoAnnotations.initMocks(object); 
every time a unit test class is created. Looks like a possible problem with 
this solution is that can only have 1 IObjectFactory defined, so if someone 
already created one of these this object factory may cause problems.

Another issue that I have noticed with the TestNG @Listeners is that if you 
define it in 1 test class it will apply to ALL test classes which may not be 
what a person wants. The reason is that Listeners are global.
http://groups.google.com/group/testng-users/browse_thread/thread/fd286e07f3ec3c9
1/b344a76c389c4bc0?
http://testng.org/doc/documentation-main.html#listeners-testng-xml

One nice thing about using a listener is that you could create a jar file with 
your listeners and it automatically gets applied to all tests as long as you ad 
the jar to your classpath.
http://testng.org/doc/documentation-main.html#listeners-service-loader

With the issues I have found with listeners the best solution in my case is to 
implement an abstract test class that all other unit tests will extend from and 
perform the setup and reset like you listed above.

public abstract class BaseMockitoTestCase {
    @BeforeClass
    public void initMocks() {
        MockitoAnnotations.initMocks(this);
    }

    @AfterMethod
    public void resetMocks() {
        for (Field field : this.getClass().getDeclaredFields()) {
            Object property = Whitebox.getInternalState(this, field.getName());
            if (new MockUtil().isMock(property)) {
                Mockito.reset(property);
            }
        }
    }
}

Original comment by kdomb...@gmail.com on 13 Jan 2012 at 11:14

GoogleCodeExporter commented 9 years ago
@Cedric I still have some issues to notify TestNG a test failure after the test 
has passed (successfully or not) for example  when Mockito statie is invalid. 
Is it even possible from the listener to notify errors after the test method 
has run ?

Original comment by brice.du...@gmail.com on 30 Jan 2012 at 10:48

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Hello
I want to share my expirience about implementation of MockitoTestNGInitializer 
which implements IInvokedMethodListener and allow to use Mockito annotation 
with TestNG.

What it does:

1. Initialize mock fields only once. It does not matter if you have 
configuration method(@BeforeClass @BeforeMethod) or not.
2. Check Mockito usage only after the test methods
3. Reset mocks after the test methods.

https://github.com/ksmster/mockitong/blob/master/src/main/java/org/mockitong/Moc
kitoTestNGInitializer.java

@Brice can you share your usecase more detail? I want to test it against my 
implementation.

Original comment by ksms...@gmail.com on 4 Feb 2012 at 1:06

Attachments:

GoogleCodeExporter commented 9 years ago
Could you repost it also on testng-users mailing list? Maybe some people would 
be happy to contribute or at least share some thoughts.

Original comment by kaczanow...@gmail.com on 7 Feb 2012 at 10:19

GoogleCodeExporter commented 9 years ago
Hi Tomek,
 I think the issue system is more relevant on that matter. It won't be lost in the long list of threads.

Also, here's the last status of my implementation (minus 6 unit tests class), 
it's like 2 weeks old at least.
I have 2 remarks on the current implementation :
 - There is one problem for sure, I need to reset/recreate Captors between each test method, though it's easily fixable.
 - I still have unwanted behaviors when running a TestNG suite in the IDE (maybe under other environment too), it's like errors are not reported as expected, that should be investigated.

This code is not yet in the repo due to other considerations. But don't worry 
it will eventually find it's place.

@Sergey If you have deep understanding of TestNG, I'll be happy to discuss it ;)

Original comment by brice.du...@gmail.com on 7 Feb 2012 at 10:47

Attachments:

GoogleCodeExporter commented 9 years ago
Hi Brice

I tested your implementation and found that one test is failing
Hire is it :

@Listeners(MockitoTestNGListener.class)
public class ConfigurationMethodTest
{
   @Mock
   private ObjectManipulator objectManipulator;

   private RealWorldClass realWorldClass;

   @BeforeMethod
   public void prepareTestObject()
   {
      when(objectManipulator.getLimit()).thenReturn(1999);
      realWorldClass = new RealWorldClass(objectManipulator, 2012);
   }

   @Test
   public void shouldBeAbleToConfigureMockInTestngConfigurationMethod() throws IOException
   {
      assertEquals(objectManipulator.getLimit(), 1999);
   }
}

It fail because you reset mock's after all methods including configuration 
method.

After changing 

public void afterInvocation(IInvokedMethod method, ITestResult testResult)
   {
      if (method.isTestMethod())
      {
         resetMocks(testResult.getInstance());
         Mockito.validateMockitoUsage();
      }
   }

in your code all works like a sharm

Original comment by ksms...@gmail.com on 8 Feb 2012 at 10:56

GoogleCodeExporter commented 9 years ago
Oh ok, didn't think the invocation was related to every method including the 
afters and befores.
Thanx Sergey, I will update my code. with your unit test.

Good catch!

Original comment by brice.du...@gmail.com on 8 Feb 2012 at 1:11

GoogleCodeExporter commented 9 years ago

Original comment by brice.du...@gmail.com on 10 Feb 2012 at 8:49

GoogleCodeExporter commented 9 years ago
It would be nice to change 
from

if (MockitoTestNGListener.class == listenerClass) {

to

if (MockitoTestNGListener.class.isAssignableFrom(listenerClass))

in hasMockitoTestNGListener method
for the case if some one decide to extend this listener.

Original comment by ksms...@gmail.com on 11 Feb 2012 at 9:28

GoogleCodeExporter commented 9 years ago
Hi Sergey,

I don't agree, if someone want to craft his own listener he shouldn't use 
inheritance. Either design for inheritance with an open / close principle, or 
just don't expose class internals. And the current design is not designed for 
inheritance.

Original comment by brice.du...@gmail.com on 12 Feb 2012 at 5:28

GoogleCodeExporter commented 9 years ago
I have a base test case that all of my other test cases extend from. I would 
like to add this listener to the base test case only so it would be helpful to 
search the entire class hierarchy when searching for the MockitoTestNGListener.

private Set<Object> allTestInstancesWithMockitoListener(ITestNGMethod[] 
testMethods) {
    Set<Object> testInstancesWithMockitoListener = new HashSet<Object>();
    for (Object testInstance : allTestInstances(testMethods)) {
        if (containsMockitoTestNGListener(testInstance.getClass())) {
            testInstancesWithMockitoListener.add(testInstance);
        }
    }
    return testInstancesWithMockitoListener;
}

private boolean containsMockitoTestNGListener(Class clazz) {
    if (clazz == null) {
        return false;
    }
    return hasMockitoTestNGListener(clazz) || containsMockitoTestNGListener(clazz.getSuperclass());
}

private boolean hasMockitoTestNGListener(Class clazz) {
    Annotation listeners = clazz.getAnnotation(Listeners.class);
    return listeners != null && containsMockitoTestNGListener((Listeners) listeners);
}

private boolean containsMockitoTestNGListener(Listeners listeners) {
    for (Class<? extends ITestNGListener> listenerClass : listeners.value()) {
        if (MockitoTestNGListener.class == listenerClass) {
            return true;
        }
    }
    return false;
}

Also should Captors be reset or is this a Mockito anti pattern?

Original comment by kdomb...@gmail.com on 13 Feb 2012 at 10:27

GoogleCodeExporter commented 9 years ago
Hi,

Yes, captors should be reset betzeen test invocqtions if using TestNG, see one 
of my previous comment. I will refactor some Mockito internals for that, in 
order to avoid code duplication.

By the way I took another approach, which now provides a proper error reporting 
when mocks can't be instantiated or when Mockito's state is incorrect. It still 
is a work around as TestNG doesn't provide a proper way that I know of to place 
callbacks before class configuration methods.

Also your case -with super test class- is being take care of. Thx anyway for 
the reporting :)

At the exception of captor stuff, I'll will post the current implementation 
tomorrow.

Original comment by brice.du...@gmail.com on 13 Feb 2012 at 11:16

GoogleCodeExporter commented 9 years ago
>if (MockitoTestNGListener.class == listenerClass) {
>to
>if (MockitoTestNGListener.class.isAssignableFrom(listenerClass))

I think it is a reasonable request. It makes the design more flexible for the 
use cases we don't know yet and I don't see any problem it can lead to.

Original comment by szcze...@gmail.com on 14 Feb 2012 at 7:16

GoogleCodeExporter commented 9 years ago
FYI. What was my motivation.

I want to deploy "mock" REST services in to the Jetty server on test start.
So I write my own ITestListener and implement server starting.
But here is the trick: the order of the listeners may be different
( Eclipse vs maven). 
exmple :
@Listeners(value = {EverrestJetty.class, MockitoTestNGListener.class})

My first idea was to extend MockitoTestNGListener call super on start and
after do what I want. But if I use MockitoTestNGListener originally writed by 
Brice it wouldn't work. So what was my motivation.

For now I use other approach to deploy services : with help of
 IInvokedMethodListener I made it just before the test, so it's guarantee what all
services will be initialized by MockitoTestNGListener. 

https://github.com/ksmster/everrest-assured/blob/master/src/main/java/org/everre
st/assured/EverrestJetty.java

So I assume that there may be other situations where need extend 
MockitoTestNGListener.
IMO better to give people a possible of choice 

Original comment by ksms...@gmail.com on 14 Feb 2012 at 7:43

GoogleCodeExporter commented 9 years ago
One can argue that you can always decorate MockitoListener should you need to. 
However, that's slightly less hackable, a bit more code to maintain, a bit more 
dependency on the TestNG. In general I agree that in this particular instance 
more flexibility on our side is desired.

@Brice - you wanted to have testNG support directly in mockito project - there 
you go - now you have me messing up with your design :)

I don't know test NG listener API well enough but it feels awkward that we need 
to perform such matching in the first place.

Original comment by szcze...@gmail.com on 14 Feb 2012 at 8:05

GoogleCodeExporter commented 9 years ago
@Szczepan Usually I would agree with that, however in this case I don't. I'd 
like to keep a strong position on this point, either design for extension or 
don't allow it. In my head on this matter I would rather favor delegation over 
inheritance.

Anyway as I said I changed my approach and only interact in the boundaries of 
IInvokedMethodListener TestNG listener interface. I took this opportunity to 
split the code as well.

> I don't know test NG listener API well enough but it feels awkward that we 
need to perform such matching in the first place.
Agreed. But when running a suite, if only one test has this listener defined 
then all tests are affected :/

AS promised here is the current implementation. To early adopters, keep in my 
this code is not yet polished. And might miss things. @Captor reinitialization 
between tests is not yet done.

Original comment by brice.du...@gmail.com on 14 Feb 2012 at 10:04

Attachments:

GoogleCodeExporter commented 9 years ago
As I said,  I found the other solution(not to extend) , so you may skip my last 
request.

Original comment by ksms...@gmail.com on 14 Feb 2012 at 10:13

GoogleCodeExporter commented 9 years ago
Oh ok.
By the way, could you try the newer code ?

Original comment by brice.du...@gmail.com on 14 Feb 2012 at 10:39

GoogleCodeExporter commented 9 years ago
All tests passed.

Original comment by ksms...@gmail.com on 14 Feb 2012 at 10:50

GoogleCodeExporter commented 9 years ago
cool that's great. thx

Original comment by brice.du...@gmail.com on 14 Feb 2012 at 10:55

GoogleCodeExporter commented 9 years ago
>either design for extension or don't allow it

I think it's one of the most horrible guidelines for API designers :) It's the 
first symptom of the frameworkitis.

The fundamental problem is that it leads to designs that are locked for the use 
cases the authors don't foresee. So it eventually leads to inflexible libraries 
that are harder to work with *and* harder to maintain as the authors usually 
have to 'unlock' the designs sooner or later.

That approach could be possible in the context where one can predict and 
analyze all potential use cases. When you build a library you cannot do that, 
I'm afraid. But even if you could, I wouldn't make the indecent use case 
impossible... I would simply make the decent use cases the baseline and focus 
the API to solve them best.

Here's my set of rules for the API design:

1. All public api is behind interfaces
2. The public api that cannot be an interface (for example static entry-point 
class like Mockito; external API integrators like MockitoJUnitRunner) should:
-not be a final class
-have at least a 'protected' constructor (or no constructor at all)

At least this is the philosophy of Mockito and I'm sticking to it. I'm not very 
keen on reviewing testng listener stuff but bear in mind that if I ever do that 
I might change the design according to the principles above without notice. 
You've been warned :)

Damn, I need to blog about it.

Original comment by szcze...@gmail.com on 14 Feb 2012 at 11:07

GoogleCodeExporter commented 9 years ago
Good points.

However I'd like to mention that MockitoTestNGListener implements an interface 
already, and the code is already split with TestNG use cases in mind, imho the 
way it's done now makes little sense to me to write this line ""if 
(MockitoTestNGListener.class.isAssignableFrom(listenerClass))".

I definitely agree with you on the flexibility argument, but still I think it's 
a leverage for API designers to make a good API ;)
Anyway I was saying that because I have often seen people / project that hack 
their way too much to the point of rewriting "legacy" code. While a good API 
can prevent that much much more. That being said, I'm not at all for locked 
designs, e.g. with finals.
Imho "delegation over inheritance" and the granularity of interfaces provides 
us a fair alternative.

For the current matter I think it would ok to expose some methods as protected 
instead in MockitoTestNGListener.

By the way this interesting discussion reminded me a small article from Joshua 
Bloch on InfoQ on API design : this definitely something i should re-read 
regularly :P
http://www.infoq.com/articles/API-Design-Joshua-Bloch

Original comment by brice.du...@gmail.com on 14 Feb 2012 at 2:48

GoogleCodeExporter commented 9 years ago
What often happens with non-final Java classes which were not designed for 
extension through inheritance is this:

java.util.Stack: extends java.util.Vector
java.util.Properties: extends java.util.Hashtable

Had Vector and Hashtable been declared final (as they should), the above 
mistakes wouldn't have happened. More than that, the "Stack" and "Properties" 
classes would most likely have been designed correctly, using *delegation* 
instead of inheritance. 
So, as it turns out, using "final" only prevents extension *by subclassing*, 
but not other forms of extension such as delegation (BTW, this is what the GoF 
meant by "favor composition over inheritance").

Original comment by rliesenf...@gmail.com on 14 Feb 2012 at 3:20

GoogleCodeExporter commented 9 years ago
>Had Vector and Hashtable been declared final (as they should), the above 
mistakes wouldn't have happened

I don't think it's true. One could just remove the final modifier and subclass 
Vector anyway. Had Vector have a javadoc comment "don't subclass me foolishly" 
wouldn't help, either.

>BTW, this is what the GoF meant by "favor composition over inheritance"

"favor composition over inheritance" is a great principle but finalizing 
classes is not the way to teach it.

You can finalize your classes as you wish in a product that you ship to the 
client. However, when you are building a library with a java api you have to be 
very careful about finalizing your public api (example: 
http://alexruiz.developerblogs.com/?p=257).

If you're keen on real examples from developing libraries

BTW. I hate class inheritance. I hate locked design and inflexible frameworks 
even more.

Hope that helps :)

Original comment by szcze...@gmail.com on 14 Feb 2012 at 3:56

GoogleCodeExporter commented 9 years ago
Hmm... The Fest Assert "Assertions" class was made non-final and got a 
protected constructor, but it *still* isn't extensible since it has no 
overridable (protected) methods at all (all methods are static, and they don't 
delegate to any replaceable object either). Am I missing something?

By the same token, the org.mockito.Mockito class cannot be extended through 
subclassing even though it is not final. For that to be possible, methods like 

public static <T> T mock(Class<T> classToMock, MockSettings mockSettings) {
    return MOCKITO_CORE.mock(classToMock, mockSettings);
}

would have to instead delegate to an overridable instance method in a "Mockito" 
or "MockitoCore" instance which could be set by user code (or better yet, an 
instance of a Java interface, accessible through a SPI). Or is there another 
way?

Original comment by rliesenf...@gmail.com on 15 Feb 2012 at 1:02

GoogleCodeExporter commented 9 years ago
In FEST-Assert 1.4 classes like BooleanAssert are not final and with protected 
or public methods.

Also Mockito class is made of static calls, such definitions are not 
"overridable" by language design. That's a trade off if you want Static Factory 
Methods in your library. Though on this very specific matter, I gather that 
Szczepan didn't meant overriding methods, but more "extending" Mockito (e.g. 
importing ExtendedMockito.recordMockInteractions, would also make available 
Mockito.mock, as well as Matchers.eq).

Finaly about the SPI, mockito already has some extension points and we are 
working on on another as we speak.

Cheers.

Original comment by brice.du...@gmail.com on 15 Feb 2012 at 2:00

GoogleCodeExporter commented 9 years ago
Making Assertions class non-final has a huge benefit for the users: one can 
create MyAssertions class that extends Assertions and have a single point of 
entry to all assertions via MyAssertions. This is the use case the FEST guys 
didn't foresee initially and they thought that finalizing Assertions is a cool 
idea (because they followed 'design for extension or lock it' principle :)

Original comment by szcze...@gmail.com on 15 Feb 2012 at 4:00

GoogleCodeExporter commented 9 years ago
Oh! No disrespect to the FEST team and Alex - the product is awesome!

Original comment by szcze...@gmail.com on 15 Feb 2012 at 4:01

GoogleCodeExporter commented 9 years ago
I don't want to drag on this discussion here (it's off-topic, after all), but 
my point is that simply making a "static facade" class such as "Assertions" or 
"Mockito" non-final is not enough to make it extensible; you would need to 
change the static methods so they delegate the actual job to an *instance* 
method of a non-final class (which may be the same, or another), *and* also 
provide a way for the user to replace the actual instance used by the static 
facade class. This can be done (hasn't been in FEST or Mockito, though), but 
it's arguable whether it's the best solution or not. The "MyAssertions" example 
would not work as stated, I believe - you could just as well make it an 
unrelated class, since there are no inherited methods anyway, and still two 
separate static imports. (And of course, FEST is a great tool. :)

Original comment by rliesenf...@gmail.com on 15 Feb 2012 at 5:31

GoogleCodeExporter commented 9 years ago
In nowadays IDE, if you import Mockito they will provide easier access to the 
static method of the upper class. Try to only import Mockito and try 
auto-completion with 'anyString()' which is declared in Matchers!

> making a "static facade" class such as "Assertions" or "Mockito" non-final is 
not enough to make it extensible
Of course. Also the target of these extension points differ, you won't do the 
same thing through subclassing than with a ServiceLoader. That's why Mockito 
has other extension points and might have other.

Original comment by brice.du...@gmail.com on 15 Feb 2012 at 6:39

GoogleCodeExporter commented 9 years ago
Thanks Brice, you're right! Only one static import is needed, that's what I was 
missing here. And, having a subclass for a static facade allows one to take 
advantage of static method *overloading* (OK) and *hiding* (not so good). With 
IDEA, however, the automatic insertion of static imports is not very smooth in 
this case.

Original comment by rliesenf...@gmail.com on 15 Feb 2012 at 7:26

GoogleCodeExporter commented 9 years ago
Yes IDEA static imports needs improvement. I agree but if you specify the 
correct qualified name then it just work, so it's an acceptable trade off.

Original comment by brice.du...@gmail.com on 15 Feb 2012 at 8:03

GoogleCodeExporter commented 9 years ago
Hello  brice

Sorry to open old discussion but I have trouble with your last implementation.

The core of the problem what I can't be sure when mock object will be created. 
So I can't get for sure the 
instance of the mock before the test method call in my other listener. Because 
some times it call before MockitoTestNGListener
some times after. 

I propose to roll-back to your first implementation when mock initialized in 
one concrete place just on onExecutionStart.
http://code.google.com/p/mockito/issues/attachmentText?id=304&aid=3040012000&nam
e=MockitoTestNGListener.java&token=L6Y7srucue64X-O7G8WUKkXRjHs%3A1330336608664

Original comment by ksms...@gmail.com on 27 Feb 2012 at 10:12

GoogleCodeExporter commented 9 years ago
Hi, thx for getting back to us.

Short answer : No, but there's other options!

No : Here's why :
-----------------
Unfortunately, the reason I changed this was to be able to report Mock, Spy or 
@InjectMocks handling errors to the user, otherwise the user just see the test 
failing *without* reason.

I already don't like the way I'm handling the "initialized instances" in the 
last iteration, it feels unnatural.
And thinking louldy we could use the previous implementation using "onStart", 
and in order to keep the reporting we would store the exceptions to rethrow 
them for the right instance before each method execution, but it would feel 
even more unnatural or weird if not wrong!

Design wise I don't like at all the fact that the listeners can change the 
state of the observed object, this is breaking the Observer pattern contract. 
Listeners should not be coupled in any way between each other. And that's 
exactly what you/we are experimenting right now.
I would definitely prefer another abstraction that would allow to register some 
code at specific point. Eventually the ordering of these extension point would 
be written explicitly. But that's another story, I think I'll need to talk to 
Cedric about that.

The other option :
------------------
Instead it is possible to make extension points around the Mockito Listener. 
Where you would register your own actions before and after our own mockito 
actions.

What do you think ?

@Szczepan Right now I better understand your reticence to block inheritence ;)

Original comment by brice.du...@gmail.com on 27 Feb 2012 at 11:17

GoogleCodeExporter commented 9 years ago
I mast admit Brice you was right.
Extending MockitoTestNGListener was wrong way.

The situation become unpredictable when you have  
tests with MockitoTestNGListener and with listener that extend 
MockitoTestNGListener. I fail to find what was wrong but they initialize "mocks"
one before another in unexpected order.
So I decide to not to extend MockitoTestNGListener.
Thanks for your advices.

Original comment by ksms...@gmail.com on 2 Mar 2012 at 8:25

GoogleCodeExporter commented 9 years ago
Well it is still possible to find a solution after the initial release.

> The situation become unpredictable when you have  
tests with MockitoTestNGListener and with listener that extend 
MockitoTestNGListener. I fail to find what was wrong but they initialize "mocks"
one before another in unexpected order.

I'm the middle of something right now, if you have some spare time to dig out a 
solution for this problem, that would be helpful :)

Original comment by brice.du...@gmail.com on 2 Mar 2012 at 9:34

GoogleCodeExporter commented 9 years ago
Fixed in revision 50afaeba43a7c108a781f53147272c0df051711f

Original comment by brice.du...@gmail.com on 30 Mar 2012 at 5:08

GoogleCodeExporter commented 9 years ago

Original comment by szcze...@gmail.com on 13 May 2012 at 3:37

GoogleCodeExporter commented 9 years ago

Original comment by szcze...@gmail.com on 3 Jun 2012 at 2:06

GoogleCodeExporter commented 9 years ago
Changing the status because we don't yet ship the listener. We need to make it 
happen soon :)

Original comment by szcze...@gmail.com on 3 Jun 2012 at 6:35

GoogleCodeExporter commented 9 years ago

Original comment by brice.du...@gmail.com on 4 Jul 2012 at 1:00

GoogleCodeExporter commented 9 years ago
Hey guys, I'm confused.  Is this fixed in the 1.9.5-rc1 release?  I'm not 
seeing the 3 classes that are part of the GitHub MockitoNG project (the one 
that defines the listener and the before/after classes).  Thanks.

If it will not be in the 1.9.5 release.  What release will it be in?  Thanks.

Original comment by adam.n.g...@gmail.com on 18 Jul 2012 at 6:50

GoogleCodeExporter commented 9 years ago
Hi,
Actually the integration with TestNG, won't be part of mockito, it will be 
released separately. Though we didn't have time to take care of the build 
scripts. Plus we have specific issues linked to our IntelliJ workspace.

And MockitoNG isn't our project.

Cheers,
Brice

Original comment by brice.du...@gmail.com on 19 Jul 2012 at 10:10