test: report which integration suite failed and why (#342347)

* test: report which integration suite failed and why Integration test steps sometimes exited non-zero with no failing test in the log. test-integration.sh and test-remote-integration.sh stop at the first failing suite (set -e) without saying which suite failed. The Agent Host E2E runner printed PASS for every suite and then exited 1 when the RAM-backed pass could not unmount its tmpfs. - test-integration.sh and test-remote-integration.sh name the failing suite and its exit code on exit, and add a GitHub Actions error annotation. Agent Host E2E is named separately from the node.js tests. - tmpfs.sh exits with status 3 when the tests passed but the tmpfs could not be cleaned up. - runner.ts lists each failed storage pass at the end of its output, and says when every suite passed but tmpfs cleanup failed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * test: expect the tmpfs cleanup failure status in the storage wrapper tests Update the storage wrapper tests for exit status 3 when an otherwise passing run cannot unmount the tmpfs, and cover that a failing test keeps its own exit code when the unmount also fails. Also remove a duplicated README sentence. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-10-28 13:26:52 +08:00 · 2026-10-18 01:24:22 +00:00
1 parent f8bae1d913
commit 09d2db6c2f
@@ -10,6 +10,26 @@ fi

 cd "$ROOT"

+# Names the suite that is running, so that a failure that stops the script
+# (set -e) reports which suite failed and with which exit code.
+CURRENT_SUITE=""
+start_suite() {
+	CURRENT_SUITE="$1"
+	echo
+	echo "### $1"
+	echo
+}
+report_failed_suite() {
+	local status=$?
+	if [[ $status -ne 0 && -n "$CURRENT_SUITE" ]]; then
+		echo "Integration tests failed in '$CURRENT_SUITE' with exit code $status." >&2
+		if [[ "$GITHUB_ACTIONS" == "true" ]]; then
+			echo "::error title=Integration tests failed::'$CURRENT_SUITE' exited with code $status"
+		fi
+	fi
+}
+trap report_failed_suite EXIT
+
 # Parse arguments
 EXTRA_ARGS=()
 BUILD_ARGS=()
@@ -176,15 +196,15 @@ fi
 # Unit tests

 if [[ -z "$SUITE_FILTER" ]]; then
-	echo
-	echo "### node.js integration tests"
-	echo
+	start_suite "node.js integration tests"
 	node --input-type=module -e "import { getAgentHostTestProfile } from './src/vs/platform/agentHost/test/node/agentHostTestProfile.ts'; console.log('Agent Host test profile:', getAgentHostTestProfile(), '(' + (process.env.AGENT_HOST_TEST_PROFILE_REASON ?? 'default') + ')');"
 	if [[ -z "$RUN_GLOB" && -z "$RUN_FILE" ]]; then
 		if [[ "$VSCODE_SKIP_AGENT_HOST_E2E" == "1" ]]; then
 			echo "Skipping Agent Host E2E tests because no relevant files changed."
 		else
+			CURRENT_SUITE="Agent Host E2E"
 			node ./test/integration/agentHost/runner.ts "${EXTRA_ARGS[@]}"
+			CURRENT_SUITE="node.js integration tests"
 		fi
 		VSCODE_SKIP_PRELAUNCH=1 ./scripts/test.sh --runGlob "**/*.integrationTest.js" --excludeRunGlob "$AGENT_HOST_E2E_GLOB" "${EXTRA_ARGS[@]}"
 	else
@@ -219,113 +239,85 @@ else
 fi

 if should_run_suite api-folder; then
-echo
-echo "### API tests (folder)"
-echo
+start_suite "API tests (folder)"
 "$INTEGRATION_TEST_ELECTRON_PATH" $ROOT/extensions/vscode-api-tests/testWorkspace --enable-proposed-api=vscode.vscode-api-tests --extensionDevelopmentPath=$ROOT/extensions/vscode-api-tests --extensionTestsPath=$ROOT/extensions/vscode-api-tests/out/singlefolder-tests $API_TESTS_EXTRA_ARGS
 kill_app
 fi

 if should_run_suite api-workspace; then
-echo
-echo "### API tests (workspace)"
-echo
+start_suite "API tests (workspace)"
 "$INTEGRATION_TEST_ELECTRON_PATH" $ROOT/extensions/vscode-api-tests/testworkspace.code-workspace --enable-proposed-api=vscode.vscode-api-tests --extensionDevelopmentPath=$ROOT/extensions/vscode-api-tests --extensionTestsPath=$ROOT/extensions/vscode-api-tests/out/workspace-tests $API_TESTS_EXTRA_ARGS
 kill_app
 fi

 if should_run_suite colorize; then
-echo
-echo "### Colorize tests"
-echo
+start_suite "Colorize tests"
 npm run test-extension -- -l vscode-colorize-tests "${GREP_ARGS[@]}"
 kill_app
 fi

 if should_run_suite terminal-suggest; then
-echo
-echo "### Terminal Suggest tests"
-echo
+start_suite "Terminal Suggest tests"
 npm run test-extension -- -l terminal-suggest --enable-proposed-api=vscode.vscode-api-tests "${GREP_ARGS[@]}"
 kill_app
 fi

 if should_run_suite typescript; then
-echo
-echo "### TypeScript tests"
-echo
+start_suite "TypeScript tests"
 "$INTEGRATION_TEST_ELECTRON_PATH" $ROOT/extensions/typescript-language-features/test-workspace --extensionDevelopmentPath=$ROOT/extensions/typescript-language-features --extensionTestsPath=$ROOT/extensions/typescript-language-features/out/test/unit $API_TESTS_EXTRA_ARGS
 kill_app
 fi

 if should_run_suite markdown; then
-echo
-echo "### Markdown tests"
-echo
+start_suite "Markdown tests"
 npm run test-extension -- -l markdown-language-features "${GREP_ARGS[@]}"
 kill_app
 fi

 if should_run_suite emmet; then
-echo
-echo "### Emmet tests"
-echo
+start_suite "Emmet tests"
 "$INTEGRATION_TEST_ELECTRON_PATH" $ROOT/extensions/emmet/test-workspace --extensionDevelopmentPath=$ROOT/extensions/emmet --extensionTestsPath=$ROOT/extensions/emmet/out/test $API_TESTS_EXTRA_ARGS
 kill_app
 fi

 if should_run_suite git; then
-echo
-echo "### Git tests"
-echo
+start_suite "Git tests"
 "$INTEGRATION_TEST_ELECTRON_PATH" $(mktemp -d 2>/dev/null) --extensionDevelopmentPath=$ROOT/extensions/git --extensionTestsPath=$ROOT/extensions/git/out/test $API_TESTS_EXTRA_ARGS
 kill_app
 fi

 if should_run_suite git-base; then
-echo
-echo "### Git Base tests"
-echo
+start_suite "Git Base tests"
 npm run test-extension -- -l git-base "${GREP_ARGS[@]}"
 kill_app
 fi

 if should_run_suite ipynb; then
-echo
-echo "### Ipynb tests"
-echo
+start_suite "Ipynb tests"
 npm run test-extension -- -l ipynb "${GREP_ARGS[@]}"
 kill_app
 fi

 if should_run_suite notebook-renderers; then
-echo
-echo "### Notebook Output tests"
-echo
+start_suite "Notebook Output tests"
 npm run test-extension -- -l notebook-renderers "${GREP_ARGS[@]}"
 kill_app
 fi

 if should_run_suite configuration-editing; then
-echo
-echo "### Configuration editing tests"
-echo
+start_suite "Configuration editing tests"
 npm run test-extension -- -l configuration-editing "${GREP_ARGS[@]}"
 kill_app
 fi

 if should_run_suite github-authentication; then
-echo
-echo "### GitHub Authentication tests"
-echo
+start_suite "GitHub Authentication tests"
 npm run test-extension -- -l github-authentication "${GREP_ARGS[@]}"
 kill_app
 fi

 if should_run_suite copilot; then
-echo
-echo "### Copilot tests"
-echo
+start_suite "Copilot tests"
 npm run test-extension -- -l copilot "${GREP_ARGS[@]}"
 kill_app
 fi
@@ -333,23 +325,17 @@ fi
 # Tests standalone (CommonJS)

 if should_run_suite css; then
-echo
-echo "### CSS tests"
-echo
+start_suite "CSS tests"
 cd "$ROOT/extensions/css-language-features/server" && "$ROOT/scripts/node-electron.sh" test/index.js
 fi

 if should_run_suite html; then
-echo
-echo "### HTML tests"
-echo
+start_suite "HTML tests"
 cd "$ROOT/extensions/html-language-features/server" && "$ROOT/scripts/node-electron.sh" test/index.js
 fi

 if should_run_suite json; then
-echo
-echo "### JSON tests"
-echo
+start_suite "JSON tests"
 "$ROOT/scripts/node-electron.sh" "$ROOT/extensions/json-language-features/client/out/test/index.js" "${BUILD_ARGS[@]}"
 fi

@@ -15,6 +15,26 @@ TESTRESOLVER_DATA_FOLDER=`mktemp -d 2>/dev/null`

 cd $ROOT

+# Names the suite that is running, so that a failure that stops the script
+# (set -e) reports which suite failed and with which exit code.
+CURRENT_SUITE=""
+start_suite() {
+	CURRENT_SUITE="$1"
+	echo
+	echo "### $1"
+	echo
+}
+report_failed_suite() {
+	local status=$?
+	if [[ $status -ne 0 && -n "$CURRENT_SUITE" ]]; then
+		echo "Integration tests failed in '$CURRENT_SUITE' with exit code $status." >&2
+		if [[ "$GITHUB_ACTIONS" == "true" ]]; then
+			echo "::error title=Integration tests failed::'$CURRENT_SUITE' exited with code $status"
+		fi
+	fi
+}
+trap report_failed_suite EXIT
+
 if [[ "$1" == "" ]]; then
 	AUTHORITY=vscode-remote://test+test
 	EXT_PATH=$ROOT/extensions
@@ -73,51 +93,35 @@ echo "Storing log files into '$VSCODELOGSDIR'."

 # Tests in the extension host

-echo
-echo "### API tests (folder)"
-echo
+start_suite "API tests (folder)"
 "$INTEGRATION_TEST_ELECTRON_PATH" --folder-uri=$REMOTE_VSCODE/vscode-api-tests/testWorkspace --extensionDevelopmentPath=$REMOTE_VSCODE/vscode-api-tests --extensionTestsPath=$REMOTE_VSCODE/vscode-api-tests/out/singlefolder-tests $API_TESTS_EXTRA_ARGS $EXTRA_INTEGRATION_TEST_ARGUMENTS
 kill_app

-echo
-echo "### API tests (workspace)"
-echo
+start_suite "API tests (workspace)"
 "$INTEGRATION_TEST_ELECTRON_PATH" --file-uri=$REMOTE_VSCODE/vscode-api-tests/testworkspace.code-workspace --extensionDevelopmentPath=$REMOTE_VSCODE/vscode-api-tests --extensionTestsPath=$REMOTE_VSCODE/vscode-api-tests/out/workspace-tests $API_TESTS_EXTRA_ARGS $EXTRA_INTEGRATION_TEST_ARGUMENTS
 kill_app

-echo
-echo "### TypeScript tests"
-echo
+start_suite "TypeScript tests"
 "$INTEGRATION_TEST_ELECTRON_PATH" --folder-uri=$REMOTE_VSCODE/typescript-language-features/test-workspace --extensionDevelopmentPath=$REMOTE_VSCODE/typescript-language-features --extensionTestsPath=$REMOTE_VSCODE/typescript-language-features/out/test/unit $API_TESTS_EXTRA_ARGS $EXTRA_INTEGRATION_TEST_ARGUMENTS
 kill_app

-echo
-echo "### Markdown tests"
-echo
+start_suite "Markdown tests"
 "$INTEGRATION_TEST_ELECTRON_PATH" --folder-uri=$REMOTE_VSCODE/markdown-language-features/test-workspace --extensionDevelopmentPath=$REMOTE_VSCODE/markdown-language-features --extensionTestsPath=$REMOTE_VSCODE/markdown-language-features/out/test $API_TESTS_EXTRA_ARGS $EXTRA_INTEGRATION_TEST_ARGUMENTS
 kill_app

-echo
-echo "### Emmet tests"
-echo
+start_suite "Emmet tests"
 "$INTEGRATION_TEST_ELECTRON_PATH" --folder-uri=$REMOTE_VSCODE/emmet/test-workspace --extensionDevelopmentPath=$REMOTE_VSCODE/emmet --extensionTestsPath=$REMOTE_VSCODE/emmet/out/test $API_TESTS_EXTRA_ARGS $EXTRA_INTEGRATION_TEST_ARGUMENTS
 kill_app

-echo
-echo "### Git tests"
-echo
+start_suite "Git tests"
 "$INTEGRATION_TEST_ELECTRON_PATH" --folder-uri=$AUTHORITY$(mktemp -d 2>/dev/null) --extensionDevelopmentPath=$REMOTE_VSCODE/git --extensionTestsPath=$REMOTE_VSCODE/git/out/test $API_TESTS_EXTRA_ARGS $EXTRA_INTEGRATION_TEST_ARGUMENTS
 kill_app

-echo
-echo "### Ipynb tests"
-echo
+start_suite "Ipynb tests"
 "$INTEGRATION_TEST_ELECTRON_PATH" --folder-uri=$AUTHORITY$(mktemp -d 2>/dev/null) --extensionDevelopmentPath=$REMOTE_VSCODE/ipynb --extensionTestsPath=$REMOTE_VSCODE/ipynb/out/test $API_TESTS_EXTRA_ARGS $EXTRA_INTEGRATION_TEST_ARGUMENTS
 kill_app

-echo
-echo "### Configuration editing tests"
-echo
+start_suite "Configuration editing tests"
 "$INTEGRATION_TEST_ELECTRON_PATH" --folder-uri=$AUTHORITY$(mktemp -d 2>/dev/null) --extensionDevelopmentPath=$REMOTE_VSCODE/configuration-editing --extensionTestsPath=$REMOTE_VSCODE/configuration-editing/out/test $API_TESTS_EXTRA_ARGS $EXTRA_INTEGRATION_TEST_ARGUMENTS
 kill_app

@@ -120,7 +120,7 @@ esac`,
 			holderDetails: result.stderr.split('\n').filter(line => line.includes('12345') && !line.includes('.c..m')),
 			diagnosticCalls: result.calls.slice(2),
 		}, {
-			status: 1,
+			status: 3,
 			errorReported: true,
 			directoryRetained: true,
 			holdersReported: true,
@@ -178,13 +178,26 @@ esac`,
 			unmountFailureReported: result.stderr.includes('Failed to unmount'),
 			diagnosticFailureReported: result.stderr.includes('holder diagnostics exited with status 29'),
 		}, {
-			status: 1,
+			status: 3,
 			directoryRetained: true,
 			unmountFailureReported: true,
 			diagnosticFailureReported: true,
 		});
 	});

+	(isWindows ? test.skip : test)('unmount failure keeps a failing test exit code', () => {
+		const result = run('umount', 7);
+		assert.deepStrictEqual({
+			status: result.status,
+			unmountFailureReported: result.stderr.includes('Failed to unmount'),
+			directoryRetained: existsSync(result.mountDirectory),
+		}, {
+			status: 7,
+			unmountFailureReported: true,
+			directoryRetained: true,
+		});
+	});
+
 	(isLinux ? test : test.skip)('real Linux fuser identifies holders with the diagnostic mount argument form', () => {
 		const root = fileURLToPath(new URL('../../../../../../', import.meta.url));
 		const directory = mkdtempSync(join(root, '.build', 'agent-host-storage-fuser-'));
@@ -707,7 +707,7 @@ Codex record/replay servers disable `features.plugins` to keep this unrelated ma

 ### Linux suites pass but tmpfs cannot be unmounted

-An `umount: target is busy` failure means a process or kernel reference still holds the mount; passing test bodies do not prove that cleanup finished. The wrapper runs a read-only `fuser -vm` probe for at most ten seconds to report holder PIDs, command names, and access types in the task log, then describes each holder it reported, within the same ten-second budget: its parent PID, process group, age in seconds, full command line, and the paths it holds open on the mount. Match the age against the test log's timestamps to find the test that started it; a parent PID of 1 means it outlived the process that started it. Probe errors are reported separately, and the original cleanup failure remains fatal. Correlate the holders with server/provider cleanup logs before changing ownership; do not use lazy unmounts, process-name kills, or retries to conceal a leak.
+An `umount: target is busy` failure means a process or kernel reference still holds the mount; passing test bodies do not prove that cleanup finished. When every suite passed, the wrapper exits with status 3 instead of the command's status, and the runner ends with `Agent Host E2E storage passes failed:` naming the RAM-backed pass and this cleanup failure. The wrapper runs a read-only `fuser -vm` probe for at most ten seconds to report holder PIDs, command names, and access types in the task log, then describes each holder it reported, within the same ten-second budget: its parent PID, process group, age in seconds, full command line, and the paths it holds open on the mount. Match the age against the test log's timestamps to find the test that started it; a parent PID of 1 means it outlived the process that started it. Probe errors are reported separately, and the original cleanup failure remains fatal. Correlate the holders with server/provider cleanup logs before changing ownership; do not use lazy unmounts, process-name kills, or retries to conceal a leak.

 ### Fixture leaks a username / absolute path / token

@@ -21,6 +21,8 @@ const incompatibleFlags = [
 	'AGENT_HOST_UPDATE_SNAPSHOTS',
 ];
 type StorageMode = 'disk' | 'tmpfs' | 'split';
+// Keep in sync with CLEANUP_FAILED_STATUS in tmpfs.sh.
+const tmpfsCleanupFailedStatus = 3;
 const diskLifecyclePattern = [
 	'session metadata history and provider context survive a host restart',
 	'archiving a never-restored session survives a host restart',
@@ -148,6 +150,7 @@ function runStoragePasses(storage: 'tmpfs' | 'split', jobs: number, args: readon
 	const environment: NodeJS.ProcessEnv = { ...process.env, VSCODE_SKIP_PRELAUNCH: '1' };
 	delete environment.ELECTRON_RUN_AS_NODE;
 	const runnerArgs = [__filename, '--storage', 'disk', '--jobs', String(jobs)];
+	const failedPasses: string[] = [];
 	const tmpfs = spawnSync('bash', [
 		join(__dirname, 'tmpfs.sh'),
 		process.execPath, ...runnerArgs, ...args,
@@ -155,8 +158,10 @@ function runStoragePasses(storage: 'tmpfs' | 'split', jobs: number, args: readon
 	if (tmpfs.error) {
 		throw tmpfs.error;
 	}
-	if (tmpfs.status !== 0) {
-		process.exitCode = 1;
+	if (tmpfs.status === tmpfsCleanupFailedStatus) {
+		failedPasses.push('RAM-backed (tmpfs) pass: every suite passed, but the test tmpfs could not be cleaned up. See the "Failed to unmount" holder diagnostics above.');
+	} else if (tmpfs.status !== 0) {
+		failedPasses.push(`RAM-backed (tmpfs) pass: exited with ${describeExit(tmpfs.status, tmpfs.signal)}. See the failure details above.`);
 	}
 	if (storage === 'split') {
 		const diskArgs = args.filter((argument, index) =>
@@ -178,7 +183,25 @@ function runStoragePasses(storage: 'tmpfs' | 'split', jobs: number, args: readon
 			throw disk.error;
 		}
 		if (disk.status !== 0) {
-			process.exitCode = 1;
+			failedPasses.push(`Disk-backed lifecycle pass: exited with ${describeExit(disk.status, disk.signal)}. See the failure details above.`);
+		}
+	}
+	if (failedPasses.length > 0) {
+		reportFailedStoragePasses(failedPasses);
+		process.exitCode = 1;
+	}
+}
+
+function describeExit(status: number | null, signal: NodeJS.Signals | null): string {
+	return signal ? `signal ${signal}` : `code ${status}`;
+}
+
+function reportFailedStoragePasses(failedPasses: readonly string[]): void {
+	console.log('\nAgent Host E2E storage passes failed:');
+	for (const failedPass of failedPasses) {
+		console.log(`  ${failedPass}`);
+		if (process.env['GITHUB_ACTIONS'] === 'true') {
+			console.log(`::error title=Agent Host E2E failed::${failedPass}`);
 		}
 	}
 }
@@ -11,6 +11,9 @@ fi

 TEST_TMPDIR=$(mktemp -d "${TMPDIR:-/tmp}/agent-host-e2e-tmpfs.XXXXXX")
 MOUNTED=false
+# Exit status when the command succeeded but the tmpfs could not be cleaned up, so the
+# caller can tell a cleanup failure apart from a test failure.
+CLEANUP_FAILED_STATUS=3

 # Describes each holder so it can be traced to the test that leaked it: its parent and process group, its age (match it
 # against the test log's timestamps), its full command line, and what it holds open on the mount. Each probe has the
@@ -61,6 +64,14 @@ reap_orphaned_codex_sandboxes() {
 	return 1
 }

+cleanup_failure_status() {
+	if [[ "$1" -eq 0 ]]; then
+		echo "$CLEANUP_FAILED_STATUS"
+	else
+		echo "$1"
+	fi
+}
+
 cleanup() {
 	local status=$?
 	trap - EXIT
@@ -78,12 +89,12 @@ cleanup() {
 			fi
 			# shellcheck disable=SC2086 # One argument per PID.
 			describe_holders $holders
-			exit 1
+			exit "$(cleanup_failure_status "$status")"
 		fi
 	fi
 	if ! rmdir -- "$TEST_TMPDIR"; then
 		echo "Failed to remove Agent Host tmpfs mount directory: $TEST_TMPDIR" >&2
-		exit 1
+		exit "$(cleanup_failure_status "$status")"
 	fi
 	exit "$status"
 }