Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tweak JUnit rule strictness at the method level #840

Closed
mockitoguy opened this issue Dec 22, 2016 · 9 comments
Closed

Tweak JUnit rule strictness at the method level #840

mockitoguy opened this issue Dec 22, 2016 · 9 comments

Comments

@mockitoguy
Copy link
Member

mockitoguy commented Dec 22, 2016

Team, I would really appreciate feedback about this enhancement.

There is a feature request reported - a way to exempt from unnecessary stubbing at method level (#792). The feature request makes sense to me. Currently, if the user wants to opt-out from JUnit rule or JUnit runner strictness, he needs to do it at the scope of the entire class. Ideally, we can opt-out from strictness at finer granularity level (method or even at mock level).

Inspired by @bric3 idea I'd like to get your feedback about this API enhancement for strict JUnit rules.

Example test class:

public class MyTest {
    //Mockito stubbing is strict in the entire test class
    @Rule public MockitoRule mockito = MockitoJUnit.rule().strictness(Strictness.STRICT_STUBS);

    @Test public void lenient_mockito() throws Throwable {
        //making Mockito lenient only for this test method
        //below API already exist but it does not have any effect inside of the test method
        mockito.strictness(Strictness.LENIENT);

        //rest of the test
    }
}

Pros:

  • we already have 'strictness' method in the MockitoRule API so it's not adding any new API
  • least surprise principle - users might expect this behavior to work but currently 'strictness' method creates new instance of the rule and not change the state of the rule.

Cons:

  • it's slightly not intuitive that 'strictness' method actually changes state (it's not a setter).
@TimvdLippe
Copy link
Contributor

Instead of a call to the rule (which I would expect to cause side-effects when I see the code) I think it is clearer to use an annotation on the method. Something like @MockitoStrictness(LENIENT)

@mockitoguy
Copy link
Member Author

Annotations are clearer and they are a good idea, we discussed it in #792, @bric3 was not in favor of annotations.

Call to the rule is something we would offer in the meantime, before we have annotations. Long term, I think annotations are a way forward.

I'm waiting for more feedback and I'm leaning to implement this new rule behavior regardless if we decide to do annotations or not :)

@bric3
Copy link
Contributor

bric3 commented Dec 23, 2016

Yes I'm not in favor of annotation, but I'm not totally against either. However annotation API, is to be carefully designed as there is way less flexibility, override, argument types, etc.

Pros

  • I believe a programtic way offers more choice for users and framework developers, including mockito.
  • least surprise principle - users might expect this behavior to work but currently 'strictness' method creates new instance of the rule and not change the state of the rule.

That's maybe the biggest issue here. To bypass this the code will need to have some way to define a global state. in spseudo code :

// for
MockitoJUnit.rule().strictness(Strictness.STRICT_STUBS);

public MockitoRule strictness(Strictness strictness) {
    return new JUnitRule(internalRuleRef, logger, strictness);
}

private JUnitRule(AtomicReference<JUnitRule> internalRef, MockitoLogger logger, Strictness strictness) {
    this.internalRule.set(new JUnitRule(logger, strictness)); // existing constructor, but should be another class ideally.
}

public Statement apply(Statement base, FrameworkMethod method, Object target) {
    return internalRule.apply(base, method, target);
}

Cons

  • it's slightly not intuitive that 'strictness' method actually changes state (it's not a setter).

Nowadays people understand fluent API outside javabean's setters or getters. I don't think that's really an issue there, especially if there's code example in the javadoc.

@mockitoguy
Copy link
Member Author

Great feedback, @bric3

I'll take a stab at making strictness() method working for us. I should be able to use a single test listener inside a rule and make this listener mutable so that it can have different behavior between test runs.

mockitoguy added a commit that referenced this issue Dec 24, 2016
Users can now change the strictness of Mockito at the junit method level. It's not as nice as with an annotation but at least it gets the job done. See discussion at #840

Pending: javadoc
@TimvdLippe
Copy link
Contributor

A solution could be that the Rule also watches for annotations and calls the corresponding method. That would require a follow-up PR of #843. WDYT?

@mockitoguy
Copy link
Member Author

A solution could be that the Rule also watches for annotations and calls the corresponding method. That would require a follow-up PR of #843. WDYT?

I like this idea a lot. There are 2 main reasons I like it:

  • annotation looks nice and clean in test code
  • annotation would also work with JUnit runner (when we make it work with runner :). For the runner, we cannot change it's state programmatically because it is a static instance attached to the test class via @RunWith annotation. With JUnit rule the situation is different - we have an instance of a rule during test execution.

@bric3
Copy link
Contributor

bric3 commented Dec 26, 2016

There's also another option like :

rule = MockitoJUnit.rule().strictness(Strictness.STRICT_STUBS);


// in some code block
ruleHandler = rule.handler() // no argument = getter
ruleHandler.strictness(Stritness.LENIENT) // takes arg = modify behavior

mockitoguy added a commit that referenced this issue Dec 27, 2016
Users can now change the strictness of Mockito at the junit method level. It's not as nice as with an annotation but at least it gets the job done. See discussion at #840

Pending: javadoc
@mockitoguy
Copy link
Member Author

Thanks @bric3 for feedback! I'd rather stick to strictness() method instead of introducing handler(). It feels the API is simpler and more intuitive to use.

mockitoguy added a commit that referenced this issue Dec 28, 2016
Users can now change the strictness of Mockito at the junit method level. It's not as nice as with an annotation but at least it gets the job done. See discussion at #840

Pending: javadoc
@mockitoguy
Copy link
Member Author

Fixed by #843

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants