From 13228ed46fff6d15fa965d3c835d7d91be0e216d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tom=C3=A1s=20Barreiro?=
 <52393857+BarreiroT@users.noreply.github.com>
Date: Thu, 1 May 2025 02:51:03 +0200
Subject: [PATCH] Fail the `test` workflow on test failures (#3197)

* Run pretest in CI to build all tests

* Alias paths when running tests

* Bundle ES modules with esbuild

* alias packages

* Preserve the test scripts exit code and display the output

* Remove outdated test
---
 .github/workflows/test.yml                    | 14 ++++-
 esbuild.js                                    |  1 +
 package.json                                  |  2 +-
 scripts/build-tests.js                        | 61 +++++++++++++++++++
 .../ModelContextTracker.test.ts               | 20 ------
 src/core/storage/disk.ts                      |  2 +-
 src/packages/execa.ts                         |  1 +
 tsconfig.json                                 |  3 +-
 8 files changed, 80 insertions(+), 24 deletions(-)
 create mode 100644 scripts/build-tests.js
 create mode 100644 src/packages/execa.ts

diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index e23a8f4ae..22aeb0240 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -81,7 +81,7 @@ jobs:
               id: extension_coverage
               continue-on-error: true
               run: |
-                  xvfb-run -a npm run test:coverage > extension_coverage.txt 2>&1 || true
+                  xvfb-run -a npm run test:coverage > extension_coverage.txt 2>&1
                   PYTHONPATH=.github/scripts python -m coverage_check extract-coverage extension_coverage.txt --type=extension --github-output --verbose
 
             # Run webview tests with coverage
@@ -106,6 +106,18 @@ jobs:
                       webview-ui/webview_coverage.txt
                   retention-period: workflow # Artifacts are automatically deleted when the workflow completes
 
+            # Set the check as failed if any of the tests failed
+            - name: Check for test failures
+              run: |
+                  # Check if any of the test steps failed
+                  # https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/accessing-contextual-information-about-workflow-runs#steps-context
+                  if [ "${{ steps.extension_coverage.outcome }}" != "success" ] || [ "${{ steps.webview_coverage.outcome }}" != "success" ]; then
+                      echo "Tests failed."
+                      cat extension_coverage.txt
+                      cat webview-ui/webview_coverage.txt
+                      exit 1
+                  fi
+
     coverage:
         needs: test
         runs-on: ubuntu-latest
diff --git a/esbuild.js b/esbuild.js
index 8e2d44cc9..e67886152 100644
--- a/esbuild.js
+++ b/esbuild.js
@@ -19,6 +19,7 @@ const aliasResolverPlugin = {
 			"@services": path.resolve(__dirname, "src/services"),
 			"@shared": path.resolve(__dirname, "src/shared"),
 			"@utils": path.resolve(__dirname, "src/utils"),
+			"@packages": path.resolve(__dirname, "src/packages"),
 		}
 
 		// For each alias entry, create a resolver
diff --git a/package.json b/package.json
index 7f72792d1..d79eb45af 100644
--- a/package.json
+++ b/package.json
@@ -292,7 +292,7 @@
 		"watch:tsc": "tsc --noEmit --watch --project tsconfig.json",
 		"package": "npm run build:webview && npm run check-types && npm run lint && node esbuild.js --production",
 		"protos": "node proto/build-proto.js && prettier src/shared/proto --write && prettier src/core/controller --write",
-		"compile-tests": "tsc -p ./tsconfig.test.json --outDir out",
+		"compile-tests": "node ./scripts/build-tests.js",
 		"watch-tests": "tsc -p . -w --outDir out",
 		"pretest": "npm run compile-tests && npm run compile && npm run lint",
 		"check-types": "tsc --noEmit",
diff --git a/scripts/build-tests.js b/scripts/build-tests.js
new file mode 100644
index 000000000..247d247b1
--- /dev/null
+++ b/scripts/build-tests.js
@@ -0,0 +1,61 @@
+const { execSync } = require("child_process")
+const esbuild = require("esbuild")
+
+const watch = process.argv.includes("--watch")
+
+/**
+ * @type {import('esbuild').Plugin}
+ */
+const esbuildProblemMatcherPlugin = {
+	name: "esbuild-problem-matcher",
+
+	setup(build) {
+		build.onStart(() => {
+			console.log("[watch] build started")
+		})
+		build.onEnd((result) => {
+			result.errors.forEach(({ text, location }) => {
+				console.error(`✘ [ERROR] ${text}`)
+				console.error(`    ${location.file}:${location.line}:${location.column}:`)
+			})
+			console.log("[watch] build finished")
+		})
+	},
+}
+
+const srcConfig = {
+	bundle: true,
+	minify: false,
+	sourcemap: true,
+	sourcesContent: true,
+	logLevel: "silent",
+	entryPoints: ["src/packages/**/*.ts"],
+	outdir: "out/packages",
+	format: "cjs",
+	platform: "node",
+	define: {
+		"process.env.IS_DEV": "true",
+		"process.env.IS_TEST": "true",
+	},
+	external: ["vscode"],
+	plugins: [esbuildProblemMatcherPlugin],
+}
+
+async function main() {
+	const srcCtx = await esbuild.context(srcConfig)
+
+	if (watch) {
+		await srcCtx.watch()
+	} else {
+		await srcCtx.rebuild()
+
+		await srcCtx.dispose()
+	}
+}
+
+execSync("tsc -p ./tsconfig.test.json --outDir out", { encoding: "utf-8" })
+
+main().catch((e) => {
+	console.error(e)
+	process.exit(1)
+})
diff --git a/src/core/context/context-tracking/ModelContextTracker.test.ts b/src/core/context/context-tracking/ModelContextTracker.test.ts
index 53ff8a4bf..74e967c49 100644
--- a/src/core/context/context-tracking/ModelContextTracker.test.ts
+++ b/src/core/context/context-tracking/ModelContextTracker.test.ts
@@ -76,26 +76,6 @@ describe("ModelContextTracker", () => {
 		}
 	})
 
-	it("should throw an error when controller is dereferenced", async () => {
-		// Create a new tracker with a controller that will be garbage collected
-		const weakTracker = new ModelContextTracker(mockContext, taskId)
-
-		// Force the WeakRef to return null by overriding the deref method
-		const weakRef = { deref: sandbox.stub().returns(null) }
-		sandbox.stub(WeakRef.prototype, "deref").callsFake(() => weakRef.deref())
-
-		try {
-			// Try to call the method - this should throw
-			await weakTracker.recordModelUsage("any-provider", "any-model", "any-mode")
-
-			// If we get here, the test should fail
-			expect.fail("Expected an error to be thrown")
-		} catch (error) {
-			// Verify the error message
-			expect(error.message).to.equal("Unable to access extension context")
-		}
-	})
-
 	it("should append model usage to existing entries", async () => {
 		// Add an existing model usage entry
 		const existingTimestamp = 1617200000000
diff --git a/src/core/storage/disk.ts b/src/core/storage/disk.ts
index ff0b93261..b480e5a49 100644
--- a/src/core/storage/disk.ts
+++ b/src/core/storage/disk.ts
@@ -6,7 +6,7 @@ import { fileExistsAtPath } from "@utils/fs"
 import { ClineMessage } from "@shared/ExtensionMessage"
 import { TaskMetadata } from "@core/context/context-tracking/ContextTrackerTypes"
 import os from "os"
-import { execa } from "execa"
+import { execa } from "@packages/execa"
 
 export const GlobalFileNames = {
 	apiConversationHistory: "api_conversation_history.json",
diff --git a/src/packages/execa.ts b/src/packages/execa.ts
new file mode 100644
index 000000000..398dede24
--- /dev/null
+++ b/src/packages/execa.ts
@@ -0,0 +1 @@
+export { execa } from "execa"
diff --git a/tsconfig.json b/tsconfig.json
index 5bad664de..97dc410f1 100644
--- a/tsconfig.json
+++ b/tsconfig.json
@@ -27,7 +27,8 @@
 			"@integrations/*": ["src/integrations/*"],
 			"@services/*": ["src/services/*"],
 			"@shared/*": ["src/shared/*"],
-			"@utils/*": ["src/utils/*"]
+			"@utils/*": ["src/utils/*"],
+			"@packages/*": ["src/packages/*"]
 		}
 	},
 	"include": ["src/**/*", "scripts/**/*"],