test: add coverage for cmd, style handlers, and main.go #20
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
add more tests for cmd add test for style handlers add test for main.go update go-testcoverag config
📝 WalkthroughWalkthroughExecute() now returns an error and main adds Run() to propagate and handle that error; tests were added/updated for command execution, configuration loading, man generation, and style handlers; test coverage threshold increased and exclude paths adjusted. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Main as Main (main.Run / main.main)
participant Cmd as Cmd (cmd.Execute)
participant Root as Root (rootCmd.Execute)
participant OS as OS (stderr / os.Exit)
User->>Main: start program
Main->>Cmd: Run() calls cmd.Execute()
Cmd->>Root: rootCmd.Execute() runs CLI
Root-->>Cmd: return error or nil
Cmd-->>Main: propagate result
alt error returned
Main->>OS: print error to stderr
Main->>OS: exit(1)
else success
Main-->>User: exit normally
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In `@cmd/man_test.go`:
- Around line 10-26: The man command currently swallows errors from
doc.GenManTree in the manCmd Run handler, so change the Cobra command to use
RunE for manCmd and return any error from doc.GenManTree (propagating filesystem
errors) instead of only printing them; update the TestMan test to assert success
by checking the generated file exists (e.g. require.FileExists for
filepath.Join(destDir, "https-wrench.1.gz")) and to expect an error when running
with a non-existent dir (rootCmd.SetArgs(... "fake-dir") should lead to a
non-nil error), ensuring rootCmd.Execute() surfaces the failure.
In `@cmd/root_test.go`:
- Around line 61-71: The test currently calls Execute() but ignores its return
value so failures won't fail the test; update TestRootCmd_Execute to capture the
result of Execute() (e.g., err := Execute()) and assert it using
require.NoError(t, err) (or require.Error if the expected behavior is a failure)
after LoadConfig(), referencing the TestRootCmd_Execute test, cfgFile,
initConfig, LoadConfig and Execute() so the test fails on unexpected Execute()
outcomes.
- Around line 15-58: The subtest mutates global test state by setting cfgFile
and calling initConfig() (which sets viper global config), so save the original
cfgFile before changing it and restore it after the subtest, and reset viper's
global state to avoid leaking into other tests: in the "LoadConfig embedded
config file" subtest capture old := cfgFile, then set cfgFile =
"./embedded/config-example.yaml", call initConfig(), and defer a cleanup that
restores cfgFile = old and calls viper.Reset() (or reinitializes the viper
instance used by initConfig()) so TestRootCmd_LoadConfig and other tests don't
produce order-dependent failures.
In `@internal/style/style_handlers_test.go`:
- Around line 44-97: In TestPrintKeyInfoStyle update the typo in the test case
name from "ed21219 private key" to "ed25519 private key" inside the tests slice
so the test case for the ed25519Key (variable ed25519Key) correctly reflects the
key type; locate the tests variable in TestPrintKeyInfoStyle and change the name
field of that struct literal only.
- Around line 100-156: The tests compare raw expected strings against
lipgloss-styled outputs (from LgSprintf, StatusCodeParse, BoolStyle) which
include ANSI escape sequences; update the tests to strip ANSI sequences before
asserting by adding a small helper (e.g., StripANSI or stripANSIEscapeCodes) and
call it on the rendered output (the value returned by LgSprintf,
StatusCodeParse, BoolStyle) prior to require.Equal so you compare plain text;
locate usages of LgSprintf, StatusCodeParse, and BoolStyle in the tests and
replace direct assertions with assertions against StripANSI(rendered) ==
expected.
In `@main_test.go`:
- Around line 9-14: Rename the test function named TestMain to TestRun to avoid
shadowing Go's special TestMain(m *testing.M); update the test declaration to
func TestRun(t *testing.T) and keep the inner subtest calling Run() (and
require.NoError), and ensure any references to TestMain in this file or test
suite are updated accordingly so the test still executes as intended.
🧹 Nitpick comments (2)
cmd/root.go (1)
83-90: Consider simplifying the return logic.The error handling can be condensed since
rootCmd.Execute()already returns the appropriate error (or nil).♻️ Suggested simplification
func Execute() error { - err := rootCmd.Execute() - if err != nil { - return err - } - - return nil + return rootCmd.Execute() }main.go (1)
39-42: Consider simplifyingRun().Similar to
Execute(), this can be a one-liner.♻️ Suggested simplification
func Run() error { - err := cmd.Execute() - return err + return cmd.Execute() }
add cleanup function to tests in cmd/root
fix error handling, typos, file checks
xenOs76
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@cmd/root_test.go`:
- Around line 78-89: TestRootCmd_Execute mutates the global cfgFile and viper
state via initConfig() but doesn't restore them; save the original cfgFile at
the start of the subtest, and defer restoring it and re-running initConfig() (or
otherwise resetting viper) so global state is returned to its prior value;
ensure you reference cfgFile and initConfig() in the defer to mirror the cleanup
pattern used in TestRootCmd_LoadConfig and TestRootCmd.
In `@internal/style/style_handlers_test.go`:
- Line 174: The long HTML sample line in the raw string inside
internal/style/style_handlers_test.go exceeds the configured line-length; fix it
by either splitting that long literal into multiple concatenated lines
(preserving the test assertions and exact content) or append a targeted nolint
directive to suppress revive for that line (e.g., add a //nolint:revive comment
on the offending raw string line) so golangci-lint no longer warns; locate the
long line by searching for the raw HTML sample used in the test and update the
raw string accordingly.
♻️ Duplicate comments (1)
internal/style/style_handlers_test.go (1)
100-155: Strip ANSI escape codes before assertions in style-output tests.
LgSprintf,StatusCodeParse, andBoolStylerender via lipgloss, which emits ANSI sequences; direct string equality is brittle and likely to fail. This was already flagged in prior review.🧽 Suggested fix (strip ANSI before comparing)
import ( "bytes" "crypto" "crypto/ecdsa" "crypto/ed25519" "crypto/elliptic" "crypto/rand" "crypto/rsa" "fmt" + "regexp" "testing" "github.com/stretchr/testify/require" ) +var ansiRE = regexp.MustCompile(`\x1b\[[0-9;]*m`) + +func stripANSI(s string) string { + return ansiRE.ReplaceAllString(s, "") +} ... - simple := LgSprintf(Status, "%s", tt) + simple := stripANSI(LgSprintf(Status, "%s", tt)) require.Equal(t, tt, simple) - pattern := LgSprintf(Status, "%s - lorem ipsum", tt) + pattern := stripANSI(LgSprintf(Status, "%s - lorem ipsum", tt)) require.Equal(t, tt+" - lorem ipsum", pattern) ... - s := StatusCodeParse(tt.statusInt) + s := stripANSI(StatusCodeParse(tt.statusInt)) require.Equal(t, tt.statusString, s) ... - s := BoolStyle(tt.statusBool) + s := stripANSI(BoolStyle(tt.statusBool)) require.Equal(t, tt.statusString, s)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
add more tests for cmd
add test for style handlers
add test for main.go
update go-testcoverag config
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.