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

Fixes #2201 : Fixed checking of declared exceptions. #2547

Merged
merged 2 commits into from Jan 26, 2022

Conversation

andrey-kozel
Copy link
Contributor

Fixed checking of declared exceptions. In case when parent contains throws keyword on its method and child overrides this method removing throws, it should be possible to mock throwing exception from child

Description.

Lets assume that we have the following interface

public interface Foo {
        void bar() throws IOException; 
    }

And we have class that extends the interface, removing throws exception

public class Baz implements Foo {
        @Override
        public void bar() {
            
        }
    }

When we trying to mock exception throw, it wasn't possible

Foo mock = Mockito.spy(new Baz());
Mockito.doThrow(new IOException())
    .when(mock)
    .bar();

Now this behavior fixed

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

…hen parent contains throws keyword on its method and child overrides this method removing throws, it should be possible to mock throwing exception from child
@codecov-commenter
Copy link

codecov-commenter commented Jan 23, 2022

Codecov Report

Merging #2547 (81c71a8) into main (faa6e92) will decrease coverage by 0.15%.
The diff coverage is 100.00%.

❗ Current head 81c71a8 differs from pull request most recent head 792da83. Consider uploading reports for the commit 792da83 to get more accurate results
Impacted file tree graph

@@             Coverage Diff              @@
##               main    #2547      +/-   ##
============================================
- Coverage     86.69%   86.54%   -0.16%     
- Complexity     2782     2790       +8     
============================================
  Files           320      320              
  Lines          8344     8359      +15     
  Branches       1022     1023       +1     
============================================
  Hits           7234     7234              
- Misses          840      857      +17     
+ Partials        270      268       -2     
Impacted Files Coverage Δ
.../internal/creation/bytebuddy/MockMethodAdvice.java 80.28% <100.00%> (+0.40%) ⬆️
...kito/internal/stubbing/answers/InvocationInfo.java 100.00% <100.00%> (ø)
...ito/internal/creation/proxy/MethodHandleProxy.java 54.54% <0.00%> (-45.46%) ⬇️
...ckito/internal/creation/proxy/ProxyRealMethod.java 42.85% <0.00%> (-28.58%) ⬇️

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 faa6e92...792da83. Read the comment docs.

@aglumova
Copy link

WOW Amazing!!

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 PR! I think we are nearly there, but there are some more edge cases we need to handle.

@andrey-kozel
Copy link
Contributor Author

Thanks for the PR! I think we are nearly there, but there are some more edge cases we need to handle.

Thank you for the comment. I've extended checking for exceptions and added tests for all the cases you've mentioned

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.

Nice work and regression tests, thanks! 🎉

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

5 participants