fix(tstest): Improve timeout and error handling in test execution along with TAP parser timeout logic improvements.

This commit is contained in:
Philipp Kunz 2025-05-24 02:22:34 +00:00
parent 23addc2d2f
commit 63280e4a9a
4 changed files with 49 additions and 14 deletions

View File

@ -1,5 +1,13 @@
# Changelog # Changelog
## 2025-05-24 - 1.11.2 - fix(tstest)
Improve timeout and error handling in test execution along with TAP parser timeout logic improvements.
- In the TAP parser, ensure that expected tests are properly set when no tests are defined to avoid false negatives on timeout.
- Use smartshell's terminate method and fallback kill to properly stop the entire process tree on timeout.
- Clean up browser, server, and WebSocket instances reliably even when a timeout occurs.
- Minor improvements in log file filtering and error logging for better clarity.
## 2025-05-24 - 1.11.1 - fix(tstest) ## 2025-05-24 - 1.11.1 - fix(tstest)
Clear timeout identifiers after successful test execution and add local CLAUDE settings Clear timeout identifiers after successful test execution and add local CLAUDE settings

View File

@ -3,6 +3,6 @@
*/ */
export const commitinfo = { export const commitinfo = {
name: '@git.zone/tstest', name: '@git.zone/tstest',
version: '1.11.1', version: '1.11.2',
description: 'a test utility to run tests that match test/**/*.ts' description: 'a test utility to run tests that match test/**/*.ts'
} }

View File

@ -36,18 +36,20 @@ export class TapParser {
* Handle test file timeout * Handle test file timeout
*/ */
public handleTimeout(timeoutSeconds: number) { public handleTimeout(timeoutSeconds: number) {
// If no tests have been defined yet, set expected to 1
if (this.expectedTests === 0) {
this.expectedTests = 1;
}
// Create a fake failing test result for timeout // Create a fake failing test result for timeout
this._getNewTapTestResult(); this._getNewTapTestResult();
this.activeTapTestResult.testOk = false; this.activeTapTestResult.testOk = false;
this.activeTapTestResult.testSettled = true; this.activeTapTestResult.testSettled = true;
this.testStore.push(this.activeTapTestResult); this.testStore.push(this.activeTapTestResult);
// Set expected vs received to force failure
this.expectedTests = 1;
this.receivedTests = 0;
// Log the timeout error // Log the timeout error
if (this.logger) { if (this.logger) {
// First log the test result
this.logger.testResult( this.logger.testResult(
`Test file timeout`, `Test file timeout`,
false, false,
@ -55,9 +57,9 @@ export class TapParser {
`Error: Test file exceeded timeout of ${timeoutSeconds} seconds` `Error: Test file exceeded timeout of ${timeoutSeconds} seconds`
); );
this.logger.testErrorDetails(`Test execution was terminated after ${timeoutSeconds} seconds`); this.logger.testErrorDetails(`Test execution was terminated after ${timeoutSeconds} seconds`);
// Force file end with failure
this.logger.testFileEnd(0, 1, timeoutSeconds * 1000);
} }
// Don't call evaluateFinalResult here, let the caller handle it
} }
private _getNewTapTestResult() { private _getNewTapTestResult() {

View File

@ -156,8 +156,9 @@ export class TsTest {
let timeoutId: NodeJS.Timeout; let timeoutId: NodeJS.Timeout;
const timeoutPromise = new Promise<void>((_resolve, reject) => { const timeoutPromise = new Promise<void>((_resolve, reject) => {
timeoutId = setTimeout(() => { timeoutId = setTimeout(async () => {
execResultStreaming.childProcess.kill('SIGTERM'); // Use smartshell's terminate() to kill entire process tree
await execResultStreaming.terminate();
reject(new Error(`Test file timed out after ${this.timeoutSeconds} seconds`)); reject(new Error(`Test file timed out after ${this.timeoutSeconds} seconds`));
}, timeoutMs); }, timeoutMs);
}); });
@ -172,6 +173,12 @@ export class TsTest {
} catch (error) { } catch (error) {
// Handle timeout error // Handle timeout error
tapParser.handleTimeout(this.timeoutSeconds); tapParser.handleTimeout(this.timeoutSeconds);
// Ensure entire process tree is killed if still running
try {
await execResultStreaming.kill(); // This kills the entire process tree with SIGKILL
} catch (killError) {
// Process tree might already be dead
}
} }
} else { } else {
await tapParser.handleTapProcess(execResultStreaming.childProcess); await tapParser.handleTapProcess(execResultStreaming.childProcess);
@ -296,6 +303,7 @@ export class TsTest {
); );
// Handle timeout if specified // Handle timeout if specified
let hasTimedOut = false;
if (this.timeoutSeconds !== null) { if (this.timeoutSeconds !== null) {
const timeoutMs = this.timeoutSeconds * 1000; const timeoutMs = this.timeoutSeconds * 1000;
let timeoutId: NodeJS.Timeout; let timeoutId: NodeJS.Timeout;
@ -315,19 +323,36 @@ export class TsTest {
clearTimeout(timeoutId); clearTimeout(timeoutId);
} catch (error) { } catch (error) {
// Handle timeout error // Handle timeout error
hasTimedOut = true;
tapParser.handleTimeout(this.timeoutSeconds); tapParser.handleTimeout(this.timeoutSeconds);
} }
} else { } else {
await evaluatePromise; await evaluatePromise;
} }
await this.smartbrowserInstance.stop(); // Always clean up resources, even on timeout
await server.stop(); try {
wss.close(); await this.smartbrowserInstance.stop();
} catch (error) {
// Browser might already be stopped
}
try {
await server.stop();
} catch (error) {
// Server might already be stopped
}
try {
wss.close();
} catch (error) {
// WebSocket server might already be closed
}
console.log( console.log(
`${cs('=> ', 'blue')} Stopped ${cs(fileNameArg, 'orange')} chromium instance and server.` `${cs('=> ', 'blue')} Stopped ${cs(fileNameArg, 'orange')} chromium instance and server.`
); );
// lets create the tap parser // Always evaluate final result (handleTimeout just sets up the test state)
await tapParser.evaluateFinalResult(); await tapParser.evaluateFinalResult();
return tapParser; return tapParser;
} }
@ -351,7 +376,7 @@ export class TsTest {
// Get all .log files in log directory (not in subdirectories) // Get all .log files in log directory (not in subdirectories)
const files = await plugins.smartfile.fs.listFileTree(logDir, '*.log'); const files = await plugins.smartfile.fs.listFileTree(logDir, '*.log');
const logFiles = files.filter(file => !file.includes('/')); const logFiles = files.filter((file: string) => !file.includes('/'));
if (logFiles.length === 0) { if (logFiles.length === 0) {
return; return;