From b8e2fd669d7bf2abc9f3121c4ecf70d2ef985adc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tom=C3=A1s=20Barreiro?= <52393857+BarreiroT@users.noreply.github.com> Date: Tue, 29 Apr 2025 01:30:40 +0200 Subject: [PATCH] fix unit test set-up and CI workflow (#3154) * Fix unit test set-up * run tests on CI * update failing tests * Update node Update the coverage job node version --- .github/workflows/test.yml | 9 +++-- .mocharc.json | 2 +- .../__tests__/ContextManager.test.ts | 34 ++++++++++--------- src/test/requires.ts | 17 ++++++++++ src/test/vscode-mock.ts | 1 + 5 files changed, 41 insertions(+), 22 deletions(-) create mode 100644 src/test/requires.ts create mode 100644 src/test/vscode-mock.ts diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index debe58c0d..210812fdf 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -23,7 +23,7 @@ jobs: - name: Setup Node.js environment uses: actions/setup-node@v4 with: - node-version: 20.15.1 + node-version: 22 # Setup Python for coverage script - name: Setup Python @@ -73,9 +73,8 @@ jobs: - name: Build Extension run: npm run compile - # Disabling due to compatability with test framework and ESM modules - # - name: Unit Tests - # run: npm run test:unit + - name: Unit Tests + run: npm run test:unit # Run extension tests with coverage - name: Extension Tests with Coverage @@ -132,7 +131,7 @@ jobs: - name: Setup Node.js environment uses: actions/setup-node@v4 with: - node-version: 20.15.1 + node-version: 22 # Cache root dependencies - only reuse if package-lock.json exactly matches - name: Cache root dependencies diff --git a/.mocharc.json b/.mocharc.json index 1cc6d751a..8b62d28f2 100644 --- a/.mocharc.json +++ b/.mocharc.json @@ -1,6 +1,6 @@ { "extension": ["ts"], "spec": "src/**/__tests__/*.ts", - "require": ["ts-node/register", "source-map-support/register"], + "require": ["ts-node/register", "source-map-support/register", "./src/test/requires.ts"], "recursive": true } diff --git a/src/core/context/context-management/__tests__/ContextManager.test.ts b/src/core/context/context-management/__tests__/ContextManager.test.ts index e180ddf76..088c8c204 100644 --- a/src/core/context/context-management/__tests__/ContextManager.test.ts +++ b/src/core/context/context-management/__tests__/ContextManager.test.ts @@ -34,24 +34,24 @@ describe("ContextManager", () => { const messages = createMessages(11) const result = contextManager.getNextTruncationRange(messages, undefined, "half") - expect(result).to.deep.equal([1, 4]) + expect(result).to.deep.equal([2, 5]) }) it("first truncation with quarter keep", () => { const messages = createMessages(11) const result = contextManager.getNextTruncationRange(messages, undefined, "quarter") - expect(result).to.deep.equal([1, 6]) + expect(result).to.deep.equal([2, 7]) }) it("sequential truncation with half keep", () => { const messages = createMessages(21) const firstRange = contextManager.getNextTruncationRange(messages, undefined, "half") - expect(firstRange).to.deep.equal([1, 10]) + expect(firstRange).to.deep.equal([2, 9]) // Pass the previous range for sequential truncation const secondRange = contextManager.getNextTruncationRange(messages, firstRange, "half") - expect(secondRange).to.deep.equal([1, 14]) + expect(secondRange).to.deep.equal([2, 13]) }) it("sequential truncation with quarter keep", () => { @@ -60,7 +60,7 @@ describe("ContextManager", () => { const secondRange = contextManager.getNextTruncationRange(messages, firstRange, "quarter") - expect(secondRange[0]).to.equal(1) + expect(secondRange[0]).to.equal(2) expect(secondRange[1]).to.be.greaterThan(firstRange[1]) }) @@ -68,20 +68,20 @@ describe("ContextManager", () => { const messages = createMessages(14) const result = contextManager.getNextTruncationRange(messages, undefined, "half") - // Check if the message at the end of range is a user message + // Check if the message at the end of range is an assistant message const lastRemovedMessage = messages[result[1]] - expect(lastRemovedMessage.role).to.equal("user") + expect(lastRemovedMessage.role).to.equal("assistant") - // Check if the next message after the range is an assistant message + // Check if the next message after the range is a user message const nextMessage = messages[result[1] + 1] - expect(nextMessage.role).to.equal("assistant") + expect(nextMessage.role).to.equal("user") }) it("handles small message arrays", () => { const messages = createMessages(3) const result = contextManager.getNextTruncationRange(messages, undefined, "half") - expect(result).to.deep.equal([1, 0]) + expect(result).to.deep.equal([2, 1]) }) it("preserves the message structure when truncating", () => { @@ -120,9 +120,10 @@ describe("ContextManager", () => { const range: [number, number] = [1, 3] const result = contextManager.getTruncatedMessages(messages, range) - expect(result).to.have.lengthOf(2) + expect(result).to.have.lengthOf(3) expect(result[0]).to.deep.equal(messages[0]) - expect(result[1]).to.deep.equal(messages[4]) + expect(result[1]).to.deep.equal(messages[1]) + expect(result[2]).to.deep.equal(messages[4]) }) it("works with a range that starts at the first message after task", () => { @@ -131,20 +132,21 @@ describe("ContextManager", () => { const range: [number, number] = [1, 2] const result = contextManager.getTruncatedMessages(messages, range) - expect(result).to.have.lengthOf(2) + expect(result).to.have.lengthOf(3) expect(result[0]).to.deep.equal(messages[0]) - expect(result[1]).to.deep.equal(messages[3]) + expect(result[1]).to.deep.equal(messages[1]) + expect(result[2]).to.deep.equal(messages[3]) }) it("correctly handles removing a range while preserving alternation pattern", () => { const messages = createMessages(5) - const range: [number, number] = [1, 2] + const range: [number, number] = [2, 3] const result = contextManager.getTruncatedMessages(messages, range) expect(result).to.have.lengthOf(3) expect(result[0]).to.deep.equal(messages[0]) - expect(result[1]).to.deep.equal(messages[3]) + expect(result[1]).to.deep.equal(messages[1]) expect(result[2]).to.deep.equal(messages[4]) expect(result[0].role).to.equal("user") diff --git a/src/test/requires.ts b/src/test/requires.ts new file mode 100644 index 000000000..5144016b5 --- /dev/null +++ b/src/test/requires.ts @@ -0,0 +1,17 @@ +const Module = require("module") +const originalRequire = Module.prototype.require + +/** + * VSCode is not available during unit tests + * @see {@link file://./vscode-mock.ts} + */ +Module.prototype.require = function (path: string) { + if (path === "vscode") { + return require("./vscode-mock") + } + + return originalRequire.call(this, path) +} + +// Required to have access to String.prototype.toPosix +import "../utils/path" diff --git a/src/test/vscode-mock.ts b/src/test/vscode-mock.ts new file mode 100644 index 000000000..336ce12bb --- /dev/null +++ b/src/test/vscode-mock.ts @@ -0,0 +1 @@ +export {}