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 more test to describe more request on jmockit requests. #662

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

yuzhg
Copy link

@yuzhg yuzhg commented Jan 6, 2025

What's changed?

Base on my actual working case, I add some test cases, which describes my hope on the enhancement behaviors of jmockit recipes.

What's your motivation?

I hope the JMockit recipe can be more flexible.

Anything in particular you'd like reviewers to focus on?

Some of the test are conflicting with current test, it is just a suggestion based on my actual cases. But I don't know much about other ones scenario, so please just take it as a discussion and pick up whatever you guys think make sense.
My tests are mainly on following points

  1. I found the recipe not works on Expectations inside Junit 5 exception check blocks or try blocks, like:
void test() {
    assertThrows(IllegalStateException.class, () -> {
        new Expectations() {{
            myObject.toString();
            result = "foo";
            times = 2;
        }};
        assertEquals("foo", myObject.toString());
        assertEquals("foo", myObject.toString());
    });
}
  1. For Expectations, if there is no times/minTimes specified, it should verify with atLeast(1), per my understanding base on jmockit doc: During replay, invocations matching a recorded expectation must occur at least once (unless specified otherwise); if, by the end of the test, no matching invocation occurred for a given recorded expectation, the test will fail with a MissingInvocation error. from https://javadoc.io/doc/org.jmockit/jmockit/latest/index.html.
  2. If there is no result specified, it's better to mock the default behavior base on jmockit doc on result section. This will be extremely useful if the mock it in spring test environment and the mocked field sometimes will be replaced with Spybeans:

If no result is recorded for a given expectation, then all matching invocations will return the appropriate default value according to the method return type:

Most java.lang types (String, Object, etc.): returns null.
java.math types (BigDecimal, etc.): returns null.
Primitive/wrapper types: returns the standard default value (false for boolean/Boolean, 0 for int/Integer, and so on).
java.util.List, java.util.Collection, or java.lang.Iterable: returns [Collections.EMPTY_LIST](https://docs.oracle.com/javase/8/docs/api/java/util/Collections.html?is-external=true#EMPTY_LIST).
java.util.Iterator or java.util.ListIterator: returns an empty iterator.
java.util.Set: returns [Collections.EMPTY_SET](https://docs.oracle.com/javase/8/docs/api/java/util/Collections.html?is-external=true#EMPTY_SET).
java.util.SortedSet: returns an unmodifiable empty sorted set.
java.util.Map: returns [Collections.EMPTY_MAP](https://docs.oracle.com/javase/8/docs/api/java/util/Collections.html?is-external=true#EMPTY_MAP).
java.util.SortedMap: returns an unmodifiable empty sorted map.
java.util.Optional: returns [Optional.empty()](https://docs.oracle.com/javase/8/docs/api/java/util/Optional.html?is-external=true#empty--).
Other reference types: returns a mocked instance through cascading.
Array types: returns an array with zero elements (empty) in each dimension.
  1. JMockit can setup expectations in setup methods or util methods, those mock need also be handled. If it's not easy to handle it all atomatically, I personally think create the mock and leave an TODO message might be a possible solution.

  2. In my case, the doReturn().when().xxx() form of mocking will be more rubost than when(a.b()).thenReturn(c) form, since the latter form sometimes will throw NPE, but the former one seems never have this problem.

Checklist

  • I've added unit tests to cover both positive and negative cases
  • I've used the IntelliJ IDEA auto-formatter on affected files

@timtebeek timtebeek marked this pull request as draft January 6, 2025 09:25
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
@timtebeek
Copy link
Contributor

Great to see @yuzhg ! Thanks for logging the proposed improvements as runnable tests; that's very helpful! Are you planning to adjust the recipe implementation as well?

@yuzhg
Copy link
Author

yuzhg commented Jan 6, 2025

Great to see @yuzhg ! Thanks for logging the proposed improvements as runnable tests; that's very helpful! Are you planning to adjust the recipe implementation as well?

Sorry I don't actually have the plan to change the original recipe now.

You can see that my suggestions actually kind of conflict with the current implementation. If I change the current recipe, it will be a lot of changes and basically most UT will need to be touched.

So before there is any agreements, I actually created a separated recipe for my own use. This one is not having all functions, not refactored, not optimized, and only reach the bottom line of usable for my own code repository. If any piece of the codes can help, you can directly use it.

@timtebeek
Copy link
Contributor

Much appreciated @yuzhg , and good to hear you're not blocked on any of this at least. We'll keep this open for now then.

@timtebeek timtebeek added enhancement New feature or request recipe Recipe request labels Jan 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request recipe Recipe request
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

2 participants