Skip to content

Commit 7db926b

Browse files
committed
refactor: remove tree-kill dependency and refactor killAllProcesses to use native childProc.kill.
Remove last `tree-kill` package usage.
1 parent b47078f commit 7db926b

File tree

5 files changed

+47
-32
lines changed

5 files changed

+47
-32
lines changed

.github/workflows/pr.yml

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ jobs:
124124
- name: Run CLI E2E tests
125125
run: pnpm bazel test --test_env=E2E_SHARD_TOTAL=6 --test_env=E2E_SHARD_INDEX=${{ matrix.shard }} --config=e2e //tests:e2e.${{ matrix.subset }}_node${{ matrix.node }}
126126

127-
build-e2e-windows-subset:
127+
build-e2e-windows:
128128
runs-on: ubuntu-latest
129129
steps:
130130
- name: Initialize environment
@@ -133,12 +133,15 @@ jobs:
133133
uses: angular/dev-infra/github-actions/bazel/setup@9cc477855b9788df6257301074a1629bc3545722
134134
- name: Setup Bazel RBE
135135
uses: angular/dev-infra/github-actions/bazel/configure-remote@9cc477855b9788df6257301074a1629bc3545722
136+
with:
137+
google_credential: ${{ secrets.RBE_TRUSTED_BUILDS_USER }}
136138
- name: Install node modules
137139
run: pnpm install --frozen-lockfile
138140
- name: Build E2E tests for Windows on Linux
139141
run: |
140142
pnpm bazel build \
141143
--config=e2e \
144+
//tests:e2e.webpack_node24 \
142145
//tests:e2e.esbuild_node24 \
143146
--platforms=tools:windows_x64
144147
- name: Store built Windows E2E tests
@@ -151,8 +154,14 @@ jobs:
151154
retention-days: 1
152155
if-no-files-found: 'error'
153156

154-
e2e-windows-subset:
155-
needs: build-e2e-windows-subset
157+
e2e-windows:
158+
needs: build-e2e-windows
159+
strategy:
160+
fail-fast: false
161+
matrix:
162+
node: [24]
163+
subset: [esbuild, webpack]
164+
shard: [0, 1, 2, 3, 4, 5]
156165
runs-on: windows-2025
157166
steps:
158167
- name: Initialize environment
@@ -167,10 +176,10 @@ jobs:
167176
- name: Run CLI E2E tests
168177
uses: ./.github/shared-actions/windows-bazel-test
169178
with:
170-
test_target_name: e2e.esbuild_node24
179+
test_target_name: e2e.${{ matrix.subset }}_node${{ matrix.node }}
171180
env:
172-
E2E_SHARD_TOTAL: 1
173-
TESTBRIDGE_TEST_ONLY: tests/basic/{build,rebuild,serve}.ts
181+
E2E_SHARD_TOTAL: 6
182+
E2E_SHARD_INDEX: ${{ matrix.shard }}
174183

175184
e2e-package-managers:
176185
needs: build

pnpm-lock.yaml

Lines changed: 0 additions & 9 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

tests/e2e/utils/BUILD.bazel

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,5 @@ ts_project(
2020
"//:node_modules/verdaccio-auth-memory",
2121
"//tests:node_modules/@types/tar-stream",
2222
"//tests:node_modules/tar-stream",
23-
"//tests:node_modules/tree-kill",
2423
],
2524
)

tests/e2e/utils/process.ts

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
import { spawn, SpawnOptions } from 'node:child_process';
22
import * as child_process from 'node:child_process';
33
import { getGlobalVariable, getGlobalVariablesEnv } from './env';
4-
import treeKill from 'tree-kill';
54
import { delimiter, join, resolve } from 'node:path';
65
import { stripVTControlCharacters, styleText } from 'node:util';
6+
import { assertIsError } from './utils';
77

88
interface ExecOptions {
99
silent?: boolean;
@@ -255,26 +255,43 @@ export async function waitForAnyProcessOutputToMatch(
255255
return matchingProcess;
256256
}
257257

258-
export async function killAllProcesses(signal = 'SIGTERM'): Promise<void> {
258+
/**
259+
* Kills a process by PID
260+
* @param pid The PID of the process to kill
261+
* @param signal The signal to send to the process
262+
*/
263+
async function killProcess(pid: number, signal: NodeJS.Signals): Promise<void> {
264+
if (process.platform === 'win32') {
265+
// /T kills child processes, /F forces it
266+
await new Promise<void>((resolve) => {
267+
child_process.exec(`taskkill /pid ${pid} /T /F`, () => resolve());
268+
});
269+
} else {
270+
// Use -pid to signal the entire process group
271+
try {
272+
process.kill(-pid, signal);
273+
} catch (error) {
274+
assertIsError(error);
275+
if (error.code !== 'ESRCH') {
276+
throw error;
277+
}
278+
}
279+
}
280+
}
281+
282+
/**
283+
* Kills all tracked processes
284+
*/
285+
export async function killAllProcesses(signal: NodeJS.Signals = 'SIGTERM'): Promise<void> {
259286
const processesToKill: Promise<void>[] = [];
260287

261288
while (_processes.length) {
262289
const childProc = _processes.pop();
263-
if (!childProc || childProc.pid === undefined) {
290+
if (!childProc || childProc.pid === undefined || childProc.killed) {
264291
continue;
265292
}
266293

267-
processesToKill.push(
268-
new Promise<void>((resolve) => {
269-
treeKill(childProc.pid!, signal, () => {
270-
// Ignore all errors.
271-
// This is due to a race condition with the `waitForMatch` logic.
272-
// where promises are resolved on matches and not when the process terminates.
273-
// Also in some cases in windows we get `The operation attempted is not supported`.
274-
resolve();
275-
});
276-
}),
277-
);
294+
processesToKill.push(killProcess(childProc.pid, signal));
278295
}
279296

280297
await Promise.all(processesToKill);

tests/package.json

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
"devDependencies": {
33
"@types/tar-stream": "3.1.4",
44
"@angular-devkit/schematics": "workspace:*",
5-
"tar-stream": "3.1.7",
6-
"tree-kill": "1.2.2"
5+
"tar-stream": "3.1.7"
76
}
87
}

0 commit comments

Comments
 (0)