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

Refactor and improve UT for PoP token verifier #371

Merged
merged 3 commits into from
Aug 9, 2023
Merged

Refactor and improve UT for PoP token verifier #371

merged 3 commits into from
Aug 9, 2023

Conversation

nehaPrakash98
Copy link

@nehaPrakash98 nehaPrakash98 commented Jul 31, 2023 •
edited
Loading

  • The goal of this PR is to improve the test coverage. All of the happy path and error cases (where feasible) are covered by unit tests.
  • In order to improve the readability of the tests, the test code that makes the test PoP tokens has been refactored. The existing code was hard to extend for new scenarios and hard to follow. The new code uses a builder pattern to create the tokens.

Previous test coverage was ~80%
Current test coverage is 91.5%

image

@nehaPrakash98 nehaPrakash98 force-pushed the refactor-and-improve-guard-ut branch 2 times, most recently from 6a7bdb8 to 05c80e7 Compare July 31, 2023 22:49
julienstroheker
julienstroheker reviewed Jul 31, 2023
View reviewed changes
azure-coverage.out Outdated Show resolved Hide resolved
@AzureMarker
Copy link
Contributor

Make sure to fix the DCO check:
https://github.com/kubeguard/guard/pull/371/checks?check_run_id=15500422959

@nehaPrakash98 nehaPrakash98 marked this pull request as ready for review August 2, 2023 19:12
@nehaPrakash98 nehaPrakash98 requested a review from a team as a code owner August 2, 2023 19:12
julienstroheker
julienstroheker suggested changes Aug 4, 2023
View reviewed changes
Copy link
Contributor

@julienstroheker julienstroheker left a comment

Choose a reason for hiding this comment

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

Overall LGTM, this is really good improvment.

I added comment about structure.

@julienstroheker
Copy link
Contributor

@weinong are you able to trigger BUILD to validates the UT on this PR please ?

@julienstroheker
Copy link
Contributor

@nehaPrakash98 please check the DCO

Neha Prakash added 3 commits August 8, 2023 12:38
Refactor and improve UT for PoP token verifier
a4fe63b 
Signed-off-by: Neha Prakash <nehaprakash@microsoft.com>
Refactored the test structure to follow builder architecture and adde…
4b345b8 
…d comments for all test cases in pop_tokenverifier_test

Signed-off-by: Neha Prakash <nehaprakash@microsoft.com>
Addressed comments, changes include - added package azure_test, added…
00571c8 
… helper file pop_tokenverifier_test_utils.go, refactored test cases to use a table driven approach

Signed-off-by: Neha Prakash <nehaprakash@microsoft.com>
julienstroheker
julienstroheker approved these changes Aug 8, 2023
View reviewed changes
Copy link
Contributor

@julienstroheker julienstroheker left a comment

Choose a reason for hiding this comment

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

LGTM

weinong
weinong approved these changes Aug 9, 2023
View reviewed changes
Copy link
Contributor

@weinong weinong left a comment

Choose a reason for hiding this comment

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

LGTM

@weinong weinong merged commit 19ffb38 into kubeguard:master Aug 9, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@julienstroheker julienstroheker julienstroheker approved these changes

@weinong weinong weinong 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.
4 participants
@nehaPrakash98 @AzureMarker @julienstroheker @weinong