kaisalmon / mockito

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

Answers.THROWS_EXCEPTION #346

Open GoogleCodeExporter opened 8 years ago

GoogleCodeExporter commented 8 years ago
I would like Mockito to throw a NotMockedException when I attempt to invoke a 
method that has not been mocked instead of returning empty list, 0, false etc. 
One caveat to this is that method void should not throw an exception.

eg:
public class FooTest {
   @Mock(answer=Answers.THROWS_EXCEPTION)
   private Foo mockFoo;
   ...
}

public enum Answers {
   ...
   THROWS_EXCEPTION(new ThrowsException());
   ...
}

public class ThrowsException implements Answer<Object> {
   public Object answer(InvocationOnMock invocation) throws Throwable {
      Method method = invocation.getMethod();
      if (void.class.equals(method.getReturnType())) {
         return null;
      }
      throw new NotMockedException(invocation);
   }
}

Original issue reported on code.google.com by ukla...@gmail.com on 28 May 2012 at 8:45

GoogleCodeExporter commented 8 years ago

Original comment by brice.du...@gmail.com on 29 May 2012 at 8:17

GoogleCodeExporter commented 8 years ago
I'd rather solve the general problem and then this feature is not really 
needed. Cheers!

Original comment by szcze...@gmail.com on 29 May 2012 at 5:19

GoogleCodeExporter commented 8 years ago
This feature would be really awesome because it could allow us to create 
"strict mocks".

Original comment by jsebfra...@gmail.com on 24 Apr 2013 at 9:38

GoogleCodeExporter commented 8 years ago

Original comment by brice.du...@gmail.com on 29 Apr 2013 at 3:24

GoogleCodeExporter commented 8 years ago
Issue 411 has been merged into this issue.

Original comment by brice.du...@gmail.com on 29 Apr 2013 at 4:10

GoogleCodeExporter commented 8 years ago
I feel strongly that this should be built in to Mockito rather than requiring a 
custom answer. In some circumstances, I really want a strict mock. I don't want 
0 or an empty list returned from my mock. I want to fail fast with an exception 
rather than having to track down the default return value.

Original comment by ukla...@gmail.com on 29 Jan 2014 at 10:26

GoogleCodeExporter commented 8 years ago
I'm ok to add this new enum but it cannot throw only for non-voids. It needs to 
throw for every invocation, otherwise it feels inconsistent.

Original comment by szcze...@gmail.com on 12 Apr 2014 at 7:36

GoogleCodeExporter commented 8 years ago
Well, that's better than nothing but it will cause unnecessary stubbing of void 
methods. Any chance you could add two answers (one that doesn't throw exception 
for void?)

Original comment by ukla...@gmail.com on 15 Apr 2014 at 10:35

GoogleCodeExporter commented 8 years ago
Typically, for void methods you verify() that they were invoked rather than 
stubbing the method.

Original comment by ukla...@gmail.com on 15 Apr 2014 at 10:38

GoogleCodeExporter commented 8 years ago
I agree with Szczepan, this doesn't feel consistent. You can verify other 
methods too.

Either way Throws_Excpetion feel to technical if the goal is to have strict 
mocks. It could be named to something like FAILS_IF_NOT_STUBBED, ideas 
welcome...

Original comment by brice.du...@gmail.com on 21 Apr 2014 at 4:38

GoogleCodeExporter commented 8 years ago
I realise that I can verify any mockito method. I want to prevent the case 
where a test can pass when a default answer is returned from mockito (0 or 
empty list). I really hate the default answers to be honest. 

I'd prefer not to stub every void method to achieve this, but I can live with 
it if you feel it's inconsistent. 

Original comment by ukla...@gmail.com on 21 Apr 2014 at 5:09

GoogleCodeExporter commented 8 years ago
I think I'd be ok to add multiple Answers, but we need good names (e.g. 
THROWS_EXCEPTION, THROWS_ON_ASKING, THROWS_ON_TELLING).

Original comment by szcze...@gmail.com on 21 Apr 2014 at 5:18

GoogleCodeExporter commented 8 years ago
[deleted comment]
GoogleCodeExporter commented 8 years ago
[deleted comment]
GoogleCodeExporter commented 8 years ago
[deleted comment]
GoogleCodeExporter commented 8 years ago
A few suggestions. I think wer agree on THROWS_EXCEPTION for the answer which 
always throws.

For the case which does throws only for a non-void return type:
 - THROWS_EXCEPTION_ON_RESULT
 - THROWS_EXCEPTION_ON_RETURN
 - THROWS_EXCEPTION_ON_REPLY
 - THROWS_EXCEPTION_ON_ANSWER
 - THROWS_EXCEPTION_ON_RESPONSE
 - THROWS_EXCEPTION_EXCEPT_VOID

Here's some more inspiration
 - http://thesaurus.com/browse/return
 - http://thesaurus.com/browse/response

Original comment by ukla...@gmail.com on 22 Apr 2014 at 9:35

GoogleCodeExporter commented 8 years ago
What about REPORTS_UNSTUBBED_CALLS ?

The different cases you show (void, reply, etc.) should be stuff to put in a 
builder, however this would then forbid any usage in annotations.

Original comment by brice.du...@gmail.com on 22 Apr 2014 at 10:16

GoogleCodeExporter commented 8 years ago
[deleted comment]
GoogleCodeExporter commented 8 years ago
Not sure I get you, all of the names I gave (above) were suggestions for the 
same thing. 

Here's what I want to do.

@RunWith(MockitoJUnitRunner.class)
public class MyTest {
   public static interface MockMe {
      void voidMethod();
      int returnMethod();
   }

   @Mock(answer=Answers.THROWS_EXCEPTION)
   private MockMe alwaysThrow;

   // I don't car if it's called THROWS_EXCEPTION_ON_RETURN or THROWS_EXCEPTION_EXCEPT_VOID etc
   @Mock(answer=Answers.THROWS_EXCEPTION_ON_RESULT)
   private MockMe throwOnResult;

   @Test
   public void test() {
       throwOnResult.voidMethod(); // does not throw exception

       try {
          throwOnResult.returnMethod(); // throws exception
          fail();
       } catch (NotMockedException e) {}

       try {
          alwaysThrow.voidMethod(); // throws exception
          fail();
       } catch (NotMockedException e) {}

       try {
          alwaysThrow.returnMethod(); // throws exception
          fail();
       } catch (NotMockedException e) {}  
   }
}

Original comment by ukla...@gmail.com on 23 Apr 2014 at 12:19

GoogleCodeExporter commented 8 years ago
I think I may have a solution for this but I will have to check with my 
employer (Google) on how best to proceed.

Original comment by Rohan.Ta...@gmail.com on 30 Apr 2014 at 6:04

GoogleCodeExporter commented 8 years ago
So, my solution is this pull request: https://github.com/mockito/mockito/pull/54

I already have approval for this from the Open Source office at Google (I had 
to get approval before releasing the code change).

Any thoughts on this so far?

Original comment by Rohan.Ta...@gmail.com on 22 May 2014 at 10:20

GoogleCodeExporter commented 8 years ago
verify thet you don't have nested exception for toString invocation take a look 
on Issue 411 (https://code.google.com/p/mockito/issues/detail?id=411) for 
possible solution

Original comment by filippo....@gmail.com on 23 May 2014 at 8:36

GoogleCodeExporter commented 8 years ago
Sorry for the delayed reply; I've been busy recently and haven't had
the time or mental space to devote to a good answer.

FYI, David, you replied to the mockito-dev mailing list rather than
updating the Mockito issue directly, so I'm copying your email here so
that others who are not on the mockito-dev mailing list can see your
concerns.  (I apologise if that was intentional on your part.)

> From: David Wallace <...@...>
> Date: Thu, May 22, 2014 at 3:32 PM
> Subject: Re: [mockito-dev] Re: Issue 346 in mockito: Answers.THROWS_EXCEPTION
> To: mockito-dev@googlegroups.com

> I wish I'd weighed in on this earlier, but this feature seems to me
> to be completely unnecessary.

> I can't see what it will give you that using
> verifyNoMoreInteractions won't do.  This is the normal Mockito way
> of making sure that unexpected method calls don't happen.  I don't
> really think there's a need to have a second way of doing the same
> thing.

> What is the motivation, other than to "make it work more like EasyMock"?

I do use verifyNoMoreInteractions() in my tests; it is useful at
times.  I also currently use Answers.RETURNS_SMART_NULLS quite
extensively, which I find to be more useful than
Answers.RETURNS_DEFAULTS since I get an indication of a problem with
stubbing much earlier (than where verifyNoMoreInteractions() would
normally be called), if null would have been returned due to an
unstubbed method.

You probably already know this but I'm repeating
http://docs.mockito.googlecode.com/hg/org/mockito/Mockito.html#RETURNS_SMART_NUL
LS
here since it is useful for this discussion:

> Optional Answer to be used with mock(Class, Answer).

> Answer can be used to define the return values of unstubbed invocations.

> This implementation can be helpful when working with legacy
> code. Unstubbed methods often return null. If your code uses the
> object returned by an unstubbed call you get a
> NullPointerException. This implementation of Answer returns
> SmartNull instead of null. SmartNull gives nicer exception message
> than NPE because it points out the line where unstubbed method was
> called. You just click on the stack trace.

> ReturnsSmartNulls first tries to return ordinary return values (see
> ReturnsMoreEmptyValues) then it tries to return SmartNull. If the
> return type is final then plain null is returned.

I like the "nicer exception message" of the ReturnsSmartNulls answer
and I like being able to click on the stack trace in Eclipse.  I'm
less interested in the ReturnsSmartNulls class delegating to
ReturnsMoreEmptyValues (which in turn delegates to ReturnsEmptyValues)
since I'd much rather be explicit in what methods get stubbed with
which parameters (possibly loosely via any*() specifications or
similar), rather than relying on the default of empty values.

I find this behaviour becomes more useful when the tests use many
mocks and when constructors and methods can take many parameters (due
to complex SUT (System Under Test) code outside of my immediate
control).  i.e. Forgetting to stub a method, even with
ReturnsSmartNulls, can lead to a lengthy debugging session trying to
figure out why a test isn't working only to find that an empty
(non-null) value was being returned by default because it wasn't
stubbed.

Mockito allows precisely defined when() and verify() specifications or
looser specifications via the any*() methods and suchlike. The tests
used by the larger team that I am on, tend to use loose stubbing and
precisely defined verifications, but it varies.  I rewrote a LOT of
tests at work to use Mockito instead of EasyMock, and I have seen a
few different patterns.  In any case, your use of Mockito is probably
different to mine already.

I hope that answers your question about motivation.  As to your
comment about making Mockito work more like EasyMock, I'm not a fan of
EasyMock and there are things about it (e.g. the explicit replay()
step) that bug me.  I can see how Mockito could be used like EasyMock
though but that's not my goal.

Regards,

Rohan

Original comment by Rohan.Ta...@gmail.com on 5 Jul 2014 at 10:08