test: Fix issues in TestSecretScanningService tests
#3760
Conversation
…ncrease code coverage
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
@gmlewis can you approve the PR, so that test can run |
There was a problem hiding this comment.
Could you provide more details about what was fixed?
The tests in this file passed successfully when run locally:
$ go test -timeout 2m -run ^(TestSecretScanningService_ListPatternConfigsForEnterprise|TestSecretScanningService_ListPatternConfigsForOrg|TestSecretScanningService_UpdatePatternConfigsForEnterprise|TestSecretScanningService_UpdatePatternConfigsForOrg|TestSecretScanningPatternConfigs_Marshal|TestSecretScanningPatternOverride_Marshal|TestSecretScanningPatternConfigsUpdate_Marshal|TestSecretScanningPatternConfigsUpdateOptions_Marshal|TestSecretScanningProviderPatternSetting_Marshal|TestSecretScanninCustomPatternSetting_Marshal)$ github.com/google/go-github/v75/github -v -race -shuffle=on -parallel=8 -failfast
-test.shuffle 1759743901880289000
=== RUN TestSecretScanningPatternConfigsUpdate_Marshal
=== PAUSE TestSecretScanningPatternConfigsUpdate_Marshal
=== RUN TestSecretScanningPatternOverride_Marshal
=== PAUSE TestSecretScanningPatternOverride_Marshal
=== RUN TestSecretScanningPatternConfigs_Marshal
=== PAUSE TestSecretScanningPatternConfigs_Marshal
=== RUN TestSecretScanningProviderPatternSetting_Marshal
=== PAUSE TestSecretScanningProviderPatternSetting_Marshal
=== RUN TestSecretScanningService_ListPatternConfigsForEnterprise
=== PAUSE TestSecretScanningService_ListPatternConfigsForEnterprise
=== RUN TestSecretScanningService_UpdatePatternConfigsForEnterprise
=== PAUSE TestSecretScanningService_UpdatePatternConfigsForEnterprise
=== RUN TestSecretScanningPatternConfigsUpdateOptions_Marshal
=== PAUSE TestSecretScanningPatternConfigsUpdateOptions_Marshal
=== RUN TestSecretScanninCustomPatternSetting_Marshal
=== PAUSE TestSecretScanninCustomPatternSetting_Marshal
=== RUN TestSecretScanningService_UpdatePatternConfigsForOrg
=== PAUSE TestSecretScanningService_UpdatePatternConfigsForOrg
=== RUN TestSecretScanningService_ListPatternConfigsForOrg
=== PAUSE TestSecretScanningService_ListPatternConfigsForOrg
=== CONT TestSecretScanningPatternOverride_Marshal
=== CONT TestSecretScanningPatternConfigsUpdateOptions_Marshal
=== CONT TestSecretScanningService_ListPatternConfigsForEnterprise
=== CONT TestSecretScanninCustomPatternSetting_Marshal
=== CONT TestSecretScanningService_ListPatternConfigsForOrg
=== CONT TestSecretScanningProviderPatternSetting_Marshal
=== CONT TestSecretScanningPatternConfigs_Marshal
=== CONT TestSecretScanningPatternConfigsUpdate_Marshal
--- PASS: TestSecretScanningPatternConfigsUpdate_Marshal (0.00s)
--- PASS: TestSecretScanningProviderPatternSetting_Marshal (0.00s)
=== CONT TestSecretScanningService_UpdatePatternConfigsForOrg
--- PASS: TestSecretScanninCustomPatternSetting_Marshal (0.00s)
=== CONT TestSecretScanningService_UpdatePatternConfigsForEnterprise
--- PASS: TestSecretScanningPatternConfigsUpdateOptions_Marshal (0.00s)
--- PASS: TestSecretScanningPatternConfigs_Marshal (0.00s)
--- PASS: TestSecretScanningPatternOverride_Marshal (0.00s)
--- PASS: TestSecretScanningService_ListPatternConfigsForOrg (0.00s)
--- PASS: TestSecretScanningService_UpdatePatternConfigsForEnterprise (0.00s)
--- PASS: TestSecretScanningService_UpdatePatternConfigsForOrg (0.00s)
--- PASS: TestSecretScanningService_ListPatternConfigsForEnterprise (0.00s)
PASS
ok github.com/google/go-github/v75/github 1.626s|
It passes successfully because the test methods are called , but none of the the test cases were actually executed , as the test cases were written inside the handler logic for TestSecretScanningService_ListPatternConfigsForOrg, TestSecretScanningService_UpdatePatternConfigsForEnterprise, TestSecretScanningService_UpdatePatternConfigsForOrg. As a part of this MR, I observed the missing code coverage and as a fix moved the test cases out of handler logic . |
Thanks for the explanation and quick reply. Please also update the PR title and description with this information. You can include before/after screenshots of the test coverage - this greatly helps during review. |
|
Hi @alexandear updated the description and title of PR. |
|
Just FYI - that is now a very long PR title that will be changed before merging. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3760 +/- ##
==========================================
+ Coverage 91.11% 91.34% +0.23%
==========================================
Files 187 187
Lines 16702 16702
==========================================
+ Hits 15218 15257 +39
+ Misses 1296 1257 -39
Partials 188 188 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
SORRY! My GitHub tab apparently freaked out. I had to do a hard-reload to see any changes. Weird. Never mind. 😂 |
TestSecretScanningService tests
There was a problem hiding this comment.
Thank you, @alkak95!
LGTM.
@alexandear - do you approve these changes, since you commented?
|
Thank you, @alexandear! |
Issue: Existing test cases for TestSecretScanningService_ListPatternConfigsForOrg, TestSecretScanningService_UpdatePatternConfigsForEnterprise, TestSecretScanningService_UpdatePatternConfigsForOrg method in secret_scanning_pattern_configs_test.go file were not executing the actual test cases, because the test cases were written inside the handler logic . This issue lead to only 25% of code coverage.
Resolution : As a fix, moved the test cases out of handler logic , which called all the test cases of the functions(TestSecretScanningService_ListPatternConfigsForOrg, TestSecretScanningService_UpdatePatternConfigsForEnterprise,TestSecretScanningService_UpdatePatternConfigsForOrg) leading to 100% of code coverage.
Code coverage percentage before this fix:
Code coverage percentage after this fix:
