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

Improve code coverage of submodule parsing #873

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

robramsaynz
Copy link
Contributor

This is a PR to improve coverage for submodule parsing that's already in the system. Specifically func getSubmodules in pkg/osvscanner/osvscanner.go.

This is already covered by a fixture based test in pkg/osvscanner/osvscanner_internal_test.go but while have execution inside getSubmodules, it doesn't actually cover the working parts of getSubmodules.

You can see coverage info by running ./scripts/generate_coverage_report.sh and looking in ./coverage.html#file133.

There are a number of states a submodule can be in:

  1. a submodule is setup and checked out
  2. a submodule is setup and but dir is empty
  3. a submodule is setup and and checked out but dir has local changes
  4. a submodule is setup and but dir is set to a different commit

I'm planning to test 1 and 2. I think 3 and 4 are nice to haves, and I plan to only tackle them if they're easy once 1 and 2 are done. Possibly submodules have more possible states, but these seem reasonable for coverage.

I see two implementation options:

@codecov-commenter
Copy link

codecov-commenter commented Mar 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.85%. Comparing base (f8953ff) to head (bf25030).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #873      +/-   ##
==========================================
+ Coverage   67.83%   67.85%   +0.02%     
==========================================
  Files         174      174              
  Lines       16769    16769              
==========================================
+ Hits        11375    11379       +4     
+ Misses       4765     4761       -4     
  Partials      629      629              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@robramsaynz
Copy link
Contributor Author

I've done a bit more poking and managed to create git-repo with submodules that doesn't have a backing github repo, so re-utilising the existing fixture based testing —func Test_scanGit— looks like the best option at this stage.

Copy link

This pull request has not had any activity for 60 days and will be automatically closed in two weeks

@github-actions github-actions bot added the stale The issue or PR is stale and pending automated closure label Jul 18, 2024
Copy link

github-actions bot commented Aug 1, 2024

Automatically closing stale pull request

@github-actions github-actions bot closed this Aug 1, 2024
@another-rex another-rex reopened this Aug 6, 2024
@github-actions github-actions bot removed the stale The issue or PR is stale and pending automated closure label Aug 6, 2024
@G-Rath G-Rath force-pushed the bugfix/improve-submodule-coverage branch from 9740dd9 to bf25030 Compare September 12, 2024 23:32
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.

3 participants