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

Update Javadoc for ArgumentCaptor #3103

Merged
merged 1 commit into from Aug 27, 2023

Conversation

StevenCurran
Copy link
Contributor

@StevenCurran StevenCurran commented Aug 24, 2023

Update Javadoc for ArgumentCaptor to indicate that the captor will match based on the generic type since Mockito 5.0

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

@StevenCurran
Copy link
Contributor Author

StevenCurran commented Aug 24, 2023

Hey all, recently updated a project to use 5.5.0. Had to go through and adjust a lot of the tests, as my times() calls were causing tests to fail. I was looking at the javadoc to see if there were changes, but it wasn't until I took a diff of the 4.7.0 and 5.5.0 tags that I could see org.mockito.internal.matchers.CapturingMatcher#matches had a behavior change. I see it was mentioned in the release notes here, but would have been useful to be in the javadoc.
#2907 (comment)

@StevenCurran StevenCurran changed the title Update Javadoc for ArgumentCaptor to indicate that the captor will co… Update Javadoc for ArgumentCaptor Aug 24, 2023
@StevenCurran StevenCurran force-pushed the feature/ArgumentCaptorJavaDoc branch 2 times, most recently from f6ddd2c to 3dd4bd3 Compare August 24, 2023 02:54
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.

2 small nits, other than that LGTM!

src/main/java/org/mockito/ArgumentCaptor.java Outdated Show resolved Hide resolved
src/main/java/org/mockito/ArgumentCaptor.java Outdated Show resolved Hide resolved
@StevenCurran
Copy link
Contributor Author

Updated with your suggestions. Commits squashed.

Thank you @TimvdLippe!

@codecov-commenter
Copy link

codecov-commenter commented Aug 27, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.02% 🎉

Comparison is base (87a99dd) 85.45% compared to head (617b3bc) 85.48%.
Report is 2 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #3103      +/-   ##
============================================
+ Coverage     85.45%   85.48%   +0.02%     
- Complexity     2889     2892       +3     
============================================
  Files           329      329              
  Lines          8822     8826       +4     
  Branches       1095     1095              
============================================
+ Hits           7539     7545       +6     
+ Misses          995      994       -1     
+ Partials        288      287       -1     
Files Changed Coverage Δ
src/main/java/org/mockito/ArgumentCaptor.java 100.00% <100.00%> (ø)

... and 3 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@TimvdLippe TimvdLippe merged commit 8962ed0 into mockito:main Aug 27, 2023
12 checks passed
@StevenCurran StevenCurran deleted the feature/ArgumentCaptorJavaDoc branch August 27, 2023 15:27
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.

None yet

3 participants