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

Updated test coverage for async verification #1429

Merged
merged 2 commits into from Jul 28, 2018
Merged

Conversation

mockitoguy
Copy link
Member

Updated tests with after() verification mode

  • Some existing tests for the feature did not work due to flaws in ReplyRule. Those tests (now ignored) consistently failed but succeeded only when used with reply rule.
  • Some existing tests had weak assertions that did not fully cover the feature.
  • Suggested simpler and cleaner way of implementing delayed execution via new 'AsyncTesting' class, instead of 'DelayedExecution'. Dropping the reply rule may introduce instabilities to the CI builds, let's observe and measure before making an action.

Although we are considering deprecating 'after()', we need good coverage for an existing feature even if it will be removed in the long run.

This way we can write specific assertions making the tests stronger. I need this for reworking of 'after' and 'timeout' tests.
- Some existing tests for the feature did not work due to flaws in ReplyRule. Some of the tests (now ignored) consistently failed but succeeded only when used with reply rule.
- Some existing tests had weak assertions that did not fully cover the feature.
- Suggested simpler and cleaner way of implementing delayed execution via new 'AsyncTesting' class, instead of 'DelayedExecution'

Although we are considering deprecating 'after()', we need good coverage for an existing feature even if it will be removed in the long run.
@codecov-io
Copy link

codecov-io commented Jul 13, 2018

Codecov Report

Merging #1429 into release/2.x will increase coverage by 0.08%.
The diff coverage is 100%.

Impacted file tree graph

@@               Coverage Diff                @@
##             release/2.x   #1429      +/-   ##
================================================
+ Coverage          88.61%   88.7%   +0.08%     
- Complexity          2357    2364       +7     
================================================
  Files                292     293       +1     
  Lines               5949    5983      +34     
  Branches             719     733      +14     
================================================
+ Hits                5272    5307      +35     
- Misses               497     498       +1     
+ Partials             180     178       -2
Impacted Files Coverage Δ Complexity Δ
...verification/MoreThanAllowedActualInvocations.java 100% <100%> (ø) 1 <1> (?)
...java/org/mockito/internal/exceptions/Reporter.java 93.68% <100%> (ø) 91 <1> (ø) ⬇️
...al/creation/bytebuddy/InlineBytecodeGenerator.java 95.13% <0%> (+2.28%) 31% <0%> (+6%) ⬆️

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 5fc152d...6f14c84. Read the comment docs.

@mockitoguy
Copy link
Member Author

@ChristianSchwarz, any feedback about this change?

@ChristianSchwarz
Copy link
Contributor

I will take a look.

@mockitoguy
Copy link
Member Author

I'll merge this if that's ok. It mostly test code change with a compatible change to exceptions. Happy to take review feedback after it's merged.

@mockitoguy mockitoguy merged commit b324274 into release/2.x Jul 28, 2018
@TimvdLippe TimvdLippe deleted the next-delay-tests branch October 9, 2018 15:51
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