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

Release Mockito 5 #2802

Closed
4 tasks done
TimvdLippe opened this issue Nov 27, 2022 · 22 comments
Closed
4 tasks done

Release Mockito 5 #2802

TimvdLippe opened this issue Nov 27, 2022 · 22 comments

Comments

@TimvdLippe
Copy link
Contributor

TimvdLippe commented Nov 27, 2022

All right, the information is scattered throughout a couple of GitHub issues, but let's summarize the changes we want to make for Mockito 5:

I propose that we do not include any other breaking changes, as I think these two will already have a large impact. Since I would like our users to be able to upgrade to the latest version of Mockito, even with breaking changes, I think these bumps are already sufficiently large.

Please see the linked GitHub issues for a status update on what we need to do.

@TimvdLippe
Copy link
Contributor Author

I wrote down the required steps for both issues in #2589 (comment) and #2798 (comment) Contributions welcome!

@big-andy-coates
Copy link
Contributor

I propose that we do not include any other breaking changes

... damn! I was about to suggest dropping the broken VarargMatcher. Can we track that for Mokito 6? :)

@TimvdLippe
Copy link
Contributor Author

@big-andy-coates To be honest, I would be open for that, given that it is an internal interface anyways. I mostly would like to avoid pulling in too many changes in 1 go, as that is going to cause headaches for our users. Hopefully I have some time next week to review your PRs (was on vacation).

@big-andy-coates
Copy link
Contributor

big-andy-coates commented Dec 14, 2022

@TimvdLippe

Sounds like a plan. Just let me know.

The main PR fixes a load of vararg related stuff, but does not drop VarargMatcher. I'll do that in a different PR, and you can choose if you want to include it in the v5 release.

@TimvdLippe
Copy link
Contributor Author

Okay, we had some hiccups with the 4.10.0 release, but I just double-checked that all artifacts were published just fine to Maven Central. As such I will start the Mockito 5 release preparations by merging several PRs that will have breaking changes (FYI @mockito/developers ).

I would still like to keep our breaking changes list minimal if we can. Therefore, I propose that we keep the currently listed 2 and discuss the inclusion of the VarargMatcher removal, but we don't include any others. These we can do later in Mockito 6 in a year or so, to keep things flowing nicely.

@big-andy-coates
Copy link
Contributor

big-andy-coates commented Dec 19, 2022

Re: VarargMatcher,

#2807 fixes loads of vararg issues, but is NOT a breaking change.

Once #2807 is reviewed and merged, I'll raise a PR to remove VarargMatcher. Which will provide people with more context to discuss if it should / should not be included in this release.

VarargMatcher may be an internal interface, but removing it will be a breaking change. However, it will also result in better support for varargs.

@ramananrpn
Copy link

Hi would like to contribute here. Please let me know if any hand is needed

@TimvdLippe
Copy link
Contributor Author

@ramananrpn I think we are good for now, as the three changes have been sorted out, but will let you know when we need more people 😄

@big-andy-coates
Copy link
Contributor

PR for the removal of VarargMatcher in progress: #2835

@TimvdLippe
Copy link
Contributor Author

#2834 is passing all checks. I will start writing up a wiki page now with release notes that we can put on GitHub, similar to what we did with 4.0: https://github.com/mockito/mockito/releases/tag/v4.0.0

@TimvdLippe
Copy link
Contributor Author

I have written up a draft on https://github.com/mockito/mockito/wiki/Draft-Mockito-5-release-notes @big-andy-coates can you please go over the draft and make sure the notes with regards to the varargs change makes sense?

Once we finalize the draft, we should also include this as a new section in the main Mockito.class JavaDoc (

public class Mockito extends ArgumentMatchers {
) so that we also include the full release notes in our artifacts. I think only the varargs release notes make sense to be included, the rest is general maintenance that is already available in our POM's.

@TimvdLippe
Copy link
Contributor Author

Oh and @mockito/developers can you please review the draft release notes as well?

@raphw
Copy link
Member

raphw commented Jan 8, 2023

Thanks for the srite up. Looks good, I made some small changes!

@big-andy-coates
Copy link
Contributor

big-andy-coates commented Jan 9, 2023

I have written up a draft on https://github.com/mockito/mockito/wiki/Draft-Mockito-5-release-notes @big-andy-coates can you please go over the draft and make sure the notes with regards to the varargs change makes sense?

Close enough.

You may want to consider simplifying the example issue to being unable to mock calls with exactly one vararg argument, as this may be easier to understand than overloaded methods. Or you may want to include both the "unable to mock calls with exactly one argument" and the current issue about overloaded methods. Up to you.

Details of the "unable to mock calls with exactly one argument" issue:

For varargs methods, there was previously a way to only match zero arguments, or two or more arguments, by using the exact number of matchers, i.e.

long call(String... args);

// Will match calls with exactly zero arguments:
when(mock.call()).thenReturn(0L);

// Will match calls with exactly two arguments:
when(mock.call(any(), any())).thenReturn(0L);

But following the pattern to match exactly one argument:

when(mock.call(any())).thenReturn(0L);

doesn't work, as any is "vararg aware", so Mockito matched the any against each element of the varargs parameter, meaning it will match any number of arguments, i.e. the above would of matched all of these:

mock.call();
mock.call("a");
mock.call("a", "b");

With the new type method, it's now possible to differentiate matching calls with any exact number of arguments, or to match any number of arguments.

// Match any number of arguments:
when(mock.call(any(String[].class))).thenReturn(1L);
// Match invocations with no arguments:
when(mock.call()).thenReturn(1L);
// Match invocations with exactly one argument:
when(mock.call(any())).thenReturn(1L);
// Alternative to match invocations with exactly one argument:
when(mock.call(any(String.class))).thenReturn(1L);
// Match invocations with exactly two arguments:
when(mock.call(any(), any())).thenReturn(1L);

@TimvdLippe
Copy link
Contributor Author

Cool, thanks @big-andy-coates I have updated the wiki page. Last step is to update the main Mockito documentation with a small section on the updates. I will probably get to that on Thursday

@TimvdLippe
Copy link
Contributor Author

Hm, I was working on adding a section to the main Mockito JavaDoc, but I now think that we shouldn't do it. The breaking changes will be clearly documented on GitHub and we haven't included the breaking changes of Mockito 4 in the JavaDoc either. Therefore, I think we can ship Mockito 5 as-is. @raphw do you agree?

@raphw
Copy link
Member

raphw commented Jan 13, 2023

I also think it can be confusing in the javadoc. Most people will look there to see how things work now. Those are no release notes in that sense.-

@TimvdLippe
Copy link
Contributor Author

Mockito 5 has been released: https://github.com/mockito/mockito/releases/tag/v5.0.0 Thanks everyone for your input, work and dedication!

@timtebeek
Copy link
Contributor

Might also want to update the README, as that currently starts with

Current version is 4.x

Still on Mockito 1.x? See what's new in Mockito 2! Mockito 3 does not introduce any breaking API changes, but now requires Java 8 over Java 6 for Mockito 2. Mockito 4 removes deprecated API.

@TimvdLippe
Copy link
Contributor Author

@timtebeek good catch! Do you mind sending a PR to fix it?

@TWiStErRob
Copy link
Contributor

@TimvdLippe I think you can unpin this.

@TimvdLippe
Copy link
Contributor Author

@TWiStErRob I want to pin it for a bit, so that everybody can get the full context for the release. I will unpin this in a bit.

@TimvdLippe TimvdLippe unpinned this issue Jan 23, 2023
neo-technology-build-agent pushed a commit to neo4j/neo4j that referenced this issue Mar 24, 2023
aws-java-sdk 1.12.377 -> 1.12.399
lucene 9.4.2 -> 9.5.0
mockito 4.11.0 -> 5.1.1
jackson 2.14.1 -> 2.14.2
jackson-databind 2.14.1 -> 2.14.2
picocli 4.7.0 -> 4.7.1
restrict-imports-enforcer-rule 2.0.0 -> 2.1.0
jna 5.12.1 -> 5.13.0
jose4j 0.7.12 -> 0.9.2
reactor-bom 2022.0.1 -> 2022.0.2
assertj-core 3.23.1 -> 3.24.2

Mockito changed a way again how they work with final methods/classes and varargs so some adaptations where made to tests. See mockito/mockito#2802 for details.
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

6 participants