test: add unit tests for executor helpers and terminal utilities#172
Conversation
Add unit tests for helper functions in executor.go and terminal.go: executor.go - JSON argument processing and barrier functions: - getMessage: extracts message field from JSON args, handles missing fields, invalid JSON, empty objects, and unicode content - argsToMarkdown: converts JSON args to markdown bullet list, skips message key, handles invalid JSON and empty objects - IsBarrierFunction: checks barrier map membership - GetBarrierToolNames: returns all registered barrier tool names terminal.go - Terminal name and output formatting: - PrimaryTerminalName: generates terminal name from flow ID - FormatTerminalInput: ANSI yellow-colored input with cwd prefix - FormatTerminalSystemOutput: ANSI blue-colored system output Signed-off-by: mason5052 <ehehwnwjs5052@gmail.com>
There was a problem hiding this comment.
Pull request overview
Adds unit tests for helper logic in backend/pkg/tools covering executor JSON argument parsing/formatting and terminal output formatting utilities.
Changes:
- Add
executor_test.goto testcustomExecutorhelpers (getMessage,argsToMarkdown) plus barrier helper methods. - Add
terminal_test.goto test terminal naming and ANSI formatting helpers (PrimaryTerminalName,FormatTerminalInput,FormatTerminalSystemOutput).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| backend/pkg/tools/executor_test.go | New unit tests for executor helper methods and barrier-related helpers. |
| backend/pkg/tools/terminal_test.go | New unit tests for terminal name generation and ANSI-colored formatting helpers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if !strings.Contains(result, tt.cwd) { | ||
| t.Errorf("result should contain cwd %q", tt.cwd) |
There was a problem hiding this comment.
In the "empty cwd" case this assertion is vacuous: strings.Contains(result, tt.cwd) will always be true when tt.cwd == "", so the test doesn’t validate the formatting for an empty cwd. Consider branching on tt.cwd != "" (or for the empty case assert an exact prefix like " $ " / the full expected string).
| if !strings.Contains(result, tt.cwd) { | |
| t.Errorf("result should contain cwd %q", tt.cwd) | |
| if tt.cwd != "" { | |
| if !strings.Contains(result, tt.cwd) { | |
| t.Errorf("result should contain cwd %q", tt.cwd) | |
| } |
| if !strings.Contains(result, tt.text) { | ||
| t.Errorf("result should contain text %q", tt.text) |
There was a problem hiding this comment.
The "empty output" case isn’t meaningfully asserted: strings.Contains(result, tt.text) will always pass when tt.text == "". To actually cover the behavior, assert the exact expected output for empty text (e.g., just the blue prefix + reset + CRLF), or at least verify that the color codes wrap an empty payload correctly.
| if !strings.Contains(result, tt.text) { | |
| t.Errorf("result should contain text %q", tt.text) | |
| if tt.text != "" { | |
| if !strings.Contains(result, tt.text) { | |
| t.Errorf("result should contain text %q", tt.text) | |
| } | |
| } else { | |
| // For empty text, ensure the blue and reset codes directly wrap an empty payload. | |
| const blue = "\033[34m" | |
| const reset = "\033[0m" | |
| blueIdx := strings.Index(result, blue) | |
| resetIdx := strings.Index(result, reset) | |
| if blueIdx == -1 || resetIdx == -1 { | |
| t.Errorf("result for empty text should contain both blue %q and reset %q codes, got %q", blue, reset, result) | |
| } else { | |
| payload := result[blueIdx+len(blue) : resetIdx] | |
| if payload != "" { | |
| t.Errorf("expected no payload between blue and reset codes for empty text, got %q", payload) | |
| } | |
| } |
|
thank you! |
Description
Add unit tests for helper functions in two core modules:
backend/pkg/tools/executor.go- JSON argument processing and barrier function managementbackend/pkg/tools/terminal.go- Terminal name generation and ANSI-formatted outputType of Change
Areas Affected
backend/pkg/tools/executor_test.go(new file)backend/pkg/tools/terminal_test.go(new file)Testing
All tests are self-contained, table-driven, and use
t.Parallel().executor_test.go (4 test functions):
TestGetMessage- extracts "message" field from JSON args; covers valid message, empty message, missing field, invalid JSON, empty object, unicode contentTestArgsToMarkdown- converts JSON args to markdown bullet list; verifies "message" key is skipped, handles single/multiple fields, invalid JSON errors, empty objectsTestIsBarrierFunction- checks barrier map membership for known barriers (done, ask_user) and non-barriersTestGetBarrierToolNames- returns all registered barrier tool names, verifies count and contentsterminal_test.go (3 test functions):
TestPrimaryTerminalName- generates "pentagi-terminal-{flowID}" format for various flow IDsTestFormatTerminalInput- ANSI yellow-colored terminal input with cwd prefix, verifies color codes and line endingsTestFormatTerminalSystemOutput- ANSI blue-colored system output, verifies color codes and content wrappingSecurity Considerations
No security impact - test-only changes.
Checklist