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

Fix for Issue2656 #2659

Merged
merged 4 commits into from Jun 2, 2022
Merged

Fix for Issue2656 #2659

merged 4 commits into from Jun 2, 2022

Conversation

big-andy-coates
Copy link
Contributor

@big-andy-coates big-andy-coates commented May 31, 2022

fix: #2656

Release 4.6.0 introduced a new feature #2650 to allow strictness to be set via the @Mock annotation. Unfortunately, the feature has a bug that results in any test-level strictness setting being ignored, e.g.

@ExtendWith(MockitorExtension.class)
@MockitoSettings(strictness = Strictness.LENIENT)
class ThingTest {

  @Mock
  private Thing lenientMock;
}

In the above code the lenientMock would actually be strict, as the @Mock's strictness defaults to STRICT_STUBS. This default was overriding the test level LENIENT.

To fix the issue the default of the Mock.strictness method needs to be something meaning not set, allowing the mock's strictness to be ignored and the test level strictness to be used.

Unfortunately, no no set value exists in the Strictness enum and it is illegal to default to null. Fixing this in a backwards compatible way would have meant polluting the public API, (e.g. adding a DEFAULT value to the existing Strictness enum, which doesn't make sense in a lot of other places the enum is used). As this feature has only just been released, and is essentially broken, a breaking change is being introduced to fix the issue.

BREAKING CHANGE: This PR changes the return value of Mock.strictness() to a local Strictness enum value. This will require users to change any code written against 4.6.0 that sets the strictness via the @Mock annotation to change their code.

Example code migration:

class ThingTest {

    @Mock(strictness = Strictness.LENIENT)
    private Thing thing;

    ...
}

To:

class ThingTest {

    @Mock(strictness = Mock.Strictness.LENIENT)
    private Thing thing;

    ...
}

Checklist

  • Read the contributing guide
  • PR should be motivated, i.e. what does it fix, why, and if relevant how
  • If possible / relevant include an example in the description, that could help all readers
    including project members to get a better picture of the change
  • Avoid other runtime dependencies
  • Meaningful commit history ; intention is important please rebase your commit history so that each
    commit is meaningful and help the people that will explore a change in 2 years
  • The pull request follows coding style
  • Mention Fixes #<issue number> in the description if relevant
  • At least one commit should mention Fixes #<issue number> if relevant

Copy link
Contributor

@TimvdLippe TimvdLippe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor changes, but overall looking good!

src/main/java/org/mockito/Mock.java Show resolved Hide resolved
src/test/java/org/mockito/MockTest.java Outdated Show resolved Hide resolved
@codecov-commenter
Copy link

codecov-commenter commented Jun 1, 2022

Codecov Report

Merging #2659 (d0bf836) into main (4f41700) will increase coverage by 0.05%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##               main    #2659      +/-   ##
============================================
+ Coverage     86.22%   86.27%   +0.05%     
- Complexity     2763     2769       +6     
============================================
  Files           314      315       +1     
  Lines          8290     8300      +10     
  Branches       1031     1029       -2     
============================================
+ Hits           7148     7161      +13     
+ Misses          873      872       -1     
+ Partials        269      267       -2     
Impacted Files Coverage Δ
src/main/java/org/mockito/Mock.java 100.00% <100.00%> (ø)
...nternal/configuration/MockAnnotationProcessor.java 97.14% <100.00%> (+0.08%) ⬆️
...rg/mockito/internal/creation/MockSettingsImpl.java 88.11% <100.00%> (ø)
...o/internal/creation/settings/CreationSettings.java 100.00% <100.00%> (ø)
.../mockito/internal/stubbing/StrictnessSelector.java 100.00% <0.00%> (+60.00%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4f41700...d0bf836. Read the comment docs.

Copy link
Contributor

@TimvdLippe TimvdLippe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix!

@TimvdLippe TimvdLippe merged commit ff98622 into mockito:main Jun 2, 2022
@big-andy-coates big-andy-coates deleted the issue2656 branch June 3, 2022 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Regression? Strictness set in @MockitoSettings ignored after upgrade from 4.5.1 to 4.6.0
3 participants