Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

New "then" method for BDD-style interaction testing #38

Merged
merged 4 commits into from

4 participants

@lpandzic

New feature in BddMockito for BDD then part of the test which allows mock behavior verification by using Mockito#verify(Object).
For example we have classes Person and Bike:

Bike bike = new Bike();
Person person = mock(Person.class);

and the following interaction

person.ride(bike);
person.ride(bike);

Current (non BDD) behavior verification would go like:

verify(person, times(2)).ride(bike);

This feature introduces the following way of verification:

then(person).should(times(2)).ride(bike);
@coveralls

Coverage Status

Coverage remained the same when pulling 62f7edd on lpandzic:bdd-then into 8293511 on mockito:master.

@bric3
Owner

I like this extension :)

What do you think of using verify too as an alias of should. Like you wrote it'll read this way

then(person).verify(times(2)).ride(bike);

Thx for the PR

@lpandzic

My pleasure.

I think the verify after then might hurt the fluency since it suggests that the person is the one verifying, but to the accustomed users it will look familiar so it might be a compromise solution for transition. I'll add it because it presents more flexibility and leaves to the user to decide which one is better and better suits them.
I need to add some javadoc and that should be it. Any opinions on the tests, should more be added?

@bric3
Owner

One last thing, could you add an javadoc item in the Mockito class. It's somehow our documentation entry point.

@coveralls

Coverage Status

Coverage remained the same when pulling 25ce710 on lpandzic:bdd-then into 8293511 on mockito:master.

@bric3 bric3 merged commit e9d91e0 into from
@lpandzic
@lpandzic lpandzic deleted the branch
@bric3
Owner

Some work is still needed regarding JDK8. But it may be easier than expected, unless the community has missed something.

Also release scripts need to be enhanced and extended to the test-ng support.
Plus many stuff I may have forgot.

By the way could you give your feedback on this PR #9, I'm not yet sure how to integrate this feature. TIA

@bric3
Owner

Hey I've been playing around a bit with your feature. I agree with you then(person).verify(times(2)).ride(bike); doesn't read well.

I'm no more inclined to add the verify alias. But if you want to keep it, what do you think of writing it this way with a no-op then(), for exemple :

then().verify(person, times(2)).ride(bike);
@lpandzic

By the way could you give your feedback on this PR #9, I'm not yet sure how to integrate this feature. TIA

I've just seen it is related to an accepted issue on the project's site on google code. I'll take a look into it when I get some time.

I'm no more inclined to add the verify alias. But if you want to keep it, what do you think of writing it this way with a no-op then(), for exemple :

I'm also in favor of removing it.

@bric3
Owner

The thing is the JSR 305 is now part of the JDK8 that could be interesting to bring at some point support for that.

@lpandzic

Sorry I didn't have any time to look at the PR, I'll try to take a look at it next week.

@lpandzic

By the way could you give your feedback on this PR #9, I'm not yet sure how to integrate this feature. TIA

I've looked at the pull request and I'm not sure I understand what the problem is. Is the main issue backwards compatibility and redesign of Invocation or integration process itself?

PS It seems this issue should be discussed somewhere, maybe best on the mockito google group?

@bric3
Owner

The comment on the PR is for that, please share your opinion or question there too :)
Thx for taking the time to take a look at it by the way :)

@lpandzic

When you get some free time please take a look at https://github.com/lpandzic/junit-bdd. I'm trying to gather some feedback from junit guys but none were given. Thanks!

@bric3
Owner

Hey can you share the link on the mailing list, so people can discuss there :)
Also I will post the link on twitter.
Anyway from just looking at the README page I find the api quite elegant.

@lpandzic

I will, thanks for feedback! :)

@szczepiq
Owner

Hey guys,

Did you consider this:

then(person, times(2)).ride(bike)
then(person).ride(bike)

@szczepiq szczepiq changed the title from - then feature for BddMockito to New 'then' method for more BDD-style interactions
@szczepiq szczepiq changed the title from New 'then' method for more BDD-style interactions to New 'then' method for BDD-style interaction testing
@szczepiq szczepiq changed the title from New 'then' method for BDD-style interaction testing to New "then" method for BDD-style interaction testing
@lpandzic

I find the original case more readable so I must say I'm against it. Is there any concrete case where it doesn't read well?

@bric3
Owner

I agree with Lovro, I find the original idea more readable.

@szczepiq
Owner

For example: then(task).should().completed()

then(mock).method() is more consistent with the original api

I really like your idea, though, I think it reads very nicely

@bric3
Owner

Yes that could work with aliases like

.shouldBe().validated()
.has().performed()
.has(times(2)).performed()
...
@lpandzic

I really like your idea, though, I think it reads very nicely

Thanks.

There is one more problematic case: timeout().
Maybe more aliases should be introduced?

@szczepiq
Owner

Hey guys,

I thought about this and I'm tempted to remove this new method from the API. I really like how it looks but on the other hand I don't want Mockito API to grow in tactical aliases that solve mini use cases. There is a room for this kind of aliases but I'd rather Mockito API to be simple and opinionated. Perhaps there should be a separate library called mockito-bdd? Also, it's pretty easy to add such aliases to any codebase.

Simpler API like "then(task).completed()" would be ok for me because it's truly an alias - it has exactly the same signature but the method name is different. Hence it does not complicate the API. However, it seems that you prefer more fluency - this could be different library that depends on mockito-core and has bunch of bdd aliases.

Lovro, I really like your coding and I'd love to get more contributions from you!

@szczepiq
Owner

BTW. Thoughts? I can always be convinced :)

@lpandzic

Lovro, I really like your coding and I'd love to get more contributions from you!

Thanks!

I must say that I still stand by my original pull request and it's design.

For example: then(task).should().completed()

I think that only listener methods can be problematic (onCompletion) with current implementation, normal methods usually don't go in past tense.

@lpandzic

But abandoning the should part and leaving only then wouldn't be bad for me, either. I just hope this gets released some time soon. :)

@szczepiq
Owner

I will release really soon. I wanted to close down this API change, actually, before the release :)

I totally respect your POV regarding should() method.

Options we have (I've conveniently removed options that don't work for me ;):

  1. Back out the feature from master. Possibly include more BDD aliases somewhere else.
  2. Change the feature to then(task).doStuff() If we do that, we will reserve the 'then' word/method and the original option won't be possible any more (even in a separate library).

I'd say we do 1) but I will concede to your votes. Lovro/Brice?

Thanks again for your input!

@szczepiq
Owner

Hey,

I thought about this and let's release it in the original shape, e.g. as it is now in the codebase :)

Do you mind improving the impl a little bit?

  1. Can you add info about this new feature from the Mockito main class?
  2. It would be better if 'Then' was an interface, and the implementing class not-public (protected or package-protected is fine).

I'll probably hide this feature from the codebase for the next release. However, once you get above items done, submit PR, we'll merge it and it should get automatically released. It will be nice test for the continuous deployment implementation. :)

@szczepiq
Owner

Ah, nevermind, let's just have it in the coming release. Do you mind doing the 1-2 items in a different PR? :)

Thanks!

@lpandzic

Hey, sorry for responding this late.

Do you mind doing the 1-2 items in a different PR?

Sure, what needs to be done? :)

@ghost
@lpandzic

Ok. I have a few comments:
2. Deprecation is not really needed. If I introduce an interface for Then in BDDMockito (and move/rename implementation) no client would be affected because the current Then class is a final class with all fields non visible hence all imports/usages would be valid after this change. But the more important question to me: what is the goal of this interface? Is it expected of clients to provide more implementations because that seems highly unlikely to me and just increases complexity unnecessary?

General direction is to deprecate the BDDMockito class?

@szczepiq
Owner
@lpandzic

I thought it had a private constructor but I see it has a public one, my mistake. So the deprecation is in order. I'll make a PR as soon as I get some free time on my hands :)

@szczepiq
Owner

Cool, no rush ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 28, 2014
  1. @lpandzic

    - added BDD(then) style verification of mock behavior

    lpandzic authored
    - added tests for this feature
Commits on Mar 13, 2014
  1. @lpandzic

    - added verify as alternative to should in BddMockito.Then

    lpandzic authored
    - added tests for new feature, added fluent tests for both options
  2. @lpandzic
Commits on Mar 18, 2014
  1. @lpandzic
This page is out of date. Refresh to see the latest.
View
88 src/org/mockito/BDDMockito.java
@@ -1,12 +1,13 @@
-/*
- * Copyright (c) 2007 Mockito contributors
- * This program is made available under the terms of the MIT License.
- */
+/*
+ * Copyright (c) 2007 Mockito contributors
+ * This program is made available under the terms of the MIT License.
+ */
package org.mockito;
import org.mockito.stubbing.Answer;
import org.mockito.stubbing.OngoingStubbing;
import org.mockito.stubbing.Stubber;
+import org.mockito.verification.VerificationMode;
/**
* Behavior Driven Development style of writing tests uses <b>//given //when //then</b> comments as fundamental parts of your test methods.
@@ -50,6 +51,31 @@
* assertEquals(failure, result);
* </code></pre>
* <p>
+ * For BDD style mock verification take a look at {@link Then} in action:
+ * <pre class="code"><code class="java">
+ * Bike bike = new Bike();
+ * Person person = mock(Person.class);
+ *
+ * public void shouldRideBikeTwice() {
+ *
+ * person.ride(bike);
+ * person.ride(bike);
+ *
+ * then(person).should(times(2)).ride(bike);
+ * }
+ * </code></pre>
+ *
+ * Syntax alternative to the last example:
+ * <pre class="code"><code class="java">
+ * public void shouldRideBikeTwice() {
+ *
+ * person.ride(bike);
+ * person.ride(bike);
+ *
+ * then(person).verify(times(2)).ride(bike);
+ * }
+ * </code></pre>
+ *
* One of the purposes of BDDMockito is also to show how to tailor the mocking syntax to a different programming style.
*
* @since 1.8.0
@@ -177,6 +203,60 @@ public BDDOngoingStubbingImpl(OngoingStubbing<T> ongoingStubbing) {
public static <T> BDDMyOngoingStubbing<T> given(T methodCall) {
return new BDDOngoingStubbingImpl<T>(Mockito.when(methodCall));
}
+
+ /**
+ * Bdd style verification of mock behavior.
+ *
+ * @see #verify(Object)
+ * @see #verify(Object, VerificationMode)
+ */
+ public static <T> Then<T> then(T mock) {
+ return new Then<T>(mock);
+ }
+
+ /**
+ * Provides fluent way of mock verification.
+ *
+ * @author Lovro Pandzic
+ * @param <T> type of the mock
+ */
+ public final static class Then<T> {
+
+ private final T mock;
+
+ public Then(T mock) {
+
+ this.mock = mock;
+ }
+
+ /**
+ * @see #verify(Object)
+ */
+ public T should() {
+ return Mockito.verify(mock);
+ }
+
+ /**
+ * @see #verify(Object, VerificationMode)
+ */
+ public T should(VerificationMode mode) {
+ return Mockito.verify(mock, mode);
+ }
+
+ /**
+ * @see #verify(Object)
+ */
+ public T verify() {
+ return Mockito.verify(mock);
+ }
+
+ /**
+ * @see #verify(Object, VerificationMode)
+ */
+ public T verify(VerificationMode mode) {
+ return Mockito.verify(mock, mode);
+ }
+ }
/**
* See original {@link Stubber}
View
9 src/org/mockito/Mockito.java
@@ -58,6 +58,7 @@
* <a href="#26">26. (**New**) Mocking details (Since 1.9.5)</a><br/>
* <a href="#27">27. (**New**) Delegate calls to real instance (Since 1.9.5)</a><br/>
* <a href="#28">28. (**New**) <code>MockMaker</code> API (Since 1.9.5)</a><br/>
+ * <a href="#29">29. (**New**) BDD style verification (Since 1.9.8)</a><br/>
* </b>
*
* <p>
@@ -927,6 +928,14 @@
* <p>For more details, motivations and examples please refer to
* the docs for {@link org.mockito.plugins.MockMaker}.
*
+ *
+ *
+ *
+ * <h3 id="29">29. (**New**) <a class="meaningful_link" href="#BDD_behavior_verification">BDD style verification</a> (Since 1.9.8)</h3>
+ *
+ * Enables Behavior Driven Development (BDD) style verification by starting verification with the BDD <b>then</b> keyword.
+ *
+ * For more information and an example see {@link BDDMockito}.
*/
@SuppressWarnings("unchecked")
public class Mockito extends Matchers {
View
121 test/org/mockitousage/customization/BDDMockitoTest.java
@@ -1,11 +1,13 @@
-/*
- * Copyright (c) 2007 Mockito contributors
- * This program is made available under the terms of the MIT License.
- */
+/*
+ * Copyright (c) 2007 Mockito contributors
+ * This program is made available under the terms of the MIT License.
+ */
package org.mockitousage.customization;
import org.junit.Test;
import org.mockito.Mock;
+import org.mockito.exceptions.misusing.NotAMockException;
+import org.mockito.exceptions.verification.WantedButNotInvoked;
import org.mockito.invocation.InvocationOnMock;
import org.mockito.stubbing.Answer;
import org.mockitousage.IMethods;
@@ -17,17 +19,17 @@
import static org.mockito.BDDMockito.*;
public class BDDMockitoTest extends TestBase {
-
+
@Mock IMethods mock;
-
+
@Test
public void shouldStub() throws Exception {
given(mock.simpleMethod("foo")).willReturn("bar");
-
+
assertEquals("bar", mock.simpleMethod("foo"));
assertEquals(null, mock.simpleMethod("whatever"));
}
-
+
@Test
public void shouldStubWithThrowable() throws Exception {
given(mock.simpleMethod("foo")).willThrow(new RuntimeException());
@@ -47,14 +49,14 @@ public void shouldStubWithThrowableClass() throws Exception {
fail();
} catch(RuntimeException e) {}
}
-
+
@Test
public void shouldStubWithAnswer() throws Exception {
given(mock.simpleMethod(anyString())).willAnswer(new Answer<String>() {
public String answer(InvocationOnMock invocation) throws Throwable {
return (String) invocation.getArguments()[0];
}});
-
+
assertEquals("foo", mock.simpleMethod("foo"));
}
@@ -73,7 +75,7 @@ public void shouldStubConsecutively() throws Exception {
given(mock.simpleMethod(anyString()))
.willReturn("foo")
.willReturn("bar");
-
+
assertEquals("foo", mock.simpleMethod("whatever"));
assertEquals("bar", mock.simpleMethod("whatever"));
}
@@ -87,11 +89,11 @@ public void shouldStubConsecutivelyWithCallRealMethod() throws Exception {
assertEquals("foo", mock.simpleMethod());
assertEquals(null, mock.simpleMethod());
}
-
+
@Test
public void shouldStubVoid() throws Exception {
willThrow(new RuntimeException()).given(mock).voidMethod();
-
+
try {
mock.voidMethod();
fail();
@@ -113,7 +115,7 @@ public void shouldStubVoidConsecutively() throws Exception {
willDoNothing()
.willThrow(new RuntimeException())
.given(mock).voidMethod();
-
+
mock.voidMethod();
try {
mock.voidMethod();
@@ -133,15 +135,15 @@ public void shouldStubVoidConsecutivelyWithExceptionClass() throws Exception {
fail();
} catch(IllegalArgumentException e) {}
}
-
+
@Test
public void shouldStubUsingDoReturnStyle() throws Exception {
willReturn("foo").given(mock).simpleMethod("bar");
-
+
assertEquals(null, mock.simpleMethod("boooo"));
assertEquals("foo", mock.simpleMethod("bar"));
}
-
+
@Test
public void shouldStubUsingDoAnswerStyle() throws Exception {
willAnswer(new Answer<String>() {
@@ -149,10 +151,10 @@ public String answer(InvocationOnMock invocation) throws Throwable {
return (String) invocation.getArguments()[0];
}})
.given(mock).simpleMethod(anyString());
-
+
assertEquals("foo", mock.simpleMethod("foo"));
}
-
+
class Dog {
public String bark() {
return "woof";
@@ -168,7 +170,7 @@ public void shouldStubByDelegatingToRealMethod() throws Exception {
//then
assertEquals("woof", dog.bark());
}
-
+
@Test
public void shouldStubByDelegatingToRealMethodUsingTypicalStubbingSyntax() throws Exception {
//given
@@ -187,4 +189,83 @@ public void shouldAllStubbedMockReferenceAccess() throws Exception {
assertEquals(expectedMock, returnedMock);
}
+
+ @Test(expected = NotAMockException.class)
+ public void shouldValidateMockWhenVerifying() {
+
+ then("notMock").should();
+ }
+
+ @Test(expected = NotAMockException.class)
+ public void shouldValidateMockWhenVerifyingWithExpectedNumberOfInvocations() {
+
+ then("notMock").should(times(19));
+ }
+
+ @Test(expected = NotAMockException.class)
+ public void shouldValidateMockWhenVerifyingNoMoreInteractions() {
+
+ then("notMock").should();
+ }
+
+ @Test(expected = WantedButNotInvoked.class)
+ public void shouldFailForExpectedBehaviorThatDidNotHappen() {
+
+ then(mock).should().booleanObjectReturningMethod();
+ }
+
+ @Test
+ public void shouldPassForExpectedBehaviorThatHappened() {
+
+ mock.booleanObjectReturningMethod();
+
+ then(mock).should().booleanObjectReturningMethod();
+ }
+
+ @Test(expected = WantedButNotInvoked.class)
+ public void shouldFailForExpectedBehaviorThatDidNotHappenWithVerify() {
+
+ then(mock).verify().booleanObjectReturningMethod();
+ }
+
+ @Test
+ public void shouldPassForExpectedBehaviorThatHappenedWithVerify() {
+
+ mock.booleanObjectReturningMethod();
+
+ then(mock).verify().booleanObjectReturningMethod();
+ }
+
+ @Test
+ public void shouldPassFluentBddScenario() {
+
+ Bike bike = new Bike();
+ Person person = mock(Person.class);
+
+ person.ride(bike);
+ person.ride(bike);
+
+ then(person).should(times(2)).ride(bike);
+ }
+
+ @Test
+ public void shouldPassFluentBddScenarioWithVerify() {
+
+ Bike bike = new Bike();
+ Person person = mock(Person.class);
+
+ person.ride(bike);
+ person.ride(bike);
+
+ then(person).verify(times(2)).ride(bike);
+ }
+
+ static class Person {
+
+ void ride(Bike bike) {}
+ }
+
+ static class Bike {
+
+ }
}
Something went wrong with that request. Please try again.