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

Add new method(s) in ApplicationErrorVerifications to permit custom VerificationMode #297

Closed
sleberknight opened this issue Oct 16, 2023 · 0 comments · Fixed by #298
Closed
Assignees
Labels
new feature A new feature such as a new class, method, package, group of classes, etc.
Milestone

Comments

@sleberknight
Copy link
Member

Add a new method(s) in ApplicationErrorVerifications to permit using custom VerificationMode. This will allow more flexibility, such as when errors are recorded in a background thread and we need to provide a timeout to the verification.

Suppose we have the following test code which needs to use a timeout, which we have no way to do currently in ApplicationErrorVerifications:

// errorDao is a Mockito mock of ApplicationErrorDao

// verify the only invocation occurs within 500 milliseconds
var argCaptor = ArgumentCaptor.forClass(ApplicationError.class);
verify(errorDao, timeout(500).only()).insertOrIncrementCount(argCaptor.capture());
var applicationError = first(argCaptor.getValue());

// now make assertions on the error, e.g.
assertAll(
    () -> assertThat(applicationError.getDescription()).isEqualTo("oops"),
    () -> assertThat(applicationError.getExceptionType()).isNull(),
    () -> assertThat(applicationError.getNumTimesOccurred()).isOne()
);

We want a method which can take the place of the verification code above, e.g.

var error = ApplicationErrorVerifications.verifyInsertOrIncrementCount(errorDao, timeout(500).only());

The above method implies that either the insertOrIncrementCount method was called exactly once or that we only care about the most recent captured value. The issue with allowing any VerificationMode here is that there can be multiple invocations such as when using the atLeast verification mode. For example:

var error = ApplicationErrorVerifications.verifyInsertOrIncrementCount(errorDao, timeout(500).atLeastOnce());

By using atLeastOnce, there might be multiple invocations, and as defined we would only get the latest captured ApplicationError. I don't see any way to define a single method which can handle both, so perhaps we add two methods and put the responsibility on the caller to call the correct method and pass the appropriate VerificationMode based on whether the verification mode can result in one or more values. (with great power...)

// static imports of ApplicationErrorVerifications.*

// assumes exactly one
ApplicationError error = verifyOneOrLatestInsertOrIncrementCount(errorDao, timeout(500).only());

// assumes may be more than one, but we only need the latest
ApplicationError error = verifyOneOrLatestInsertOrIncrementCount(errorDao, timeout(500).atLeastOnce());

// assumes may be more than one, and we need all the errors
List<ApplicationError> errors = verifyManyInsertOrIncrementCount(errorDao, timeout(500).atLeastOnce());

// assumes exactly a certain number of times, and we need all the errors
List<ApplicationError> errors = verifyManyInsertOrIncrementCount(errorDao, timeout(250).times(3));

So, if someone passes timeout(500).only() to verifyManyInsertOrIncrementCount and (somehow) expects to get more than one result, they will always be disappointed and will need to figure out the problem themselves. It would be a bad idea for us to inspect the actual type of the VerificationMode to obtain the class (e.g. org.mockito.internal.verification.Only) and try to detect invalid verification modes. One simple reason is hinted at by the package name containing the word "internal" - it's not supposed to be accessed by users, whether enforced by the Java module system or not.

I prefer having the flexibility and take responsibility for using the methods correctly. Of course, the Javadocs should be very clear about the intended usage and provide appropriate caveats and warnings.

@sleberknight sleberknight added the new feature A new feature such as a new class, method, package, group of classes, etc. label Oct 16, 2023
@sleberknight sleberknight self-assigned this Oct 20, 2023
@sleberknight sleberknight added this to the 2.1.0 milestone Oct 20, 2023
sleberknight added a commit that referenced this issue Oct 20, 2023
Add new methods verifyOneOrLatestInsertOrIncrementCount and
verifyManyInsertOrIncrementCount to ApplicationErrorVerifications.
These permit callers to specify a custom VerificationMode for
more flexibility than the existing methods.

Closes #297
sleberknight added a commit that referenced this issue Oct 20, 2023
…de (#298)

Add new methods verifyOneOrLatestInsertOrIncrementCount and
verifyManyInsertOrIncrementCount to ApplicationErrorVerifications.
These permit callers to specify a custom VerificationMode for
more flexibility than the existing methods.

Closes #297
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature A new feature such as a new class, method, package, group of classes, etc.
Projects
None yet
1 participant