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.

Sign up for GitHub

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

Jump to bottom

Fixes #2510: Remove ExpectedException from internal test suite #2518

Merged
merged 72 commits into from
Dec 24, 2021
Merged

Fixes #2510: Remove ExpectedException from internal test suite #2518

merged 72 commits into from
Dec 24, 2021

Conversation

temp-droid
Copy link
Contributor

@temp-droid temp-droid commented Dec 16, 2021 •
edited
Loading

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

@codecov-commenter
Copy link

codecov-commenter commented Dec 16, 2021 •
edited
Loading

Codecov Report

Merging #2518 (dc2231c) into main (95bc5b2) will not change coverage.
The diff coverage is n/a.

❗ Current head dc2231c differs from pull request most recent head 367fb12. Consider uploading reports for the commit 367fb12 to get more accurate results
Impacted file tree graph

@@            Coverage Diff            @@
##               main    #2518   +/-   ##
=========================================
  Coverage     86.66%   86.66%           
  Complexity     2781     2781           
=========================================
  Files           320      320           
  Lines          8344     8344           
  Branches       1022     1022           
=========================================
  Hits           7231     7231           
  Misses          842      842           
  Partials        271      271
Impacted Files Coverage Δ
src/main/java/org/mockito/Mockito.java 92.20% <0.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 95bc5b2...367fb12. Read the comment docs.

@temp-droid
Replace ExpectedException in NumberOfInvocationsInOrderCheckerTest
bf50938
@temp-droid
Replace ExpectedException in NumberOfInvocationsCheckerTest
054a01c
@temp-droid
Replace @test(expected=...) in ConstructionMockTest
58b7ece
@temp-droid
Replace @test(expected=...) in StaticMockTest
08c15fa
@temp-droid
Replace @test(expected=...) in MocksTest
8dad4e8
@temp-droid
Replace @test(expected=...) in MockitoTest
a271c76
@temp-droid
Fix actual & expected order in last MockitoTest assertion
96a36e2
@temp-droid
Replace @test(expected=...) in MockAnnotationProcessorTest
21eb375
@temp-droid
Replace @test(expected=...) in MockInjectionTest
0f2b920
@temp-droid
Replace isEqualTo(true) with isTrue()
3660c41
@temp-droid
Replace @test(expected=...) in MockSettingsImplTest
cd6dfe1
@temp-droid
Replace junit assertions with assertj
d2e47eb
@temp-droid
Replace @test(expected=...) in ConstructorInstantiatorTest
982b56c
@temp-droid
Replace junit assertions with assertj
cb96576
@temp-droid
Replace @test(expected=...) in ReporterTest
5f6a321
@temp-droid
Replace @test(expected=...) in DefaultMockitoFrameworkTest
c520d44
@temp-droid
Remove unused import in VerificationCollectorImplTest
a100d94
@temp-droid
Replace @test(expected=...) in MatchersTest
7627fdf
@temp-droid
Replace @test(expected=...) in ReflectionMatchersTest
db0981b
@temp-droid
Replace @test(expected=...) in DescriptiveMessagesOnMisuseTest
a348c26
@temp-droid
Replace @test(expected=...) in InvalidUsageTest
75ccfa3
@temp-droid
Replace @test(expected=...) in StubbingConsecutiveAnswersTest
695021d
@temp-droid
Replace @test(expected=...) in StubbingUsingDoReturnTest
2efc8a2
@temp-droid
Replace @test(expected=...) in StubbingWarningsTest
1b4f335
@temp-droid
Replace @test(expected=...) in BasicVerificationInOrderTest
291a04f
@temp-droid
Replace @test(expected=...) in BasicVerificationTest
c65c55c
@temp-droid
Replace @test(expected=...) in NoMoreInteractionsVerificationTest
2dd30c7
@temp-droid
Replace @test(expected=...) in RelaxedVerificationInOrderTest
0dd975c
@temp-droid
Replace @test(expected=...) in VerificationExcludingStubsTest
67436f4
@temp-droid
Replace @test(expected=...) in VerificationInOrderMixedWithOrdiraryVe…
d9edde1 
…rificationTest
@temp-droid
Rename VerificationInOrderMixedWithOrdiraryVerificationTest to Verifi…
02e57ef 
…cationInOrderMixedWithOrdinaryVerificationTest
@temp-droid
Replace @test(expected=...) in ClassLoadersTest
797d0a6
@temp-droid
Replace @test(expected=...) in SafeJUnitRuleTest
c2c73be
@temp-droid
Replace @test(expected=...) in MockHandlerImplTest
f5e895d
@temp-droid
Replace @test(expected=...) in AtLeastMarksAllInvocationsVerified
dc2231c
@temp-droid
Replace @test(expected=...) in InvocationNotifierHandlerTest
367fb12
@temp-droid
Copy link
Contributor Author

Hey @TimvdLippe!

Outside one ExpectedException case in SpyAnnotationTest that I'm unsure of (not used in the past?) and one @Test(expected… in the osgi-test I'm not confortable touching, the rest should be replaced.

Before continuing replacing the try fail finally cases, I wanted to check with you if it wouldn't be best to create another PR for those. We are already at 72 commits for this one and I counted more than 380 occurrences in ~120 files for fail().

I feel like this would be too much to review in a single PR, but you tell me.

@TimvdLippe
Copy link
Contributor

Yes. This PR is already massive, so let's cut it here. I hope to have time on Friday to review. Thanks for your effort!

Note that in this case I am okay with so many commits, since it is mechanical. However, for other PRs let's try to keep them small 😄

@temp-droid temp-droid changed the title Fixes #2510: Remove ExpectedException from internal test suite (WIP) Fixes #2510: Remove ExpectedException from internal test suite Dec 22, 2021
@temp-droid temp-droid marked this pull request as ready for review December 22, 2021 20:08
@temp-droid
Copy link
Contributor Author

Do you have suggestions how to approach the new PR? First creating an issue to link it to? Or referencing the same one?

I'm afraid it might create confusion to have 2 PRs for the same issue qua closing.

@TimvdLippe
Copy link
Contributor

Let's file a new issue for that. Thanks!

TimvdLippe
TimvdLippe approved these changes Dec 24, 2021
View reviewed changes
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 cleanup!

@TimvdLippe TimvdLippe merged commit 6b818ba into mockito:main Dec 24, 2021
xuelin2020 pushed a commit to xuelin2020/mockito that referenced this pull request Dec 28, 2021
@temp-droid @xuelin2020
Fixes mockito#2510: Remove ExpectedException from internal test suite (…
3d53496 
…mockito#2518)
@theaspect theaspect mentioned this pull request Jan 25, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TimvdLippe TimvdLippe TimvdLippe approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@temp-droid @codecov-commenter @TimvdLippe