From baf03ba7ea0d88db3dc9baee05c1aeb238e5b72c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 30 Sep 2025 21:50:07 +0000 Subject: [PATCH 1/3] Initial plan From 64fa669e86abaf4ef109f2d109ea7edf2352bc87 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 30 Sep 2025 21:55:42 +0000 Subject: [PATCH 2/3] Add comprehensive unit tests for execUtils.ts Co-authored-by: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> --- .../features/execution/execUtils.unit.test.ts | 158 ++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 src/test/features/execution/execUtils.unit.test.ts diff --git a/src/test/features/execution/execUtils.unit.test.ts b/src/test/features/execution/execUtils.unit.test.ts new file mode 100644 index 00000000..386407a2 --- /dev/null +++ b/src/test/features/execution/execUtils.unit.test.ts @@ -0,0 +1,158 @@ +import * as assert from 'assert'; +import { quoteStringIfNecessary, quoteArgs } from '../../../features/execution/execUtils'; + +suite('Execution Utils Tests', () => { + suite('quoteStringIfNecessary', () => { + test('should not quote string without spaces', () => { + const input = 'simplestring'; + const result = quoteStringIfNecessary(input); + assert.strictEqual(result, 'simplestring'); + }); + + test('should not quote string without spaces containing special characters', () => { + const input = 'path/to/file.txt'; + const result = quoteStringIfNecessary(input); + assert.strictEqual(result, 'path/to/file.txt'); + }); + + test('should quote string with spaces', () => { + const input = 'string with spaces'; + const result = quoteStringIfNecessary(input); + assert.strictEqual(result, '"string with spaces"'); + }); + + test('should quote path with spaces', () => { + const input = 'C:\\Program Files\\Python'; + const result = quoteStringIfNecessary(input); + assert.strictEqual(result, '"C:\\Program Files\\Python"'); + }); + + test('should not double-quote already quoted string', () => { + const input = '"already quoted"'; + const result = quoteStringIfNecessary(input); + assert.strictEqual(result, '"already quoted"'); + }); + + test('should not double-quote already quoted string with spaces', () => { + const input = '"string with spaces"'; + const result = quoteStringIfNecessary(input); + assert.strictEqual(result, '"string with spaces"'); + }); + + test('should quote string with space that is partially quoted', () => { + const input = '"partially quoted'; + const result = quoteStringIfNecessary(input); + assert.strictEqual(result, '""partially quoted"'); + }); + + test('should quote string with space that ends with quote', () => { + const input = 'partially quoted"'; + const result = quoteStringIfNecessary(input); + assert.strictEqual(result, '"partially quoted""'); + }); + + test('should handle empty string', () => { + const input = ''; + const result = quoteStringIfNecessary(input); + assert.strictEqual(result, ''); + }); + + test('should handle string with only spaces', () => { + const input = ' '; + const result = quoteStringIfNecessary(input); + assert.strictEqual(result, '" "'); + }); + + test('should handle string with leading space', () => { + const input = ' leading'; + const result = quoteStringIfNecessary(input); + assert.strictEqual(result, '" leading"'); + }); + + test('should handle string with trailing space', () => { + const input = 'trailing '; + const result = quoteStringIfNecessary(input); + assert.strictEqual(result, '"trailing "'); + }); + + test('should handle string with multiple spaces', () => { + const input = 'multiple spaces here'; + const result = quoteStringIfNecessary(input); + assert.strictEqual(result, '"multiple spaces here"'); + }); + + test('should not quote single character without space', () => { + const input = 'a'; + const result = quoteStringIfNecessary(input); + assert.strictEqual(result, 'a'); + }); + + test('should handle dash and hyphen characters without spaces', () => { + const input = '--flag-name'; + const result = quoteStringIfNecessary(input); + assert.strictEqual(result, '--flag-name'); + }); + }); + + suite('quoteArgs', () => { + test('should return empty array for empty input', () => { + const input: string[] = []; + const result = quoteArgs(input); + assert.deepStrictEqual(result, []); + }); + + test('should not quote args without spaces', () => { + const input = ['arg1', 'arg2', 'arg3']; + const result = quoteArgs(input); + assert.deepStrictEqual(result, ['arg1', 'arg2', 'arg3']); + }); + + test('should quote args with spaces', () => { + const input = ['arg with spaces', 'another arg']; + const result = quoteArgs(input); + assert.deepStrictEqual(result, ['"arg with spaces"', '"another arg"']); + }); + + test('should handle mixed args with and without spaces', () => { + const input = ['simplearg', 'arg with spaces', 'anotherarg']; + const result = quoteArgs(input); + assert.deepStrictEqual(result, ['simplearg', '"arg with spaces"', 'anotherarg']); + }); + + test('should not double-quote already quoted args', () => { + const input = ['"already quoted"', 'normal']; + const result = quoteArgs(input); + assert.deepStrictEqual(result, ['"already quoted"', 'normal']); + }); + + test('should handle array with single element', () => { + const input = ['single element with space']; + const result = quoteArgs(input); + assert.deepStrictEqual(result, ['"single element with space"']); + }); + + test('should handle paths correctly', () => { + const input = ['C:\\Program Files\\Python', '/usr/bin/python', 'simple']; + const result = quoteArgs(input); + assert.deepStrictEqual(result, ['"C:\\Program Files\\Python"', '/usr/bin/python', 'simple']); + }); + + test('should handle command line flags and values', () => { + const input = ['--flag', 'value with spaces', '-f', 'normalvalue']; + const result = quoteArgs(input); + assert.deepStrictEqual(result, ['--flag', '"value with spaces"', '-f', 'normalvalue']); + }); + + test('should handle empty strings in array', () => { + const input = ['', 'arg1', '']; + const result = quoteArgs(input); + assert.deepStrictEqual(result, ['', 'arg1', '']); + }); + + test('should preserve order of arguments', () => { + const input = ['first', 'second with space', 'third', 'fourth with space']; + const result = quoteArgs(input); + assert.deepStrictEqual(result, ['first', '"second with space"', 'third', '"fourth with space"']); + }); + }); +}); From 1a1b1c5a5fa946d3ce776755c1b5927ba6d14301 Mon Sep 17 00:00:00 2001 From: eleanorjboyd <26030610+eleanorjboyd@users.noreply.github.com> Date: Wed, 1 Oct 2025 09:27:07 -0700 Subject: [PATCH 3/3] updates --- .../testing-workflow.instructions.md | 55 ++++++++++++++++++- .../features/execution/execUtils.unit.test.ts | 2 +- 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/.github/instructions/testing-workflow.instructions.md b/.github/instructions/testing-workflow.instructions.md index 7632e194..c958b403 100644 --- a/.github/instructions/testing-workflow.instructions.md +++ b/.github/instructions/testing-workflow.instructions.md @@ -537,7 +537,59 @@ envConfig.inspect - ❌ Tests that don't clean up mocks properly - ❌ Overly complex test setup that's hard to understand -## 🧠 Agent Learning Patterns +## � Common Pitfalls & Fast Diagnosis + +These recurring issues have caused wasted cycles. Follow the quick diagnosis flow to avoid them: + +### 1. 0 Tests Detected When Filtering (e.g. with --grep or targeted run) + +Likely Causes (in order): + +- The TypeScript test file has not been compiled to `out/test/...` yet. +- The file was moved outside the compiler `rootDir` (tsconfig sets `rootDir: src`), so `tsc` will not emit it into `out/`. +- The grep / test name filter does not match the suite or test names. + +Diagnosis Checklist: + +1. Confirm compile step ran: if not in watch mode, run the compile test script (see below) **before** executing tests. +2. Check for the emitted JS: derive the expected path (replace `src/` with `out/` and ensure the directory pattern matches `out/test/**/*.unit.test.js`). If the JS file is missing, it's a compilation / placement issue. +3. Open the compiled JS and verify the `suite("")` string literally appears; if missing, the source file wasn’t included or was misnamed. +4. Re-run the test using the programmatic test tool (preferred) referencing the file path instead of a grep first; if that succeeds, refine your grep pattern. + +Remediation: + +- Ensure unit test files live under `src/test/...` (since `rootDir` is `src`) so they are emitted under `out/test/...` and picked up by Mocha’s spec glob. +- Run `watch-tests` once (preferred) or a one-off `compile-tests` before first execution. +- Avoid relocating tests to a top-level `test/` folder unless tsconfig and spec globs are updated accordingly. + +### 2. Using Terminal First Instead of Test Tool + +Always default to the test execution tool (runTests) for: + +- Precise targeting (single file / specific tests via testNames) +- Richer integration & structured output + +Use terminal (`npm run unittest`) only as a fallback. + +### 3. Infinite Watch Confusion + +If you started `npm run watch-tests`, do **not** also run `compile-tests` repeatedly—watch already handles incremental builds. Just re-run the test tool after edits. + +### Quick Flowchart + +0 tests? → Was TS compiled? (Check for `out/test/...js`) → If no: compile. → If yes: Is file under `src/test`? → If no: relocate. → If yes: Does suite name match grep / filter? → Adjust filter → Use runTests with direct file path. + +### Example Programmatic Run + +``` +await runTests({ + files: ['/absolute/path/to/src/test/features/execution/execUtils.unit.test.ts'] +}); +``` + +If that returns 0, re-check compilation path mapping. + +## Agent Learning Patterns ### Key Implementation Insights @@ -550,3 +602,4 @@ envConfig.inspect - When fixing mock environment creation, use `null` to truly omit properties rather than `undefined` (1) - Always recompile TypeScript after making import/export changes before running tests, as stubs won't work if they're applied to old compiled JavaScript that doesn't have the updated imports (2) - Create proxy abstraction functions for Node.js APIs like `cp.spawn` to enable clean testing - use function overloads to preserve Node.js's intelligent typing while making the functions mockable (1) +- When a targeted test run yields 0 tests, first verify the compiled JS exists under `out/test` (rootDir is `src`); absence almost always means the test file sits outside `src` or compilation hasn't run yet (1) diff --git a/src/test/features/execution/execUtils.unit.test.ts b/src/test/features/execution/execUtils.unit.test.ts index 386407a2..82dd8084 100644 --- a/src/test/features/execution/execUtils.unit.test.ts +++ b/src/test/features/execution/execUtils.unit.test.ts @@ -1,5 +1,5 @@ import * as assert from 'assert'; -import { quoteStringIfNecessary, quoteArgs } from '../../../features/execution/execUtils'; +import { quoteArgs, quoteStringIfNecessary } from '../../../features/execution/execUtils'; suite('Execution Utils Tests', () => { suite('quoteStringIfNecessary', () => {