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

Code coverage of OBO Authentication #3428

Merged
merged 25 commits into from
Oct 12, 2023

Conversation

RyanL1997
Copy link
Collaborator

@RyanL1997 RyanL1997 commented Sep 29, 2023

Description

Code coverage of OBO Authentication

  • Category (Enhancement, New feature, Bug fix, Test fix, Refactoring, Maintenance, Documentation)
    Enhancement

Issues Resolved

Check List

  • New functionality includes testing
  • New functionality has been documented
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@codecov
Copy link

codecov bot commented Sep 29, 2023

Codecov Report

Merging #3428 (ac5fa2d) into main (6b0b682) will increase coverage by 0.28%.
Report is 9 commits behind head on main.
The diff coverage is 66.66%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #3428      +/-   ##
============================================
+ Coverage     64.71%   64.99%   +0.28%     
- Complexity     3616     3638      +22     
============================================
  Files           281      281              
  Lines         20570    20581      +11     
  Branches       3400     3398       -2     
============================================
+ Hits          13312    13377      +65     
+ Misses         5557     5517      -40     
+ Partials       1701     1687      -14     
Files Coverage Δ
...nsearch/security/http/OnBehalfOfAuthenticator.java 95.83% <66.66%> (+27.51%) ⬆️

... and 7 files with indirect coverage changes

@RyanL1997 RyanL1997 marked this pull request as ready for review October 2, 2023 18:21
@RyanL1997 RyanL1997 force-pushed the obo-tests-cov branch 2 times, most recently from 641ae3e to b166fee Compare October 5, 2023 06:15
@peternied
Copy link
Member

@RyanL1997 it looks like spotless found some issues in this PR, can you fix those and push? It should also clean up the failing tests as I don't think they were related to your change.

Signed-off-by: Ryan Liang <[email protected]>
Copy link
Member

@DarshitChanpura DarshitChanpura left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @RyanL1997. Could you add some final % number for code coverage?

Signed-off-by: Ryan Liang <[email protected]>
Signed-off-by: Ryan Liang <[email protected]>
@DarshitChanpura
Copy link
Member

@RyanL1997 Is this ready for final review?

@RyanL1997
Copy link
Collaborator Author

Final numbers:

  • OBOAuthenticator cov: 95.83%
  • JWTVendor cov: 96.97%

Signed-off-by: Darshit Chanpura <[email protected]>
Copy link
Member

@DarshitChanpura DarshitChanpura left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great stuff @RyanL1997 boosting the code coverage!

@stephen-crawford stephen-crawford merged commit 387c8a5 into opensearch-project:main Oct 12, 2023
59 checks passed
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.

[Feature/Extension] 100% Code coverage of OBO Authenticator and Jwt Vendor
4 participants