Reputation: 2026
We have made our own framework that makes it easy to setup an analysis pipeline. Every time an analysis ends, finish() is called. finish() uploads files that were generated during the analysis. To ensure that the framework was used correctly, we have made a check that finish() is not called twice.
Now, I want to test that finish() is called for a specific step in the pipeline. I do this by calling the following in my test:
verify(consumer).finish();
But apparently, verify() also calls finish() so an exception is thrown and the test fails.
Now, my question is:
EDIT
A quick setup of the problem:
Analysis
package mockitoTwice;
public class Analysis extends Finishable {
@Override
public void finishHelper() {
System.out.println("Calling finishHelper()");
}
}
Finishable
package mockitoTwice;
public abstract class Finishable {
protected boolean finished = false;
public final void finish() {
System.out.println("Calling finish()");
if (finished) {
throw new IllegalStateException();
}
finished = true;
finishHelper();
}
public abstract void finishHelper();
}
Pipeline
package mockitoTwice;
public class Pipeline {
private Analysis analysis;
public Pipeline(Analysis analysis) {
this.analysis = analysis;
}
public void runAnalyses() {
analysis.finish();
}
}
PipelineTest
package mockitoTwice;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import org.junit.Test;
public class PipelineTest {
@Test
public void test() {
Analysis analysis = mock(Analysis.class);
Pipeline pipeline = new Pipeline(analysis);
pipeline.runAnalyses();
verify(analysis).finish();
}
}
Upvotes: 1
Views: 891
Reputation: 452
Testing frameworks all have their quirks, but when you encounter problems like this then the first step is to assess your class and test design.
First, I notice that AnalysisTest isn't really testing the Analysis class. It's mocking Analysis and actually testing the Pipeline class. A proper test for Analysis would look something like this:
@Test
public void testFinishTwice() {
Analysis analysis = new Analysis();
try {
analysis.finish();
analysis.finish();
Assert.fail();
} catch (IllegalStateException ex) {
// Optionally assert something about ex
}
}
This will verify the implied contract that Analysis throws IllegalStateException when finish() is called more than once. There are a variety of solutions to your problem and most of them depend on validating this.
Next, the abstract Finishable class with a final finish() method is not quite as foolproof as it looks. Since the finishHelper method has protected access, then it is still accessible directly to any class in the package. So in your example, if Pipeline and Analysis are in the same package, then Pipeline could call finishHelper directly. I would guess that's the single biggest risk of the actual finish code getting called twice. How easy would it be to accidentally let your IDE autocomplete to finishHelper? Even if your unit test worked as you wanted, it could not catch this.
Now that I've addressed that, we can get to the root of the problem. The finish method is marked as final, so Mockito can't override it. Normally, Mockito would create a stub method for it but here it has to use the original code from Finishable. A true mock wouldn't even print "Calling finish()" when finish is called. Since it's stuck with original implementation, the real finish method gets called both by pipeline and then again by verify(analysis).finish();
So what do we do about it? There's no perfect answer and it really depends on details of the situation.
The simplest approach would be to drop the final keyword on the finish method. Then you just have to make sure that Analysis and Pipeline are not in the same package. The test you wrote ensures and the Pipeline only calls finish once. The test I suggested guarantees an exception the second time finish is called on Analysis. This happens even if it would override finish. You could still abuse it, but you'd have to deliberately go out of your way to do it.
You could also switch Finishable to an interface and rename your current class AbstractFinishable as a base implementation. Then switch Analysis to an interface that extends Finishable and create an ExampleAnalysis class that extends AbstractFinishable and implements Analysis. Then Pipeline references the Analysis interface. We have to do it that way because otherwise it could get access to finishHelper and we are back where we started. Here's a sketch of the code:
public interface Finishable {
public void finish();
}
public abstract class AbstractFinishable implements Finishable {
// your current Finishable class with final finish method goes here
}
public interface Analysis extends Finishable {
// Other analysis methods that Pipeline needs go here
}
public ExampleAnalysis extends AbstractFinishable implements Analysis {
// Implementations of Analysis methods go here
}
So that's one way to do it. It's essentially switching the classes to be coded to interfaces of their dependencies rather than specific class implementations. That's generally easier to mock and test. You could also use the delegate pattern and just put a Finishable on ExampleAnalysis rather than extending AbstractFinishable. There are other ways, too, and these are just ideas. You should know the specifics of your project well enough to decide the best route.
Upvotes: 2
Reputation: 2026
The problem can be solved by catching the exception of the framework as follows:
@Rule
public ExpectedException exception;
@Test
public void test() {
Analysis analysis = mock(Analysis.class);
Pipeline pipeline = new Pipeline(analysis);
pipeline.runAnalyses();
exception.expect(IllegalStateException.class);
verify(analysis).finish();
}
If finish() is called too few times, the verify handles the problem as one would expect.
If finish() is called too many times, the exception is called on pipeline.runAnalyses()
.
Otherwise, the test is successful.
Upvotes: 0