Add comprehensive feature coverage for Console contracts in tests/feature#107
Add comprehensive feature coverage for Console contracts in tests/feature#107
tests/feature#107Conversation
Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>
Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>
Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>
tests/feature
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughA new end-to-end test suite for console commands is added to test command registration and execution with argument/option parsing, defaults, and timestamp handling through the Artisan CLI. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
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 review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (3)
tests/feature/console_test.go (3)
222-226: Consider test isolation with shared command state.Using
SetupSuitemeans the same command instances are reused across all tests. Sincecaptureis overwritten on eachHandlecall, this works for the current tests. However, if tests run in parallel or if you later add tests that don't invoke commands,capturemay hold stale state from previous tests.Consider using
SetupTestinstead if test isolation becomes a concern, or uses.Require().NotNil(capture)assertions (which you already do) to guard against stale state.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/feature/console_test.go` around lines 222 - 226, The test suite currently registers shared command instances in SetupSuite (consoleSingleCommand and consoleSliceCommand) which can cause stale shared state in capture across tests; change setup to register fresh command instances per test by moving the registration into SetupTest (create new consoleSingleCommand/consoleSliceCommand and call facades.Artisan().Register there) or, if you must keep SetupSuite, add per-test guards such as s.Require().NotNil(capture) before using capture (and ensure Handle resets capture) to avoid leaking state between tests.
222-226: Consider adding TeardownSuite for cleanup.The registered commands remain in the Artisan registry after tests complete. If other test suites in the same package also use
facades.Artisan(), these commands will persist. Consider whether cleanup is needed or verify the framework resets state between test suites.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/feature/console_test.go` around lines 222 - 226, The test registers commands in SetupSuite via facades.Artisan().Register using consoleSingleCommand and consoleSliceCommand but never unregisters them; add a TeardownSuite method that removes those commands (or resets the Artisan registry) to avoid leaking state across suites — implement TeardownSuite on ConsoleTestSuite to call the appropriate facades.Artisan().Unregister or facades.Artisan().Reset (or equivalent) for consoleSingleCommand and consoleSliceCommand so other tests start with a clean registry.
307-309: Consider using element-wise delta comparison for float slices.
s.Equalperforms exact comparison on float slices. While this may pass if parsing is deterministic, floating-point representation issues could cause flaky failures, especially for[]float32values like1.1which cannot be represented exactly.♻️ Suggested improvement for robust float slice assertions
- s.Equal([]float32{1.1, 2.2}, capture.ArgumentFloat32Slice) - s.Equal([]float64{3.3, 4.4}, capture.ArgumentFloat64Slice) + s.Require().Len(capture.ArgumentFloat32Slice, 2) + s.InDelta(1.1, float64(capture.ArgumentFloat32Slice[0]), 0.0001) + s.InDelta(2.2, float64(capture.ArgumentFloat32Slice[1]), 0.0001) + s.Require().Len(capture.ArgumentFloat64Slice, 2) + s.InDelta(3.3, capture.ArgumentFloat64Slice[0], 0.0001) + s.InDelta(4.4, capture.ArgumentFloat64Slice[1], 0.0001)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/feature/console_test.go` around lines 307 - 309, The tests currently use exact equality for floating slices (capture.ArgumentFloat32Slice and capture.ArgumentFloat64Slice) which can be flaky; replace those s.Equal assertions with element-wise delta comparisons—e.g., use s.InDeltaSlice(expectedFloat32Slice, capture.ArgumentFloat32Slice, 1e-6) and s.InDeltaSlice(expectedFloat64Slice, capture.ArgumentFloat64Slice, 1e-9) or alternatively iterate and call s.InDelta on each pair—to allow small representation differences while keeping assertions strict.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/feature/console_test.go`:
- Around line 222-226: The test suite currently registers shared command
instances in SetupSuite (consoleSingleCommand and consoleSliceCommand) which can
cause stale shared state in capture across tests; change setup to register fresh
command instances per test by moving the registration into SetupTest (create new
consoleSingleCommand/consoleSliceCommand and call facades.Artisan().Register
there) or, if you must keep SetupSuite, add per-test guards such as
s.Require().NotNil(capture) before using capture (and ensure Handle resets
capture) to avoid leaking state between tests.
- Around line 222-226: The test registers commands in SetupSuite via
facades.Artisan().Register using consoleSingleCommand and consoleSliceCommand
but never unregisters them; add a TeardownSuite method that removes those
commands (or resets the Artisan registry) to avoid leaking state across suites —
implement TeardownSuite on ConsoleTestSuite to call the appropriate
facades.Artisan().Unregister or facades.Artisan().Reset (or equivalent) for
consoleSingleCommand and consoleSliceCommand so other tests start with a clean
registry.
- Around line 307-309: The tests currently use exact equality for floating
slices (capture.ArgumentFloat32Slice and capture.ArgumentFloat64Slice) which can
be flaky; replace those s.Equal assertions with element-wise delta
comparisons—e.g., use s.InDeltaSlice(expectedFloat32Slice,
capture.ArgumentFloat32Slice, 1e-6) and s.InDeltaSlice(expectedFloat64Slice,
capture.ArgumentFloat64Slice, 1e-9) or alternatively iterate and call s.InDelta
on each pair—to allow small representation differences while keeping assertions
strict.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7a623304-dd84-41ea-8dbf-236668e22cc3
📒 Files selected for processing (1)
tests/feature/console_test.go
tests/feature/console_test.go
Outdated
| type consoleSingleCapture struct { | ||
| OptionBool bool | ||
| OptionFloat64 float64 | ||
| OptionFloat64Slice []float64 | ||
| OptionInt int | ||
| OptionIntSlice []int | ||
| OptionInt64 int64 | ||
| OptionInt64Slice []int64 | ||
| OptionString string | ||
| OptionStringSlice []string | ||
|
|
||
| OptionDefaultString string | ||
| OptionDefaultInt int | ||
| OptionDefaultBool bool | ||
|
|
||
| ArgumentFloat32 float32 | ||
| ArgumentFloat64 float64 | ||
| ArgumentInt int | ||
| ArgumentInt8 int8 | ||
| ArgumentInt16 int16 | ||
| ArgumentInt32 int32 | ||
| ArgumentInt64 int64 | ||
| ArgumentUint uint | ||
| ArgumentUint8 uint8 | ||
| ArgumentUint16 uint16 | ||
| ArgumentUint32 uint32 | ||
| ArgumentUint64 uint64 | ||
| ArgumentTimestamp time.Time | ||
|
|
||
| ArgumentDefaultString string | ||
| ArgumentDefaultInt int | ||
| } | ||
|
|
||
| type consoleSingleCommand struct { | ||
| capture *consoleSingleCapture | ||
| } | ||
|
|
||
| func (r *consoleSingleCommand) Signature() string { | ||
| return "test:console-single" | ||
| } | ||
|
|
||
| func (r *consoleSingleCommand) Description() string { | ||
| return "Test console command with single arguments and all flag types" | ||
| } | ||
|
|
||
| func (r *consoleSingleCommand) Extend() command.Extend { | ||
| return command.Extend{ | ||
| Flags: []command.Flag{ | ||
| &command.BoolFlag{Name: "bool"}, | ||
| &command.Float64Flag{Name: "float64"}, | ||
| &command.Float64SliceFlag{Name: "float64-slice"}, | ||
| &command.IntFlag{Name: "int"}, | ||
| &command.IntSliceFlag{Name: "int-slice"}, | ||
| &command.Int64Flag{Name: "int64"}, | ||
| &command.Int64SliceFlag{Name: "int64-slice"}, | ||
| &command.StringFlag{Name: "string"}, | ||
| &command.StringSliceFlag{Name: "string-slice"}, | ||
| }, | ||
| Arguments: []command.Argument{ | ||
| &command.ArgumentFloat32{Name: "float32Arg", Required: true}, | ||
| &command.ArgumentFloat64{Name: "float64Arg", Required: true}, | ||
| &command.ArgumentInt{Name: "intArg", Required: true}, | ||
| &command.ArgumentInt8{Name: "int8Arg", Required: true}, | ||
| &command.ArgumentInt16{Name: "int16Arg", Required: true}, | ||
| &command.ArgumentInt32{Name: "int32Arg", Required: true}, | ||
| &command.ArgumentInt64{Name: "int64Arg", Required: true}, | ||
| &command.ArgumentUint{Name: "uintArg", Required: true}, | ||
| &command.ArgumentUint8{Name: "uint8Arg", Required: true}, | ||
| &command.ArgumentUint16{Name: "uint16Arg", Required: true}, | ||
| &command.ArgumentUint32{Name: "uint32Arg", Required: true}, | ||
| &command.ArgumentUint64{Name: "uint64Arg", Required: true}, | ||
| &command.ArgumentTimestamp{Name: "timestampArg", Required: true, Layouts: []string{time.RFC3339}}, | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| func (r *consoleSingleCommand) Handle(ctx console.Context) error { | ||
| ctx.Comment("console single command") | ||
| ctx.Info("running single command") | ||
| ctx.Warning("warning") | ||
| ctx.Success("success") | ||
|
|
||
| r.capture = &consoleSingleCapture{ | ||
| OptionBool: ctx.OptionBool("bool"), | ||
| OptionFloat64: ctx.OptionFloat64("float64"), | ||
| OptionFloat64Slice: ctx.OptionFloat64Slice("float64-slice"), | ||
| OptionInt: ctx.OptionInt("int"), | ||
| OptionIntSlice: ctx.OptionIntSlice("int-slice"), | ||
| OptionInt64: ctx.OptionInt64("int64"), | ||
| OptionInt64Slice: ctx.OptionInt64Slice("int64-slice"), | ||
| OptionString: ctx.Option("string"), | ||
| OptionStringSlice: ctx.OptionSlice("string-slice"), | ||
|
|
||
| OptionDefaultString: ctx.Option("missing-option"), | ||
| OptionDefaultInt: ctx.OptionInt("missing-option"), | ||
| OptionDefaultBool: ctx.OptionBool("missing-option"), | ||
|
|
||
| ArgumentFloat32: ctx.ArgumentFloat32("float32Arg"), | ||
| ArgumentFloat64: ctx.ArgumentFloat64("float64Arg"), | ||
| ArgumentInt: ctx.ArgumentInt("intArg"), | ||
| ArgumentInt8: ctx.ArgumentInt8("int8Arg"), | ||
| ArgumentInt16: ctx.ArgumentInt16("int16Arg"), | ||
| ArgumentInt32: ctx.ArgumentInt32("int32Arg"), | ||
| ArgumentInt64: ctx.ArgumentInt64("int64Arg"), | ||
| ArgumentUint: ctx.ArgumentUint("uintArg"), | ||
| ArgumentUint8: ctx.ArgumentUint8("uint8Arg"), | ||
| ArgumentUint16: ctx.ArgumentUint16("uint16Arg"), | ||
| ArgumentUint32: ctx.ArgumentUint32("uint32Arg"), | ||
| ArgumentUint64: ctx.ArgumentUint64("uint64Arg"), | ||
| ArgumentTimestamp: ctx.ArgumentTimestamp("timestampArg"), | ||
|
|
||
| ArgumentDefaultString: ctx.ArgumentString("missing-argument"), | ||
| ArgumentDefaultInt: ctx.ArgumentInt("missing-argument"), | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
| type consoleSliceCapture struct { | ||
| ArgumentStringSlice []string | ||
| ArgumentFloat32Slice []float32 | ||
| ArgumentFloat64Slice []float64 | ||
| ArgumentIntSlice []int | ||
| ArgumentInt8Slice []int8 | ||
| ArgumentInt16Slice []int16 | ||
| ArgumentInt32Slice []int32 | ||
| ArgumentInt64Slice []int64 | ||
| ArgumentUintSlice []uint | ||
| ArgumentUint8Slice []uint8 | ||
| ArgumentUint16Slice []uint16 | ||
| ArgumentUint32Slice []uint32 | ||
| ArgumentUint64Slice []uint64 | ||
| ArgumentTimestampSlice []time.Time | ||
|
|
||
| MissingStringSliceIsNil bool | ||
| MissingFloat32SliceIsNil bool | ||
| MissingTimestampIsNil bool | ||
| } | ||
|
|
||
| type consoleSliceCommand struct { | ||
| capture *consoleSliceCapture | ||
| } | ||
|
|
||
| func (r *consoleSliceCommand) Signature() string { | ||
| return "test:console-slice" | ||
| } | ||
|
|
||
| func (r *consoleSliceCommand) Description() string { | ||
| return "Test console command with slice arguments" | ||
| } | ||
|
|
||
| func (r *consoleSliceCommand) Extend() command.Extend { | ||
| return command.Extend{ | ||
| Arguments: []command.Argument{ | ||
| &command.ArgumentStringSlice{Name: "stringSliceArg", Min: 1, Max: 2}, | ||
| &command.ArgumentFloat32Slice{Name: "float32SliceArg", Min: 1, Max: 2}, | ||
| &command.ArgumentFloat64Slice{Name: "float64SliceArg", Min: 1, Max: 2}, | ||
| &command.ArgumentIntSlice{Name: "intSliceArg", Min: 1, Max: 2}, | ||
| &command.ArgumentInt8Slice{Name: "int8SliceArg", Min: 1, Max: 2}, | ||
| &command.ArgumentInt16Slice{Name: "int16SliceArg", Min: 1, Max: 2}, | ||
| &command.ArgumentInt32Slice{Name: "int32SliceArg", Min: 1, Max: 2}, | ||
| &command.ArgumentInt64Slice{Name: "int64SliceArg", Min: 1, Max: 2}, | ||
| &command.ArgumentUintSlice{Name: "uintSliceArg", Min: 1, Max: 2}, | ||
| &command.ArgumentUint8Slice{Name: "uint8SliceArg", Min: 1, Max: 2}, | ||
| &command.ArgumentUint16Slice{Name: "uint16SliceArg", Min: 1, Max: 2}, | ||
| &command.ArgumentUint32Slice{Name: "uint32SliceArg", Min: 1, Max: 2}, | ||
| &command.ArgumentUint64Slice{Name: "uint64SliceArg", Min: 1, Max: 2}, | ||
| &command.ArgumentTimestampSlice{Name: "timestampSliceArg", Min: 1, Max: 2, Layouts: []string{time.RFC3339}}, | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| func (r *consoleSliceCommand) Handle(ctx console.Context) error { | ||
| r.capture = &consoleSliceCapture{ | ||
| ArgumentStringSlice: ctx.ArgumentStringSlice("stringSliceArg"), | ||
| ArgumentFloat32Slice: ctx.ArgumentFloat32Slice("float32SliceArg"), | ||
| ArgumentFloat64Slice: ctx.ArgumentFloat64Slice("float64SliceArg"), | ||
| ArgumentIntSlice: ctx.ArgumentIntSlice("intSliceArg"), | ||
| ArgumentInt8Slice: ctx.ArgumentInt8Slice("int8SliceArg"), | ||
| ArgumentInt16Slice: ctx.ArgumentInt16Slice("int16SliceArg"), | ||
| ArgumentInt32Slice: ctx.ArgumentInt32Slice("int32SliceArg"), | ||
| ArgumentInt64Slice: ctx.ArgumentInt64Slice("int64SliceArg"), | ||
| ArgumentUintSlice: ctx.ArgumentUintSlice("uintSliceArg"), | ||
| ArgumentUint8Slice: ctx.ArgumentUint8Slice("uint8SliceArg"), | ||
| ArgumentUint16Slice: ctx.ArgumentUint16Slice("uint16SliceArg"), | ||
| ArgumentUint32Slice: ctx.ArgumentUint32Slice("uint32SliceArg"), | ||
| ArgumentUint64Slice: ctx.ArgumentUint64Slice("uint64SliceArg"), | ||
| ArgumentTimestampSlice: ctx.ArgumentTimestampSlice("timestampSliceArg"), | ||
|
|
||
| MissingStringSliceIsNil: ctx.ArgumentStringSlice("missing-slice") == nil, | ||
| MissingFloat32SliceIsNil: ctx.ArgumentFloat32Slice("missing-slice") == nil, | ||
| MissingTimestampIsNil: ctx.ArgumentTimestampSlice("missing-slice") == nil, | ||
| } | ||
|
|
||
| return nil | ||
| } |
There was a problem hiding this comment.
@copilot The commands should be moved to the app/conolse/commands and should be registered in bootstrap/app.go by WithCommands.
Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>
Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>
Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>
Co-authored-by: hwbrzzl <24771476+hwbrzzl@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. |
This PR adds integration-style coverage for the Console module in
goravel/example, focused on validating behavior exposed bygoravel/framework/contracts/console.The goal is to ensure command flag/argument parsing and retrieval semantics are exercised end-to-end from the example app.
Scope
tests/feature/console_test.goapp/console/commandsWithCommands(Commands)inbootstrap/app.goConsole contract coverage added
facades.Artisan().Run(...)facades.Artisan().Call(...)OptionBool,OptionFloat64,OptionFloat64Slice,OptionInt,OptionIntSlice,OptionInt64,OptionInt64Slice,Option,OptionSliceTest design
test:console-singlefor full flag + single-argument coveragetest:console-slicefor slice-argument coveragetests.TestCase(alongsidesuite.Suite) to align with existing feature test conventionsapp/console/commandsto avoid shared-state racesOriginal prompt
📍 Connect Copilot coding agent with Jira, Azure Boards or Linear to delegate work to Copilot in one click without leaving your project management tool.
Console Module Feature Tests
This PR adds comprehensive integration tests for the Console module by introducing a new feature test suite that exercises the full contract surface defined in
goravel/framework/contracts/console.Main Features
Two Test Commands
test:console-single: Exercises all flag types and single-value arguments with parsing and default-value validationtest:console-slice: Exercises slice arguments across all numeric and string types with nil-behavior verificationComplete Flag Type Coverage
Single-Value Argument Types
Slice Argument Types
Execution Methods
facades.Artisan().Run()for direct argument array executionfacades.Artisan().Call()for string-based command invocationfacades.Artisan().Register()Fallback Semantics
Test Examples
The
TestRunSingleCommandmethod executes the single-argument command with all flags and arguments present, validating parsed values. TheTestRunSliceCommandmethod verifies slice argument handling with multiple values and nil checks for missing slices. TheTestCallSingleCommanddemonstrates the string-based invocation API.Statistics: