[PR #181] [CLOSED] test: add unit tests for terminal ExecCommand detach behavior #218

New Issue

2026-06-06T22:09:45-04:00

yindo commented

2026-06-06 22:09:45 -04:00

📋 Pull Request Information

Original PR: https://github.com/vxcontrol/pentagi/pull/181
Author: @mason5052
Created: 3/5/2026
Status: ❌ Closed

Base: master ← Head: test/terminal-detached-context

📝 Commits (2)

9524cbb test: add unit tests for terminal ExecCommand detach behavior
977d0e0 fix: tighten detach timing assertion from 2s to 1s

📊 Changes

1 file changed (+174 additions, -0 deletions)

View changed files

📝 backend/pkg/tools/terminal_test.go (+174 -0)

📄 Description

Dependency: Merge #179 first -- this PR tests the detached context isolation fix introduced in #179.

Description of the Change

Problem

The terminal ExecCommand function (408 lines) had zero test coverage. The detached execution path -- recently fixed for context isolation (#176, #179) -- particularly needed validation.

Solution

Add mock infrastructure and 4 test cases for terminal command execution:

Mock infrastructure:

mockDockerClient: Implements full docker.DockerClient interface with configurable responses, timing delays, and error injection. Uses net.Pipe() for realistic streaming output simulation.
mockTermLogProvider: Implements TermLogProvider interface (no-op for tests).
Both include compile-time interface checks (var _ Interface = (*mock)(nil)).

Test cases:

TestExecCommandDetachReturnsQuickly: Verifies detach=true returns "Command started in background" within ~500ms even when the command takes 2s+
TestExecCommandDetachQuickCompletion: Verifies detach=true returns actual output when command finishes before the quick check timeout
TestExecCommandNonDetachWaitsForCompletion: Verifies detach=false blocks until command completes and returns full output
TestExecCommandContainerNotRunning: Verifies proper error handling when container is not running

The mock infrastructure is reusable for future terminal.go test expansion (ReadFile, WriteFile, etc.).

Related to #176
Adds test coverage for #179

Type of Change

Tests (adding or updating tests)

Areas Affected

Core Services (Backend API)

Testing and Verification

Test Configuration

PentAGI Version: master
Go Version: 1.24

Test Steps

go test ./pkg/tools/ -run "TestExecCommand" -v -- all 4 tests pass
go vet ./pkg/tools/ -- no warnings
Pre-existing TestSploitusParseExploitsResponse/nginx failure is unrelated (Windows Defender file access issue on test data)

Security Considerations

No security impact. Test-only change. Mock infrastructure does not interact with real Docker.

Checklist

My code follows the project's coding standards
All new and existing tests pass
I have run go fmt and go vet
Security implications considered
Changes are backward compatible

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/vxcontrol/pentagi/pull/181 **Author:** [@mason5052](https://github.com/mason5052) **Created:** 3/5/2026 **Status:** ❌ Closed **Base:** `master` ← **Head:** `test/terminal-detached-context` --- ### 📝 Commits (2) - [`9524cbb`](https://github.com/vxcontrol/pentagi/commit/9524cbbd374bcc44bab57eb77f325e5eefa544a2) test: add unit tests for terminal ExecCommand detach behavior - [`977d0e0`](https://github.com/vxcontrol/pentagi/commit/977d0e0f3075f555f0a7eea76f0bf9e39164052a) fix: tighten detach timing assertion from 2s to 1s ### 📊 Changes **1 file changed** (+174 additions, -0 deletions) <details> <summary>View changed files</summary> 📝 `backend/pkg/tools/terminal_test.go` (+174 -0) </details> ### 📄 Description > **Dependency:** Merge #179 first -- this PR tests the detached context isolation fix introduced in #179. ### Description of the Change #### Problem The terminal `ExecCommand` function (408 lines) had zero test coverage. The detached execution path -- recently fixed for context isolation (#176, #179) -- particularly needed validation. #### Solution Add mock infrastructure and 4 test cases for terminal command execution: **Mock infrastructure:** - `mockDockerClient`: Implements full `docker.DockerClient` interface with configurable responses, timing delays, and error injection. Uses `net.Pipe()` for realistic streaming output simulation. - `mockTermLogProvider`: Implements `TermLogProvider` interface (no-op for tests). - Both include compile-time interface checks (`var _ Interface = (*mock)(nil)`). **Test cases:** - `TestExecCommandDetachReturnsQuickly`: Verifies `detach=true` returns "Command started in background" within ~500ms even when the command takes 2s+ - `TestExecCommandDetachQuickCompletion`: Verifies `detach=true` returns actual output when command finishes before the quick check timeout - `TestExecCommandNonDetachWaitsForCompletion`: Verifies `detach=false` blocks until command completes and returns full output - `TestExecCommandContainerNotRunning`: Verifies proper error handling when container is not running The mock infrastructure is reusable for future terminal.go test expansion (ReadFile, WriteFile, etc.). Related to #176 Adds test coverage for #179 ### Type of Change - [x] Tests (adding or updating tests) ### Areas Affected - [x] Core Services (Backend API) ### Testing and Verification #### Test Configuration - PentAGI Version: master - Go Version: 1.24 #### Test Steps 1. `go test ./pkg/tools/ -run "TestExecCommand" -v` -- all 4 tests pass 2. `go vet ./pkg/tools/` -- no warnings 3. Pre-existing `TestSploitusParseExploitsResponse/nginx` failure is unrelated (Windows Defender file access issue on test data) ### Security Considerations No security impact. Test-only change. Mock infrastructure does not interact with real Docker. ### Checklist - [x] My code follows the project's coding standards - [x] All new and existing tests pass - [x] I have run `go fmt` and `go vet` - [x] Security implications considered - [x] Changes are backward compatible --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

yindo added the pull-request label 2026-06-06 22:09:45 -04:00

yindo closed this issue

2026-06-06 22:09:45 -04:00

yindo referenced this issue

2026-06-06 22:09:52 -04:00

[PR #218] [MERGED] Quick fix for assistant mode #244

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: vxcontrol/pentagi#218