Skip to content

Conversation

@vertti
Copy link
Owner

@vertti vertti commented Jan 1, 2026

  • Add Version.String() test (pkg/version now at 100%)
  • Add tests for cmd error paths (invalid versions, flags)
  • Add tests for hash algorithms (sha384, sha1, auto)
  • Add tests for JSON flag validation
  • Add tests for prometheus thresholds
  • Add tests for resource command paths
  • Add realFileChecker test
  • Extract main() to separate file for cleaner coverage

Summary by CodeRabbit

  • Tests

    • Significantly expanded unit test coverage for command validation and error handling across all major components, including environment variable constraints and min/max validation, JSON processing, Prometheus integration, and resource operations with comprehensive edge case testing scenarios.
    • Added test verification for version string formatting.
  • Chores

    • Internal reorganization and improvement of the main entry point structure.

✏️ Tip: You can customize this high-level summary in your review settings.

- Add Version.String() test (pkg/version now at 100%)
- Add tests for cmd error paths (invalid versions, flags)
- Add tests for hash algorithms (sha384, sha1, auto)
- Add tests for JSON flag validation
- Add tests for prometheus thresholds
- Add tests for resource command paths
- Add realFileChecker test
- Extract main() to separate file for cleaner coverage
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 1, 2026

📝 Walkthrough

Walkthrough

This pull request refactors the preflight command's entry point by moving the main() function from main.go to a new main_entry.go file, while expanding test coverage with new unit tests across multiple command validation paths and error handling scenarios.

Changes

Cohort / File(s) Summary
Entry point refactoring
cmd/preflight/main.go, cmd/preflight/main_entry.go
Relocated main() function logic from main.go (now empty) to new main_entry.go, preserving argument transformation, file checking, root command execution, and optional post-execution external command invocation via runExec.
Validation and error handling tests
cmd/preflight/execute_test.go
Added comprehensive test functions: TestEnvMinMaxValue (numeric constraints), TestRealFileChecker (file validation), TestJSONCommandErrorPaths, TestPrometheusCommandErrorPaths, TestResourceCommandErrorPaths (error scenarios for command handlers). Enhanced TestHashCommandMore (sha384, sha1, auto-detection) and TestCmdCommand (version argument validation).
Version string formatting test
pkg/version/parse_test.go
Added TestVersion\_String to verify Version.String() method formatting for typical and edge cases.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

🐰 The main march moves to a new home,
Tests multiply like clover blooms,
Entry points dance, validation zooms,
Error paths checked with careful tooth,
A refactor's gentle, bugless truth! 🌿

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main objective of the PR: adding tests to improve code coverage to a specific percentage. It directly reflects the changeset's primary focus.
✨ Finishing touches
  • 📝 Docstrings were successfully generated.

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 656afe9 and e4aced0.

📒 Files selected for processing (4)
  • cmd/preflight/execute_test.go
  • cmd/preflight/main.go
  • cmd/preflight/main_entry.go
  • pkg/version/parse_test.go
💤 Files with no reviewable changes (1)
  • cmd/preflight/main.go
🧰 Additional context used
🧬 Code graph analysis (1)
pkg/version/parse_test.go (1)
pkg/version/parse.go (1)
  • Version (12-16)
🔇 Additional comments (13)
pkg/version/parse_test.go (1)

5-21: LGTM! Well-structured test for Version.String().

The test covers typical, edge, and varied version values with clear assertions. The table-driven approach follows Go best practices.

cmd/preflight/main_entry.go (2)

8-27: LGTM! Clean entrypoint with proper separation of concerns.

The main function properly orchestrates hashbang handling, exec args extraction, command execution, and external command invocation. Error handling for runExec is explicit and clear.


18-20: No action needed. Cobra's Execute() method automatically prints errors to stderr by default unless SilenceErrors is explicitly set to true. Since no such configuration exists in your setup, errors are already being printed correctly. The code pattern at lines 18-20 is standard and appropriate.

cmd/preflight/execute_test.go (10)

134-164: LGTM! Comprehensive testing of numeric environment variable validation.

The test properly validates both min-value and max-value checks with pass and fail scenarios. The logic is correct and the subtest structure is appropriate.


225-239: LGTM! Good coverage of version flag validation errors.

These test cases properly validate error handling for invalid version arguments across min, max, and exact flags.


436-437: LGTM! Helpful comment clarifying test scope.

The updated comment appropriately explains the testing constraints for the run command.


612-630: LGTM! Good coverage of SHA384 and SHA1 hash validation.

The tests properly validate both SHA384 and SHA1 hash algorithms with correct hash values.


642-650: LGTM! Auto-detection test validates hash algorithm inference.

The test properly verifies that the --auto flag can detect the hash algorithm based on the hash length.


672-678: LGTM! Good error path coverage for missing hash flag.

The test ensures that the hash command properly fails when no hash algorithm flag is provided.


681-701: LGTM! Comprehensive coverage of realFileChecker behavior.

The test properly validates all three expected behaviors: existing files return true, nonexistent files return false, and directories return false.


703-750: LGTM! Excellent coverage of JSON command validation paths.

The tests comprehensively cover error paths (missing file, missing required flags) and success paths (has-key, key with exact, key with match). The validation logic is sound.


752-795: LGTM! Good coverage of Prometheus command validation and threshold checks.

The tests properly validate error paths (invalid URL) and threshold validation (min, max, exact) with appropriate pass and fail scenarios.


797-824: LGTM! Comprehensive coverage of resource command error paths.

The tests properly validate error handling for nonexistent paths, invalid size formats, and success cases with custom paths. Good coverage of both error and success scenarios.


Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link

codecov bot commented Jan 1, 2026

Codecov Report

❌ Patch coverage is 0% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.47%. Comparing base (656afe9) to head (e4aced0).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
cmd/preflight/main_entry.go 0.00% 11 Missing ⚠️

❌ Your patch status has failed because the patch coverage (0.00%) is below the target coverage (70.00%). You can increase the patch coverage or adjust the target coverage.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #82      +/-   ##
==========================================
+ Coverage   91.39%   93.47%   +2.08%     
==========================================
  Files          47       48       +1     
  Lines        1825     1825              
==========================================
+ Hits         1668     1706      +38     
+ Misses        109       84      -25     
+ Partials       48       35      -13     
Files with missing lines Coverage Δ
cmd/preflight/main.go 100.00% <ø> (+37.83%) ⬆️
cmd/preflight/main_entry.go 0.00% <0.00%> (ø)

... and 7 files with indirect coverage changes

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

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 1, 2026

Note

Docstrings generation - SUCCESS
Generated docstrings for this pull request at #83

coderabbitai bot added a commit that referenced this pull request Jan 1, 2026
Docstrings generation was requested by @vertti.

* #82 (comment)

The following files were modified:

* `cmd/preflight/main.go`
* `cmd/preflight/main_entry.go`
@vertti
Copy link
Owner Author

vertti commented Jan 1, 2026

@CodeRabbit 1) i do not want unnecessary noise which your comments would create in the code 2) i do not know what do you even mean by "coverage of docstrings"

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 1, 2026

Tip

For best results, initiate chat on the files or code changes.

@vertti I apologize for any confusion. Let me address your concerns:

  1. No code comments: Understood completely. I won't suggest adding comments or docstrings to your code unless you explicitly request them.

  2. "Coverage of docstrings": I apologize if this caused confusion. I may have been unclear in my previous comment. I'll focus on the actual test coverage improvements you're making in this PR.

I'll respect your workflow preferences and keep feedback focused on what you're asking for. Your PR is successfully improving test coverage to 93.5% with well-structured tests, which is the goal you set out to achieve.

@vertti vertti merged commit 734e126 into main Jan 1, 2026
18 of 19 checks passed
@vertti vertti deleted the improve-test-coverage branch January 1, 2026 12:03
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.

2 participants