diff --git a/contributions/66390.md b/contributions/66390.md new file mode 100644 index 00000000..eac7fc91 --- /dev/null +++ b/contributions/66390.md @@ -0,0 +1,237 @@ +--- +pr-url: https://github.com/nodejs/node/pull/66390 +--- + +# test_runner: avoid waiting on inherited stdio with force exit + +## 문제 내용 + +Node.js Test Runner에서 `--test-force-exit`를 사용하더라도, test file이 `stdio: 'inherit'`로 child process를 생성한 경우 inherited stdout이 계속 열린 상태로 남아 Test Runner가 바로 종료되지 않는 문제를 분석했다. + +관련 Issue: #66336 + +문제 흐름은 다음과 같았다. + +```text +Test file 실행 +→ child process 생성 +→ stdio: inherit +→ Test file 종료 +→ child process는 계속 살아 있음 +→ inherited stdout pipe도 계속 열려 있음 +→ Parent Test Runner가 stdout EOF를 기다림 +→ --test-force-exit인데도 종료 지연 +``` + +## 원인 분석 + +기존 Test Runner는 test file process가 종료된 것뿐 아니라 child stdout stream까지 완료되기를 기다리고 있었다. + +기존 흐름은 다음과 같은 형태였다. + +```js +await SafePromiseAll([ + once(child, 'exit', { signal: t.signal }), + finished(child.stdout, { signal: t.signal }), +]); +``` + +즉 child process 자체가 종료돼도 `finished(child.stdout)`이 완료되지 않으면 Test Runner는 계속 대기한다. + +일반적인 실행에서는 stdout의 데이터를 끝까지 기다리는 것이 필요하지만, `--test-force-exit`에서는 모든 알려진 테스트 결과가 이미 완료된 경우 inherited stdout 때문에 계속 기다리는 것이 옵션의 목적과 맞지 않았다. + +## 해결 방향 + +stdout EOF만 기다리는 대신 Test Report 자체가 완료됐다는 신호를 추가로 사용할 수 있도록 했다. + +이를 위해 `FileTest`에 report 완료 상태를 나타내는 Promise를 추가했다. + +```js +#reportFinished; +#resolveReportFinished; +``` + +constructor에서 Promise를 생성했다. + +```js +const { + promise, + resolve: resolveReportFinished, +} = PromiseWithResolvers(); + +this.#reportFinished = promise; +this.#resolveReportFinished = resolveReportFinished; +``` + +그리고 `test:summary` event가 들어오면 report가 완료된 것으로 처리했다. + +```js +if (item.type === 'test:summary') { + this.#resolveReportFinished(); +} +``` + +## forceExit일 때만 추가 완료 신호 사용 + +기존 stdout completion 동작을 모든 경우에 변경하지는 않았다. + +`--test-force-exit`를 사용할 때만 stdout EOF와 test report 완료 중 먼저 끝나는 것을 사용하도록 했다. + +```js +const stdoutFinished = finished( + child.stdout, + { __proto__: null, signal: t.signal }, +); + +const reportFinished = opts.forceExit ? + SafePromiseRace([stdoutFinished, subtest.reportFinished]) : + stdoutFinished; +``` + +즉 동작은 다음과 같다. + +```text +일반 실행 +→ stdout EOF까지 기다림 + +--test-force-exit +→ stdout EOF 또는 test:summary 중 + 먼저 완료되는 시점을 사용 +``` + +이를 통해 일반 Test Runner의 기존 동작은 유지하면서 force exit일 때만 inherited stdio 대기를 우회하도록 했다. + +## Regression Test + +문제를 재현하기 위한 fixture를 추가했다. + +fixture에서는 child process를 생성하고 stdio를 상속하도록 했다. + +```js +'use strict'; + +const { spawn } = require('node:child_process'); +const { test } = require('node:test'); + +test('leaks a child with inherited stdio', () => { + spawn( + process.execPath, + [ + '-e', + `setTimeout(() => {}, ${process.env.TEST_RUNNER_STALL_MS})`, + ], + { stdio: 'inherit' }, + ); +}); +``` + +child process는 일정 시간 동안 살아 있도록 했다. + +## 종료 시간을 직접 검증 + +이번 문제는 결국 종료되느냐보다 얼마나 빨리 종료되느냐가 중요했다. + +그래서 Test Runner 실행 시간을 직접 측정했다. + +```js +const maxDuration = common.platformTimeout(2000); +const stallDuration = common.platformTimeout(4000); +``` + +child process는 약 4초 동안 살아 있도록 하고, `--test-force-exit`를 사용하는 Test Runner는 그보다 훨씬 빠른 2초 이내에 종료되어야 하도록 구성했다. + +실제 Test Runner는 다음과 같이 실행했다. + +```js +const result = spawnSync( + process.execPath, + [ + '--test', + '--test-force-exit', + fixture, + ], + { + encoding: 'utf8', + env: { + ...process.env, + TEST_RUNNER_STALL_MS: String(stallDuration), + }, + }, +); +``` + +그리고 종료 시간을 확인했다. + +```js +assert.ok( + duration < maxDuration, + `test runner took ${duration}ms to exit`, +); +``` + +## Reviewer 질문 + +PR을 올린 뒤 `addaleax`에게 다음과 같은 질문을 받았다. + +```text +Inherited stdout pipe가 열린 상태는 +--test-force-exit와 관계없이 발생할 수 있는 것 아닌가? +``` + +실제로 inherited stdout 자체는 force exit 여부와 관계없이 열려 있을 수 있다. + +하지만 이번 수정에서 중요한 차이는 종료 정책이었다. + +```text +일반 실행 +→ stdout이 끝날 때까지 기존처럼 기다림 + +--test-force-exit +→ 알려진 테스트가 모두 완료됐다면 + inherited stdout EOF 때문에 계속 기다리지 않음 +``` + +그래서 이번 변경은 inherited stdout 문제 자체를 force exit에서만 발생하는 문제로 보는 것이 아니라, **force exit를 사용할 때만 대기 조건을 다르게 적용하는 변경**이라고 답변했다. + +## Codecov 상태 + +Codecov에서는 현재 patch coverage가 약 `86.95%`로 표시됐으며, 변경된 코드 중 3개 line이 coverage에 포함되지 않은 상태다. + +전체 project coverage는 유지되고 있지만, PR이 아직 Open 상태이기 때문에 이 부분은 이후 리뷰 과정에서 추가 확인이 필요할 수 있다. + +## 현재 상태 + +현재 PR #66390은 아직 Open 상태다. + +진행 흐름은 다음과 같다. + +```text +Issue #66336 분석 +→ inherited stdio 문제 재현 +→ Test Runner 종료 조건 분석 +→ test:summary를 report 완료 신호로 추가 +→ forceExit일 때 SafePromiseRace 적용 +→ Regression fixture 추가 +→ 실제 종료 시간 측정 테스트 작성 +→ PR #66390 생성 +→ @nodejs/test_runner 리뷰 요청 +→ addaleax 질문 +→ forceExit와 일반 실행의 차이 설명 +→ 추가 리뷰 대기 중 +``` + +## 배운 점 + +이번 기여를 통해 process lifecycle, stdout stream lifecycle, Test Report lifecycle이 서로 같은 시점에 끝나는 것은 아니라는 점을 배웠다. + +특히 inherited stdio가 존재하면 다음과 같은 상태가 가능하다. + +```text +Test Report 완료 +≠ +stdout EOF +``` + +일반 실행에서는 stdout까지 기다리는 기존 동작을 유지해야 하지만, `--test-force-exit`에서는 이미 test report가 완료됐다면 inherited stdout 때문에 계속 대기하지 않도록 종료 조건을 다르게 적용할 수 있다는 점을 이해하게 됐다. + +또한 timing 기반 regression test를 작성할 때는 CI 환경 차이까지 고려해 `common.platformTimeout()`을 사용하는 것이 중요하다는 점도 경험했다.