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

Increase unit test coverage to 100% #652

Closed
Justin-W opened this issue Dec 15, 2021 · 2 comments
Closed

Increase unit test coverage to 100% #652

Justin-W opened this issue Dec 15, 2021 · 2 comments
Labels
Stale

Comments

@Justin-W
Copy link
Contributor

Is your feature request related to a problem? Please describe.

The PR template states that 100% code coverage is a requirement for every PR, but coverage seems to have gone down to ~99% quite a while ago.

Describe the solution you'd like

Ideally, more specs should be added to cover all currently-uncovered lines of code and return the unit test suite to 100% of lines covered.

Note: PR #651 added 9 Pending (unimplemented) unit specs (plus corresponding TODO: coverage: comments) to make it clearer which specs should be added to achieve this goal.

  • 6 of the 9 specs relate directly to lines of code currently without coverage.
  • the other 3 of 9 Pending specs added relate to effectively-uncovered code (i.e. sections of code which are technically covered by being executed by at least some unit specs, but for which there do not appear to be any specs which actually have relevant spec expectations which would cause spec failures if the code is defective), which happens to be near some of the uncovered lines of code.
    • Note: There are likely other sections of "effectively-uncovered code" (possibly many more) besides the ones related to these 3 recently-added Pending specs. I only noticed that these 3 specs were "missing" while trying to locate where the other 6 Pending specs should go.

Describe alternatives you've considered

Alternatively, the PR template message could be updated so that 100% is no longer a stated requirement for PR submission/approval.

Additional context

Note: The existing unit tests/specs would likely continue to pass even if any "effectively-uncovered code" sections (either those referenced above and/or others) are and/or become broken, and therefore the unit tests are likely to fail to detect a variety of potential functional regressions/defects affecting such code.
@Justin-W Justin-W mentioned this issue Dec 16, 2021
Merged
1 task
@rkoster rkoster moved this from Inbox to Waiting for Changes | Open for Contribution in Foundational Infrastructure Working Group Dec 16, 2021
@rkoster
Copy link
Contributor

rkoster commented Dec 16, 2021

I'm surprised there was a 100% coverage requirement, this is not something we have in the other bosh related repo's. Could you make a PR to remove the line about the 100% from: https://github.com/cloudfoundry/bosh-azure-cpi-release/tree/master/.github?

@Justin-W Justin-W mentioned this issue Dec 16, 2021
Closed
Justin-W added a commit to Justin-W/bosh-azure-cpi-release that referenced this issue Dec 20, 2021
@Justin-W
issue-652: Updated the PR template, per the request of @rkoster.
9b46970 
See: cloudfoundry#652 (comment)
Justin-W added a commit to Justin-W/bosh-azure-cpi-release that referenced this issue Dec 20, 2021
@Justin-W
issue-652: Updated the PR template, per the request of @rkoster.
5650c70 
See: cloudfoundry#652 (comment)
Justin-W added a commit to Justin-W/bosh-azure-cpi-release that referenced this issue Dec 20, 2021
@Justin-W
issue-652: Updated the PR template, per the request of @rkoster.
87c12f7 
See: cloudfoundry#652 (comment)

Note: The new template content is similar to corresponding content within the Vsphere CPI's PR template.
@Justin-W Justin-W mentioned this issue Dec 20, 2021
Merged
3 tasks
@Justin-W
Copy link
Contributor Author

I'm surprised there was a 100% coverage requirement, this is not something we have in the other bosh related repo's. Could you make a PR to remove the line about the 100%

Done. See #654.

Justin-W added a commit to Justin-W/bosh-azure-cpi-release that referenced this issue Dec 22, 2021
@Justin-W
issue-652: coverage: Removed pending specs for uncovered lines of code.
4777dd1 
These Pending specs were removed at the request of @beyhan.

See also:
- cloudfoundry#651 (review)
- cloudfoundry#652
@cloudfoundry cloudfoundry deleted a comment from bosh-admin-bot Jan 11, 2022
@rkoster rkoster closed this as completed Jan 11, 2022
Repository owner moved this from Waiting for Changes | Open for Contribution to Done in Foundational Infrastructure Working Group Jan 11, 2022
Justin-W added a commit to Justin-W/bosh-azure-cpi-release that referenced this issue Jan 12, 2022
@Justin-W
Added new specs for uncovered lines of code (cloudfoundry#652): agent…
a8bb7c6 
…_config_spec.rb.

Note: These new specs cover some previously-uncovered lines of existing code.

Related to commit #c7c47dddd64e515a1dbb2d7971b91d676110f216.
Justin-W added a commit to Justin-W/bosh-azure-cpi-release that referenced this issue Jan 12, 2022
@Justin-W
Added new specs for uncovered lines of code (cloudfoundry#652): confi…
77cd537 
…g_spec.rb.

Note: These new specs cover some previously-uncovered lines of existing code.

Related to commit #c7c47dddd64e515a1dbb2d7971b91d676110f216.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Stale
Projects
Foundational Infrastructure Working Group
Archived in project
Milestone
No milestone
Development

No branches or pull requests
3 participants
@Justin-W @rkoster @bosh-admin-bot