Skip to content

Add tests to increase line coverage from 82% to 91.51% - #296

Merged
david22swan merged 7 commits into
mainfrom
increase-code-coverage-to-91-percent
Sep 8, 2026
Merged

david22swan merged 7 commits into
mainfrom
increase-code-coverage-to-91-percent

Conversation

@bronachfalls

Copy link
Copy Markdown
Contributor

New tests cover previously untested paths in abs, provision_service, vagrant, and task_helper: CI URL detection branches, Windows platform provisioning, invoke_cloud_request delete/error paths, generate_vagrantfile options, vagrant_version memoization, validation error paths, and rescue blocks.

Summary

Provide a detailed description of all the changes present in this pull request.

Additional Context

Add any additional context about the problem here.

  • Root cause and the steps to reproduce. (If applicable)
  • Thought process behind the implementation.

Related Issues (if any)

Mention any related issues or pull requests.

Checklist

  • 🟢 Spec tests.
  • 🟢 Acceptance tests.
  • Manually verified.

New tests cover previously untested paths in abs, provision_service,
vagrant, and task_helper: CI URL detection branches, Windows platform
provisioning, invoke_cloud_request delete/error paths, generate_vagrantfile
options, vagrant_version memoization, validation error paths, and rescue blocks.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@bronachfalls
bronachfalls requested a review from a team as a code owner September 7, 2026 13:16
- Replace /regex/ with %r{} (Style/RegexpLiteral)
- Replace allow_any_instance_of with explicit instance stubs (RSpec/AnyInstance)
- Nest new describe blocks inside existing top-level describe (RSpec/MultipleDescribes)
- Add before(:each) to allow real File.read in generate_vagrantfile tests

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
david22swan
david22swan previously approved these changes Sep 7, 2026

Copilot AI 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.

🟡 Changes recommended

Several specs execute advertised branches without verifying their behavior, and the environment helper leaks CI variable changes.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds Ruby specs to raise coverage for provisioning helpers and task edge cases.

Changes:

  • Tests validation, error handling, and task dispatch.
  • Covers CI URLs, Windows inventory, Vagrant options, and cloud requests.
  • Adds fog-file failure-path coverage.
File summaries
File Description
spec/unit/task_helper_spec.rb Tests fog-token failures.
spec/tasks/vagrant_spec.rb Tests Vagrant helpers and validation.
spec/tasks/provision_service_spec.rb Tests dispatch, parameters, and HTTP errors.
spec/tasks/abs_spec.rb Tests CI, Windows, credentials, and validation paths.
Review details

Suppressed comments (4)

spec/tasks/abs_spec.rb:156

  • This example only checks the final task result; the unrestricted stub does not verify that the GitHub Actions URL was placed in the request. Match the posted jenkins_build_url so a regression in this branch fails the spec.
      stub_request(:post, 'https://abs-prod.k8s.infracore.puppet.net/api/v2/request')
        .to_return({ status: 202 }, { status: 200, body: response_body.to_json })

spec/tasks/abs_spec.rb:164

  • The request stub accepts every payload, so this example does not distinguish the fallback branch from either CI branch. Match the fallback URL in the posted body to validate the behavior described by the example.
      stub_request(:post, 'https://abs-prod.k8s.infracore.puppet.net/api/v2/request')
        .to_return({ status: 202 }, { status: 200, body: response_body.to_json })

spec/tasks/provision_service_spec.rb:78

  • This stub does not require tear_down to be called, so .run could skip dispatch and the exit-status assertion would still pass. Use a message expectation with the forwarded arguments.
      allow(runner).to receive(:tear_down).and_return('{}')

spec/tasks/provision_service_spec.rb:212

  • This assertion is identical in effect to the JSON-response example and never checks preservation of the raw body or body_json: false. Assert those fields in stdout so the non-JSON rescue path is covered behaviorally.
      expect { svc.invoke_cloud_request({}, uri, nil, 'post', 0) }.to(
        raise_error(SystemExit) { |e| expect(e.status).to eq(1) },
      )
  • Files reviewed: 4/4 changed files
  • Comments generated: 4
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread spec/tasks/abs_spec.rb
Comment thread spec/tasks/abs_spec.rb
Comment thread spec/tasks/provision_service_spec.rb Outdated
Comment thread spec/tasks/provision_service_spec.rb
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
bronachfalls and others added 4 commits September 7, 2026 15:51
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Lines inserted at column 0 instead of matching surrounding indentation.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Previously the ensure block deleted every key unconditionally, removing
pre-existing CI env vars for all subsequent examples. Now each key's
original value is snapshotted before the block and restored after.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@david22swan
david22swan merged commit c78f707 into main Sep 8, 2026
4 checks passed
@david22swan
david22swan deleted the increase-code-coverage-to-91-percent branch September 8, 2026 15:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants