test: Add unit tests for internal/gen command flags#27
Closed
miladev95 wants to merge 1 commit intogo-gorm:masterfrom
Closed
test: Add unit tests for internal/gen command flags#27miladev95 wants to merge 1 commit intogo-gorm:masterfrom
miladev95 wants to merge 1 commit intogo-gorm:masterfrom
Conversation
Comment on lines
+28
to
+46
| // Save original RunE and restore later. | ||
| origRunE := cmd.RunE | ||
| defer func() { cmd.RunE = origRunE }() | ||
|
|
||
| // Replace RunE with a no-op to avoid side effects from file operations. | ||
| cmd.RunE = func(cmd *cobra.Command, args []string) error { | ||
| return nil | ||
| } | ||
|
|
||
| // Execute without setting input flag - cobra will check required flags during ParseFlags | ||
| // Use cobra's Flag error handling by calling PreRunE if present then ExecuteC. | ||
| _, err := cmd.ExecuteC() | ||
| if err == nil { | ||
| t.Fatalf("expected error when executing command without required 'input' flag") | ||
| } | ||
|
|
||
| // Now set the required flag and ensure no flag parsing error occurs. | ||
| cmd.SetArgs([]string{"--input", "dummy.go"}) | ||
| _, err = cmd.ExecuteC() |
There was a problem hiding this comment.
[BestPractice]
Test isolation issue: Lines 29-30 save and restore cmd.RunE, but this doesn't fully reset the command state. If ExecuteC() modifies internal cobra command state (parsed flags, etc.), the second ExecuteC() call on line 46 may not execute cleanly. Consider creating a fresh command instance for the second test case:
Suggested change
| // Save original RunE and restore later. | |
| origRunE := cmd.RunE | |
| defer func() { cmd.RunE = origRunE }() | |
| // Replace RunE with a no-op to avoid side effects from file operations. | |
| cmd.RunE = func(cmd *cobra.Command, args []string) error { | |
| return nil | |
| } | |
| // Execute without setting input flag - cobra will check required flags during ParseFlags | |
| // Use cobra's Flag error handling by calling PreRunE if present then ExecuteC. | |
| _, err := cmd.ExecuteC() | |
| if err == nil { | |
| t.Fatalf("expected error when executing command without required 'input' flag") | |
| } | |
| // Now set the required flag and ensure no flag parsing error occurs. | |
| cmd.SetArgs([]string{"--input", "dummy.go"}) | |
| _, err = cmd.ExecuteC() | |
| // Test without required flag | |
| cmd1 := New() | |
| cmd1.RunE = func(cmd *cobra.Command, args []string) error { | |
| return nil | |
| } | |
| _, err := cmd1.ExecuteC() | |
| if err == nil { | |
| t.Fatalf("expected error when executing command without required 'input' flag") | |
| } | |
| // Test with required flag using fresh command | |
| cmd2 := New() | |
| cmd2.RunE = func(cmd *cobra.Command, args []string) error { | |
| return nil | |
| } | |
| cmd2.SetArgs([]string{"--input", "dummy.go"}) | |
| _, err = cmd2.ExecuteC() |
Context for Agents
Test isolation issue: Lines 29-30 save and restore `cmd.RunE`, but this doesn't fully reset the command state. If `ExecuteC()` modifies internal cobra command state (parsed flags, etc.), the second `ExecuteC()` call on line 46 may not execute cleanly. Consider creating a fresh command instance for the second test case:
```suggestion
// Test without required flag
cmd1 := New()
cmd1.RunE = func(cmd *cobra.Command, args []string) error {
return nil
}
_, err := cmd1.ExecuteC()
if err == nil {
t.Fatalf("expected error when executing command without required 'input' flag")
}
// Test with required flag using fresh command
cmd2 := New()
cmd2.RunE = func(cmd *cobra.Command, args []string) error {
return nil
}
cmd2.SetArgs([]string{"--input", "dummy.go"})
_, err = cmd2.ExecuteC()
```
File: internal/gen/gen_test.go
Line: 46
Member
|
Hi @miladev95 thanks for the PR. I think these tests aren't strictly necessary at this stage, especially since the project hasn't reached v1.0 yet. We can keep it simple for now |
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
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.
What did this pull request do?
adds unit tests for
internal/gento verify flags and required-flag behaviorAdd unit tests for flag handling in
internal/gencommandAdds a single test file that exercises the
New()command ininternal/gen, ensuring all expected flags (typed,output,input) are present and that theinputflag is marked as required. Additional coverage verifiescobra.MarkFlagRequirederror behaviour on a standalone command.No production code is modified; only test coverage increases.
Key Changes
• Created
internal/gen/gen_test.gowith 61 new lines of tests• Added
TestNewCommandFlagsto validate flag presence, required-flag enforcement and safe execution paths• Added
TestMarkFlagRequiredBehaviorto assert correct behaviour ofcobra.MarkFlagRequiredwhen the flag existsAffected Areas
•
internal/gen(tests only)This summary was automatically generated by @propel-code-bot