Conversation
WalkthroughThe PR refactors flag and option handling for Tree and Table formatters by converting them from boolean to pointer-to-boolean types, introduces new reflection-based utility functions, adds CSS class deduplication logic, enhances error and debug logging, and disables several tests via function renaming. Changes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
formatters/tests/map_fields_test.go (1)
55-86: Fix syntax error from incomplete comment block.The test code starting at line 55 is commented out with
//FIXME, but the test body (lines 62-86) remains uncommented, creating invalid syntax. The closing brace at line 86 doesn't match any opening brace, causing the compilation error flagged by the pipeline.Apply this diff to properly comment out the entire disabled test:
// Test ParseDataWithSchema //FIXME - // t.Run("ParseDataWithSchema", func(t *testing.T) { - // prettyData, err := parser.ParseDataWithSchema(testData, schema) - // if err != nil { - // t.Fatalf("ParseDataWithSchema failed: %v", err) - // } - - // Check that scalar fields are parsed - if _, exists := prettyData.GetValue("name"); !exists { - t.Error("name field not found in Values") - } - if _, exists := prettyData.GetValue("age"); !exists { - t.Error("age field not found in Values") - } - - // Check that map fields are parsed - if _, exists := prettyData.GetValue("address"); !exists { - t.Error("address map field not found in Values") - } - if _, exists := prettyData.GetValue("metadata"); !exists { - t.Error("metadata map field not found in Values") - } - - // Check that table data is parsed - if _, exists := prettyData.GetTable("items"); !exists { - t.Error("items table not found in Tables") - } - - if table, exists := prettyData.GetTable("items"); exists && len(table.Rows) != 2 { - t.Errorf("Expected 2 items in table, got %d", len(table.Rows)) - } - }) + // t.Run("ParseDataWithSchema", func(t *testing.T) { + // prettyData, err := parser.ParseDataWithSchema(testData, schema) + // if err != nil { + // t.Fatalf("ParseDataWithSchema failed: %v", err) + // } + // + // // Check that scalar fields are parsed + // if _, exists := prettyData.GetValue("name"); !exists { + // t.Error("name field not found in Values") + // } + // if _, exists := prettyData.GetValue("age"); !exists { + // t.Error("age field not found in Values") + // } + // + // // Check that map fields are parsed + // if _, exists := prettyData.GetValue("address"); !exists { + // t.Error("address map field not found in Values") + // } + // if _, exists := prettyData.GetValue("metadata"); !exists { + // t.Error("metadata map field not found in Values") + // } + // + // // Check that table data is parsed + // if _, exists := prettyData.GetTable("items"); !exists { + // t.Error("items table not found in Tables") + // } + // + // if table, exists := prettyData.GetTable("items"); exists && len(table.Rows) != 2 { + // t.Errorf("Expected 2 items in table, got %d", len(table.Rows)) + // } + // })formatters/options.go (1)
37-38: *Fix field types: Tree and Table must be bool, not bool.Lines 37-38 declare
TreeandTableasbool, but all the new code in this file (lines 48-54, 80-85, 145-153, 174-182) and related files (formatters/manager.go, flags.go) treats them as*boolwith nil-checks and pointer dereferencing. This fundamental type mismatch is causing compilation failures throughout the codebase.Apply this diff to fix the field declarations:
// Display structure flags (additive with format flags) - Tree bool `json:"tree,omitempty"` // Display in tree structure - Table bool `json:"table,omitempty"` // Display in table structure + Tree *bool `json:"tree,omitempty"` // Display in tree structure + Table *bool `json:"table,omitempty"` // Display in table structureThis change will resolve the compilation errors in:
- formatters/manager.go (lines 211, 213)
- formatters/options.go (lines 49, 53, 80-85, 146-147, 150-152, 175-176, 179-181)
- flags.go (lines 70, 75)
🧹 Nitpick comments (2)
formatters/reflect.go (2)
80-140: Consider early return consistency and potential nil slice input.Two observations:
- Lines 97-105: Excessive blank lines between
result = append(...)and theelsebranches reduce readability.- The function returns
(nil, false)for empty input, but(result, len(result) > 0)at line 107 returns([], false)when the input slice is non-empty but contains zero elements that match type T—this is inconsistent with Case 2 which returns(result, len(result) > 0)at line 139.if typed, ok := elem.Interface().(T); ok { result = append(result, typed) - } else { - return nil, false // Not all elements are T } } else { - return nil, false }
207-214: Redundant case-folding:strings.EqualFoldalready handles case-insensitive comparison.
strings.EqualFold(field.Name, lowerName)comparesfield.Nameagainst a pre-lowercasedname. This works but is slightly misleading—EqualFoldperforms its own case-insensitive comparison, so pre-loweringnameis unnecessary.// Try case-insensitive match - lowerName := strings.ToLower(name) for i := 0; i < typ.NumField(); i++ { field := typ.Field(i) - if strings.EqualFold(field.Name, lowerName) { + if strings.EqualFold(field.Name, name) { return val.Field(i) } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
api/text.go(1 hunks)cobra_command.go(1 hunks)exec/exec.go(1 hunks)flags.go(1 hunks)formatters/manager.go(1 hunks)formatters/options.go(5 hunks)formatters/parser.go(3 hunks)formatters/reflect.go(1 hunks)formatters/tests/formatters_test.go(3 hunks)formatters/tests/map_fields_test.go(1 hunks)formatters/tests/parser_test.go(1 hunks)formatters/tests/sorting_test.go(1 hunks)formatters/tests/tree_test.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
formatters/tests/parser_test.go (1)
formatters/options.go (1)
FormatOptions(18-46)
formatters/options.go (2)
format.go (1)
FormatOptions(17-17)api/icons/icons.go (1)
Table(195-195)
cobra_command.go (1)
format.go (1)
Errorf(51-53)
api/text.go (2)
format.go (1)
Text(112-117)api/tailwind/tailwind.go (1)
Style(244-261)
formatters/parser.go (1)
api/meta.go (1)
PrettyData(18-24)
exec/exec.go (3)
format.go (2)
Tracef(65-69)Text(112-117)api/text.go (1)
Text(50-57)api/icons/icons.go (2)
MinimalArrow(156-156)Add(211-211)
🪛 GitHub Actions: Lint
formatters/parser.go
[error] 558-558: golangci-lint: undefined: api.TypeOptions
formatters/tests/map_fields_test.go
[error] 86-86: Go compiler error: expected declaration, found ')'
🪛 GitHub Actions: PR #54
formatters/parser.go
[error] 558-558: Build failed: undefined: api.TypeOptions (formatters/parser.go:558:35) during 'go build -o clicky ./cmd/clicky/'
🪛 GitHub Actions: Test
formatters/parser.go
[error] 558-558: compile error: undefined: api.TypeOptions in formatters/parser.go:558
🪛 GitHub Check: golangci-lint
formatters/manager.go
[failure] 213-213:
invalid operation: cannot indirect options.Table (variable of type bool)
[failure] 213-213:
invalid operation: options.Table != nil (mismatched types bool and untyped nil)
[failure] 211-211:
invalid operation: cannot indirect options.Tree (variable of type bool)
[failure] 211-211:
invalid operation: options.Tree != nil (mismatched types bool and untyped nil)
formatters/options.go
[failure] 49-49:
invalid operation: cannot indirect o.Tree (variable of type bool)
[failure] 49-49:
invalid operation: o.Tree != nil (mismatched types bool and untyped nil)
[failure] 49-49:
invalid operation: cannot indirect o.Table (variable of type bool)
[failure] 49-49:
invalid operation: o.Table != nil (mismatched types bool and untyped nil)
formatters/parser.go
[failure] 568-568:
undefined: api.TypeOptions
[failure] 558-558:
undefined: api.TypeOptions
🪛 GitHub Check: Lint
formatters/manager.go
[failure] 213-213:
invalid operation: cannot indirect options.Table (variable of type bool)
[failure] 213-213:
invalid operation: options.Table != nil (mismatched types bool and untyped nil)
[failure] 211-211:
invalid operation: cannot indirect options.Tree (variable of type bool)
[failure] 211-211:
invalid operation: options.Tree != nil (mismatched types bool and untyped nil)
formatters/options.go
[failure] 49-49:
invalid operation: cannot indirect o.Tree (variable of type bool)
[failure] 49-49:
invalid operation: o.Tree != nil (mismatched types bool and untyped nil)
[failure] 49-49:
invalid operation: cannot indirect o.Table (variable of type bool)
[failure] 49-49:
invalid operation: o.Table != nil (mismatched types bool and untyped nil)
formatters/parser.go
[failure] 568-568:
undefined: api.TypeOptions
[failure] 558-558:
undefined: api.TypeOptions
🪛 GitHub Check: Test (ubuntu-latest, Go 1.25)
formatters/manager.go
[failure] 213-213:
invalid operation: cannot indirect options.Table (variable of type bool)
[failure] 213-213:
invalid operation: options.Table != nil (mismatched types bool and untyped nil)
[failure] 211-211:
invalid operation: cannot indirect options.Tree (variable of type bool)
[failure] 211-211:
invalid operation: options.Tree != nil (mismatched types bool and untyped nil)
formatters/options.go
[failure] 49-49:
invalid operation: cannot indirect o.Tree (variable of type bool)
[failure] 49-49:
invalid operation: o.Tree != nil (mismatched types bool and untyped nil)
[failure] 49-49:
invalid operation: cannot indirect o.Table (variable of type bool)
[failure] 49-49:
invalid operation: o.Table != nil (mismatched types bool and untyped nil)
formatters/parser.go
[failure] 568-568:
undefined: api.TypeOptions
[failure] 558-558:
undefined: api.TypeOptions
🔇 Additional comments (6)
exec/exec.go (1)
586-586: LGTM: Appropriate log level adjustment.Changing from
DebugftoTracefcorrectly increases verbosity for detailed command execution tracing, consistent with the trace-level output at line 636.cobra_command.go (1)
175-175: LGTM: Enhanced error logging.Adding error logging before returning provides better debugging visibility for command failures. The use of
WithSkipReportLevel(1)correctly adjusts the call stack for accurate error reporting.formatters/reflect.go (2)
11-73: LGTM! Well-structured slice flattening with proper nil handling.The function correctly handles edge cases: non-slice inputs, empty slices, nil outer elements, and interface dereferencing. The logic flow is clear and the comments are helpful.
142-166: > Likely an incorrect or invalid review comment.formatters/tests/formatters_test.go (2)
88-90: LGTM! Test expectation aligned with RFC3339 format.The updated expectation
"2024-01-15T10:30:00Z"correctly reflects ISO8601/RFC3339 formatting for JSON output, which is the standard for interoperable date serialization.
123-125: LGTM! Consistent date format expectation for YAML.The YAML formatter now uses the same RFC3339 format as JSON, ensuring consistency across structured output formats.
| func uniqueStyles(existing string, styles ...string) string { | ||
| styleSet := make(map[string]struct{}) | ||
| if existing != "" { | ||
| for _, s := range strings.Split(existing, " ") { | ||
| styleSet[s] = struct{}{} | ||
| } | ||
| } | ||
| for _, style := range styles { | ||
| for _, s := range strings.Split(style, " ") { | ||
| if s != "" { | ||
| styleSet[s] = struct{}{} | ||
| } | ||
| } | ||
| } | ||
| uniq := "" | ||
| for s := range styleSet { | ||
| if uniq == "" { | ||
| uniq = s | ||
| } else { | ||
| uniq += " " + s | ||
| } | ||
| } | ||
|
|
||
| return uniq | ||
| } |
There was a problem hiding this comment.
Non-deterministic style ordering due to map iteration.
The uniqueStyles function builds the result by iterating over a map (lines 232-237), which has non-deterministic order in Go. This will produce inconsistent CSS class ordering across runs.
Apply this diff to ensure consistent, sorted output:
func uniqueStyles(existing string, styles ...string) string {
styleSet := make(map[string]struct{})
if existing != "" {
for _, s := range strings.Split(existing, " ") {
styleSet[s] = struct{}{}
}
}
for _, style := range styles {
for _, s := range strings.Split(style, " ") {
if s != "" {
styleSet[s] = struct{}{}
}
}
}
- uniq := ""
+ // Convert to sorted slice for deterministic output
+ uniqueList := make([]string, 0, len(styleSet))
for s := range styleSet {
- if uniq == "" {
- uniq = s
- } else {
- uniq += " " + s
- }
+ uniqueList = append(uniqueList, s)
}
-
- return uniq
+ sort.Strings(uniqueList)
+ return strings.Join(uniqueList, " ")
}Note: You'll need to add "sort" to the imports at the top of the file.
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In api/text.go around lines 217 to 241, iteration over the map `styleSet`
produces non-deterministic class ordering; change the function to collect map
keys into a slice, sort the slice (add "sort" to imports), then join the sorted
keys with spaces to build and return a deterministic string of unique styles.
| flags.BoolFunc("tree", "Display in tree structure (additive with format)", func(s string) error { | ||
| Flags.Tree = lo.ToPtr(s == "true") | ||
| return nil | ||
| }) | ||
| flags.BoolFunc("table", "Display in table structure (additive with format), or false to disable tables", | ||
| func(b string) error { | ||
| Flags.Table = lo.ToPtr(b == "true") | ||
| return nil | ||
| }) |
There was a problem hiding this comment.
*Fix type mismatch: assigning bool to bool field.
Lines 70 and 75 assign lo.ToPtr(...) (which returns *bool) to Flags.Tree and Flags.Table, but these fields are declared as bool in the FormatOptions struct, not *bool. This causes the same compilation errors seen in formatters/manager.go.
The field declarations must be updated in formatters/options.go:
// In formatters/options.go FormatOptions struct (lines 37-38):
Tree *bool `json:"tree,omitempty"` // Display in tree structure
Table *bool `json:"table,omitempty"` // Display in table structureOnce corrected, this pointer-based flag binding will work as intended.
🤖 Prompt for AI Agents
In flags.go around lines 69 to 77 there is a type mismatch: the flag handlers
assign lo.ToPtr(...) (type *bool) to Flags.Tree and Flags.Table, but those
fields are declared as bool; update the FormatOptions fields in
formatters/options.go (where Tree and Table are defined, around lines 37-38) to
be pointer types (Tree *bool `json:"tree,omitempty"` and Table *bool
`json:"table,omitempty"`) so the flag binding uses the correct *bool types and
the compilation errors are resolved.
| if options.Tree != nil && *options.Tree { | ||
| return f.treeFormatter.Format(data...) | ||
| } else if options.Table { | ||
| } else if options.Table != nil && *options.Table { |
There was a problem hiding this comment.
*Fix type mismatch: Tree and Table are bool, not bool.
The code attempts nil checks and pointer dereferencing on options.Tree and options.Table, but these fields are declared as bool in FormatOptions (line 37-38 in formatters/options.go), not *bool. This causes compilation failures across multiple files.
The field declarations in FormatOptions need to be updated to pointer types:
// In formatters/options.go, update lines 37-38:
Tree *bool `json:"tree,omitempty"` // Display in tree structure
Table *bool `json:"table,omitempty"` // Display in table structureOnce the field types are corrected, this pointer-checking logic will work correctly.
🧰 Tools
🪛 GitHub Check: golangci-lint
[failure] 213-213:
invalid operation: cannot indirect options.Table (variable of type bool)
[failure] 213-213:
invalid operation: options.Table != nil (mismatched types bool and untyped nil)
[failure] 211-211:
invalid operation: cannot indirect options.Tree (variable of type bool)
[failure] 211-211:
invalid operation: options.Tree != nil (mismatched types bool and untyped nil)
🪛 GitHub Check: Lint
[failure] 213-213:
invalid operation: cannot indirect options.Table (variable of type bool)
[failure] 213-213:
invalid operation: options.Table != nil (mismatched types bool and untyped nil)
[failure] 211-211:
invalid operation: cannot indirect options.Tree (variable of type bool)
[failure] 211-211:
invalid operation: options.Tree != nil (mismatched types bool and untyped nil)
🪛 GitHub Check: Test (ubuntu-latest, Go 1.25)
[failure] 213-213:
invalid operation: cannot indirect options.Table (variable of type bool)
[failure] 213-213:
invalid operation: options.Table != nil (mismatched types bool and untyped nil)
[failure] 211-211:
invalid operation: cannot indirect options.Tree (variable of type bool)
[failure] 211-211:
invalid operation: options.Tree != nil (mismatched types bool and untyped nil)
🤖 Prompt for AI Agents
In formatters/manager.go around lines 211-213 the code treats options.Tree and
options.Table as *bool, but FormatOptions currently declares them as bool;
update the declarations in formatters/options.go (around lines 37-38) to be
pointer types (Tree *bool and Table *bool) so the nil checks and dereferences
are valid, then rebuild to catch and fix any remaining type mismatches in other
files.
| func mergeTypeOptions(opts ...api.TypeOptions) api.TypeOptions { | ||
| merged := api.TypeOptions{} | ||
| for _, opt := range opts { | ||
| merged.SkipTable = merged.SkipTable || opt.SkipTable | ||
| merged.SkipTree = merged.SkipTree || opt.SkipTree | ||
| } | ||
| return merged | ||
| } | ||
|
|
||
| // ToPrettyData converts various input types to PrettyData | ||
| func ToPrettyData(data interface{}) (*api.PrettyData, error) { | ||
| func ToPrettyData(data interface{}, opts ...api.TypeOptions) (*api.PrettyData, error) { |
There was a problem hiding this comment.
Fix undefined type: api.TypeOptions does not exist.
The code references api.TypeOptions in the mergeTypeOptions function (line 558) and ToPrettyData signature (line 568), but this type is undefined, causing compilation failures across all pipelines.
The api.TypeOptions type needs to be defined in the api package. Based on usage context (lines 561-562 checking SkipTable and SkipTree), it should likely be:
// In api package (e.g., api/meta.go or api/options.go):
type TypeOptions struct {
SkipTable bool
SkipTree bool
}Alternatively, if the intent was to use FormatOptions from the formatters package, the signature should be:
func ToPrettyData(data interface{}, opts ...FormatOptions) (*api.PrettyData, error) {
// And update mergeTypeOptions to use FormatOptions
}Do you want me to generate the complete fix based on the intended design?
🧰 Tools
🪛 GitHub Actions: Lint
[error] 558-558: golangci-lint: undefined: api.TypeOptions
🪛 GitHub Actions: PR #54
[error] 558-558: Build failed: undefined: api.TypeOptions (formatters/parser.go:558:35) during 'go build -o clicky ./cmd/clicky/'
🪛 GitHub Actions: Test
[error] 558-558: compile error: undefined: api.TypeOptions in formatters/parser.go:558
🪛 GitHub Check: golangci-lint
[failure] 568-568:
undefined: api.TypeOptions
[failure] 558-558:
undefined: api.TypeOptions
🪛 GitHub Check: Lint
[failure] 568-568:
undefined: api.TypeOptions
[failure] 558-558:
undefined: api.TypeOptions
🪛 GitHub Check: Test (ubuntu-latest, Go 1.25)
[failure] 568-568:
undefined: api.TypeOptions
[failure] 558-558:
undefined: api.TypeOptions
🤖 Prompt for AI Agents
In formatters/parser.go around lines 558 to 568, the code references an
undefined type api.TypeOptions in mergeTypeOptions and ToPrettyData; define the
type in the api package or switch usages to the correct existing type. Either
add a new type in the api package (e.g., TypeOptions with SkipTable and SkipTree
bool fields) and update imports, or change mergeTypeOptions and ToPrettyData to
accept the existing formatters.FormatOptions (or api.FormatOptions) type used
elsewhere; ensure all call sites compile by updating signatures and any
construction of option values accordingly.
| } | ||
|
|
||
| opts := FormatOptions{Table: true} | ||
| opts := FormatOptions{Table: lo.ToPtr(true)} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n 'samber/lo' formatters/tests/parser_test.goRepository: flanksource/clicky
Length of output: 44
🏁 Script executed:
head -n 300 formatters/tests/parser_test.go | tail -n +1 | head -n 30Repository: flanksource/clicky
Length of output: 617
🏁 Script executed:
sed -n '3,8p; 285,295p' formatters/tests/parser_test.goRepository: flanksource/clicky
Length of output: 426
🏁 Script executed:
#!/bin/bash
# Check if lo is imported in this file
rg -n 'samber/lo' formatters/tests/parser_test.goRepository: flanksource/clicky
Length of output: 44
Add missing import for lo package.
The lo.ToPtr call at line 290 requires the import github.com/samber/lo, which is not present in the imports section (lines 3-8).
🤖 Prompt for AI Agents
In formatters/tests/parser_test.go around line 290, the test uses lo.ToPtr but
the file is missing the import for github.com/samber/lo; add that package to the
import block (e.g. import "github.com/samber/lo"), save and run goimports/gofmt
to ensure imports are ordered and formatted correctly.
| opts := FormatOptions{Table: lo.ToPtr(true)} | ||
| prettyData, err := ToPrettyDataWithOptions(input, opts) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find the FormatOptions struct definition
ast-grep --pattern 'type FormatOptions struct {
$$$
}'Repository: flanksource/clicky
Length of output: 180
🏁 Script executed:
#!/bin/bash
# Also search with rg to find FormatOptions definition across the codebase
rg 'type FormatOptions' -A 10Repository: flanksource/clicky
Length of output: 1315
🏁 Script executed:
#!/bin/bash
# Read the formatters/options.go file to see the full struct
cat -n formatters/options.goRepository: flanksource/clicky
Length of output: 9497
The struct definition at formatters/options.go line 38 should declare Table *bool, not Table bool.
The FormatOptions struct declares Table bool, but the codebase consistently treats it as a pointer (*bool): methods check != nil and dereference with *, and flag binding assigns lo.ToPtr() results to it. The test code at lines 290-291 correctly uses lo.ToPtr(true), but only works because the struct field definition itself is incorrect. Update line 38 to declare Table *bool to match the actual usage pattern.
🤖 Prompt for AI Agents
In formatters/options.go around line 38, the FormatOptions struct currently
declares Table as a non-pointer (Table bool) but the codebase expects a pointer;
change the field declaration to Table *bool so callers that check for nil and
use lo.ToPtr() work correctly, update any nearby comments if needed, run `go
test ./...` to verify and fix any compile errors that surface from the type
change.
| ) | ||
|
|
||
| func TestSortRows(t *testing.T) { | ||
| func XTestSortRows(t *testing.T) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for TODO/FIXME comments or issues referencing this disabled test
rg -n -i 'XTestSortRows|sortrows.*skip|sortrows.*disable' --type=goRepository: flanksource/clicky
Length of output: 132
🏁 Script executed:
# Also check the test file itself for any comments around the disabled test
cat -n formatters/tests/sorting_test.go | head -20Repository: flanksource/clicky
Length of output: 1402
🏁 Script executed:
# Search for any issue references or TODO comments in the file
rg -n 'TODO|FIXME|XXX' formatters/tests/sorting_test.goRepository: flanksource/clicky
Length of output: 44
Test disabled via XTest prefix—add tracking or documentation for this disabled test.
TestSortRows has been renamed to XTestSortRows, which disables it from go test discovery. There is no TODO, FIXME, or issue reference explaining why this test is disabled. Either add an inline comment explaining the reason (e.g., "disabled pending refactor of SortRows") or create a tracking issue to ensure it is not forgotten.
🤖 Prompt for AI Agents
In formatters/tests/sorting_test.go around line 11, the test has been renamed
from TestSortRows to XTestSortRows which disables it from go test discovery; add
a brief inline comment on the same line or immediately above explaining why the
test is disabled (e.g., "disabled pending refactor of SortRows" or reference to
an issue/PR number), or create a tracking issue and reference its ID in the
comment; ensure the comment includes owner/context and a TODO/FIXME tag so it’s
discoverable by linters and future maintainers.
| } | ||
|
|
||
| func TestCustomRenderFunction(t *testing.T) { | ||
| func XTestCustomRenderFunction(t *testing.T) { |
There was a problem hiding this comment.
Second test disabled via XTest prefix—pattern raises concern.
Combined with XTestSortRows in sorting_test.go, this PR disables two tests without explanation. If these tests are failing due to the refactoring, consider:
- Adding
t.Skip("reason")inside the test with an explanation (like line 156 does) - Creating a tracking issue for re-enabling them
- Documenting in the PR description why these are temporarily disabled
🤖 Prompt for AI Agents
In formatters/tests/tree_test.go around line 128 the test was disabled by
renaming it to XTestCustomRenderFunction; replace the XTest prefix by restoring
the original Test name or, if you intend to keep it skipped temporarily, revert
the name to TestCustomRenderFunction and add an explicit t.Skip("brief reason
and tracking issue URL") as the first statement, create a tracking issue to
re-enable the test, and update the PR description with the reason and issue link
so disabled tests are documented and traceable.
Description
Brief description of the changes in this PR.
Type of Change
Testing
Checklist
Breaking Changes
If this is a breaking change, please describe the impact and migration path for existing users:
Additional Notes
Add any additional notes, screenshots, or context about the changes here.
Summary by CodeRabbit
Release Notes
New Features
Improvements
Tests
✏️ Tip: You can customize this high-level summary in your review settings.