Skip to content

test: Fix issues in TestSecretScanningService tests - #3760

Merged
gmlewis merged 2 commits into
google:masterfrom
alkak95:go-github-add-test
Oct 6, 2025
Merged

test: Fix issues in TestSecretScanningService tests#3760
gmlewis merged 2 commits into
google:masterfrom
alkak95:go-github-add-test

Conversation

@alkak95

@alkak95alkak95 commented Oct 6, 2025

Copy link
Copy Markdown
Contributor

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:
snip2

Code coverage percentage after this fix:
snip1

@google-cla

google-claBot commented Oct 6, 2025

Copy link
Copy Markdown

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.

@alkak95

Copy link
Copy Markdown
ContributorAuthor

@gmlewis can you approve the PR, so that test can run

@alexandearalexandear left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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)PASSok github.com/google/go-github/v75/github	1.626s

@alkak95

Copy link
Copy Markdown
ContributorAuthor

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.
And hence the code for these methods were not covered.

As a part of this MR, I observed the missing code coverage and as a fix moved the test cases out of handler logic .
@alexandear

@alexandear

Copy link
Copy Markdown
Contributor

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. And hence the code for these methods were not covered.

As a part of this MR, I observed the missing code coverage and as a fix moved the test cases out of handler logic . @alexandear

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.

@alkak95alkak95 changed the title fix exisiting test case for secret_scanning_pattern_configs_test to i…fix non-functioning existing test case for secret_scanning_pattern_configs_test.go file to increase code coverageOct 6, 2025
@alkak95

Copy link
Copy Markdown
ContributorAuthor

Hi @alexandear updated the description and title of PR.
Kindly review.

@gmlewis

Copy link
Copy Markdown
Collaborator

Just FYI - that is now a very long PR title that will be changed before merging.

@codecov

codecovBot commented Oct 6, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 91.34%. Comparing base (3f8720b) to head (0f4bc61).
⚠️ Report is 1 commits behind head on master.

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@alkak95alkak95 changed the title fix non-functioning existing test case for secret_scanning_pattern_configs_test.go file to increase code coveragefix non-functioning existing test case for secret_scanning_pattern_configs_test.go fileOct 6, 2025
@alkak95alkak95 changed the title fix non-functioning existing test case for secret_scanning_pattern_configs_test.go filefix issue in existing test case for secret_scanning_pattern_configs_test.go fileOct 6, 2025
@alkak95alkak95 changed the title fix issue in existing test case for secret_scanning_pattern_configs_test.go filetest: fix non-functioning existing test caseOct 6, 2025
@alkak95alkak95 changed the title test: fix non-functioning existing test casetest: fix issues in existing test caseOct 6, 2025
@gmlewis

gmlewis commented Oct 6, 2025

Copy link
Copy Markdown
Collaborator

@alkak95 - according to GitHub, there are now NO changes in this PR. Maybe the merge destroyed them or this test was fixed by another PR?!?!?

SORRY! My GitHub tab apparently freaked out. I had to do a hard-reload to see any changes. Weird.

Never mind. 😂

@gmlewisgmlewis changed the title test: fix issues in existing test casetest: Fix issues in TestSecretScanningService testsOct 6, 2025
@gmlewisgmlewis added the NeedsReview PR is awaiting a review before merging. label Oct 6, 2025

@gmlewisgmlewis left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you, @alkak95!
LGTM.

@alexandear - do you approve these changes, since you commented?

@gmlewis

Copy link
Copy Markdown
Collaborator

Thank you, @alexandear!
Merging.

@gmlewis
gmlewis merged commit 960f978 into google:masterOct 6, 2025
7 checks passed
@gmlewisgmlewis removed the NeedsReview PR is awaiting a review before merging. label Oct 8, 2025
jlaportebot added a commit to jlaportebot/go-github that referenced this pull request Jun 28, 2026
Sign up for freeto 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.

3 participants

@alkak95@alexandear@gmlewis