From 89b76adaef99cd225d6a70a47cd5b9fb8f899202 Mon Sep 17 00:00:00 2001 From: "anatoly.shipitz" Date: Thu, 30 Oct 2025 11:48:02 +0100 Subject: [PATCH 1/2] Add test isolation for process.exit in handleRunError function - Introduced a new documentation file outlining a bug related to unexpected process.exit calls during test execution, which blocks CI/CD pipelines. - Updated tests in src/index.test.ts to use fake timers and ensure that process.exit is called only after the test completes, preventing race conditions. - Enhanced test coverage to verify that process.exit is not called immediately and is invoked correctly after a timeout. These changes improve test reliability and prevent CI/CD failures due to unhandled process exits. --- docs/bugs/process-exit-in-tests-bug.md | 89 ++++++++++++++++++++++++++ workers/main/src/index.test.ts | 31 ++++++++- 2 files changed, 117 insertions(+), 3 deletions(-) create mode 100644 docs/bugs/process-exit-in-tests-bug.md diff --git a/docs/bugs/process-exit-in-tests-bug.md b/docs/bugs/process-exit-in-tests-bug.md new file mode 100644 index 00000000..fe503ec5 --- /dev/null +++ b/docs/bugs/process-exit-in-tests-bug.md @@ -0,0 +1,89 @@ +# Bug: Unexpected process.exit Call During Test Execution + +## Title + +Tests failing due to actual process.exit invocation in test environment + +## Background + +When executing the CI/CD pipeline in GitHub Actions (Job: "Add permission to write comments #574"), an error occurs in the workers/main module tests. Tests for the handleRunError function fail with the error "process.exit unexpectedly called with '1'". + +## Actual Behavior + +- Tests for the handleRunError function execute +- At the end of the test suite execution, a real process.exit(1) call occurs +- Vitest detects the unexpected process.exit call and terminates testing with an error +- All three tests in src/index.test.ts file fail with the same error +- CI/CD pipeline stops at the "Run tests with coverage" step + +## Expected Behavior + +- Tests should completely isolate the process.exit call through mocks +- Real process.exit should not be invoked in the test environment +- All tests should complete successfully +- CI/CD pipeline should pass without errors + +## Detailed Description + +### Technical Problem + +The handleRunError function uses a deferred process.exit call via setTimeout with a 100ms delay. In the current test implementation: + +1. A mock is created for process.exit that throws an error when called +2. The test executes, calling handleRunError +3. handleRunError starts a setTimeout that should call process.exit after 100ms +4. The test completes before the timer expires +5. After test completion, mocks are restored +6. After 100ms, the real process.exit fires since the mock has been removed +7. Vitest intercepts the unexpected process.exit call and fails with an error + +### Root Cause + +The asynchronous nature of setTimeout combined with synchronous test execution creates a race condition. The test does not wait for the deferred call to complete before restoring mocks. + +### Impact + +- All tests in src/index.test.ts fail +- CI/CD pipeline is blocked +- Pull Request #574 cannot be merged +- Development is blocked until fixed + +## Acceptance Criteria + +- [ ] All tests in src/index.test.ts execute successfully locally +- [ ] Tests pass successfully in GitHub Actions CI/CD +- [ ] No real process.exit calls occur during test execution +- [ ] Mocks correctly isolate side effects of the handleRunError function +- [ ] Test coverage does not degrade + +## Additional Information + +### Affected Files + +- workers/main/src/index.ts (line 47: process.exit call in setTimeout) +- workers/main/src/index.test.ts (tests for handleRunError function) + +### GitHub Actions + +- Job: Add permission to write comments #574 +- Failed at step: Run tests with coverage +- Error count: 4 identical errors + +### Tech Stack + +- Test Framework: Vitest +- Runtime: Node.js +- CI/CD: GitHub Actions + +## Priority + +**HIGH** - blocks CI/CD and Pull Request merge + +## Labels + +- bug +- testing +- CI/CD +- vitest +- process-exit +- race-condition diff --git a/workers/main/src/index.test.ts b/workers/main/src/index.test.ts index a42c2429..4f5af83d 100644 --- a/workers/main/src/index.test.ts +++ b/workers/main/src/index.test.ts @@ -6,14 +6,20 @@ describe('handleRunError', () => { let processExitSpy: ReturnType; beforeEach(() => { + // Use fake timers to control setTimeout in handleRunError + vi.useFakeTimers(); // Mock process.exit to prevent actual process termination during tests - processExitSpy = vi.spyOn(process, 'exit').mockImplementation(() => { - throw new Error('process.exit called'); - }); + processExitSpy = vi + .spyOn(process, 'exit') + .mockImplementation((() => {}) as never); }); afterEach(() => { + // Run all pending timers before restoring mocks to prevent race condition + vi.runAllTimers(); processExitSpy.mockRestore(); + // Restore real timers + vi.useRealTimers(); }); it('should log the error', () => { @@ -21,6 +27,7 @@ describe('handleRunError', () => { const logSpy = vi.spyOn(logger, 'error').mockImplementation(() => {}); handleRunError(error); + expect(logSpy).toHaveBeenCalledWith( `Error in main worker: ${error.message}`, ); @@ -46,4 +53,22 @@ describe('handleRunError', () => { ); logSpy.mockRestore(); }); + + it('should call process.exit with code 1 after timeout', () => { + const error = new Error('test error'); + const logSpy = vi.spyOn(logger, 'error').mockImplementation(() => {}); + + handleRunError(error); + + // Process.exit should not be called immediately + expect(processExitSpy).not.toHaveBeenCalled(); + + // Fast-forward time by 100ms + vi.advanceTimersByTime(100); + + // Now process.exit should have been called + expect(processExitSpy).toHaveBeenCalledWith(1); + + logSpy.mockRestore(); + }); }); From 0c52f2d0c34607593ec2bae0f9e465dd692fdec0 Mon Sep 17 00:00:00 2001 From: "anatoly.shipitz" Date: Thu, 30 Oct 2025 12:02:50 +0100 Subject: [PATCH 2/2] Remove documentation for process.exit bug in tests - Deleted the documentation file outlining the bug related to unexpected process.exit calls during test execution, which was previously blocking CI/CD pipelines. - This removal reflects the resolution of the issue and the completion of related fixes in the testing framework. The changes streamline the documentation and indicate that the problem has been addressed. --- docs/bugs/process-exit-in-tests-bug.md | 89 -------------------------- 1 file changed, 89 deletions(-) delete mode 100644 docs/bugs/process-exit-in-tests-bug.md diff --git a/docs/bugs/process-exit-in-tests-bug.md b/docs/bugs/process-exit-in-tests-bug.md deleted file mode 100644 index fe503ec5..00000000 --- a/docs/bugs/process-exit-in-tests-bug.md +++ /dev/null @@ -1,89 +0,0 @@ -# Bug: Unexpected process.exit Call During Test Execution - -## Title - -Tests failing due to actual process.exit invocation in test environment - -## Background - -When executing the CI/CD pipeline in GitHub Actions (Job: "Add permission to write comments #574"), an error occurs in the workers/main module tests. Tests for the handleRunError function fail with the error "process.exit unexpectedly called with '1'". - -## Actual Behavior - -- Tests for the handleRunError function execute -- At the end of the test suite execution, a real process.exit(1) call occurs -- Vitest detects the unexpected process.exit call and terminates testing with an error -- All three tests in src/index.test.ts file fail with the same error -- CI/CD pipeline stops at the "Run tests with coverage" step - -## Expected Behavior - -- Tests should completely isolate the process.exit call through mocks -- Real process.exit should not be invoked in the test environment -- All tests should complete successfully -- CI/CD pipeline should pass without errors - -## Detailed Description - -### Technical Problem - -The handleRunError function uses a deferred process.exit call via setTimeout with a 100ms delay. In the current test implementation: - -1. A mock is created for process.exit that throws an error when called -2. The test executes, calling handleRunError -3. handleRunError starts a setTimeout that should call process.exit after 100ms -4. The test completes before the timer expires -5. After test completion, mocks are restored -6. After 100ms, the real process.exit fires since the mock has been removed -7. Vitest intercepts the unexpected process.exit call and fails with an error - -### Root Cause - -The asynchronous nature of setTimeout combined with synchronous test execution creates a race condition. The test does not wait for the deferred call to complete before restoring mocks. - -### Impact - -- All tests in src/index.test.ts fail -- CI/CD pipeline is blocked -- Pull Request #574 cannot be merged -- Development is blocked until fixed - -## Acceptance Criteria - -- [ ] All tests in src/index.test.ts execute successfully locally -- [ ] Tests pass successfully in GitHub Actions CI/CD -- [ ] No real process.exit calls occur during test execution -- [ ] Mocks correctly isolate side effects of the handleRunError function -- [ ] Test coverage does not degrade - -## Additional Information - -### Affected Files - -- workers/main/src/index.ts (line 47: process.exit call in setTimeout) -- workers/main/src/index.test.ts (tests for handleRunError function) - -### GitHub Actions - -- Job: Add permission to write comments #574 -- Failed at step: Run tests with coverage -- Error count: 4 identical errors - -### Tech Stack - -- Test Framework: Vitest -- Runtime: Node.js -- CI/CD: GitHub Actions - -## Priority - -**HIGH** - blocks CI/CD and Pull Request merge - -## Labels - -- bug -- testing -- CI/CD -- vitest -- process-exit -- race-condition