fix(tstest): Improve timeout warning timer management and summary output formatting in the test runner.
This commit is contained in:
		| @@ -1,5 +1,13 @@ | ||||
| # Changelog | ||||
|  | ||||
| ## 2025-05-26 - 2.2.6 - fix(tstest) | ||||
| Improve timeout warning timer management and summary output formatting in the test runner. | ||||
|  | ||||
| - Removed the global timeoutWarningTimer and replaced it with local warning timers in runInNode and runInChrome methods. | ||||
| - Added warnings when test files run for over one minute if no timeout is specified. | ||||
| - Ensured proper clearing of warning timers on successful completion or timeout. | ||||
| - Enhanced quiet mode summary output to clearly display passed and failed test counts. | ||||
|  | ||||
| ## 2025-05-26 - 2.2.5 - fix(protocol) | ||||
| Fix inline timing metadata parsing and enhance test coverage for performance metrics and timing edge cases | ||||
|  | ||||
|   | ||||
| @@ -3,6 +3,6 @@ | ||||
|  */ | ||||
| export const commitinfo = { | ||||
|   name: '@git.zone/tstest', | ||||
|   version: '2.2.5', | ||||
|   version: '2.2.6', | ||||
|   description: 'a test utility to run tests that match test/**/*.ts' | ||||
| } | ||||
|   | ||||
| @@ -18,7 +18,6 @@ export class TsTest { | ||||
|   public startFromFile: number | null; | ||||
|   public stopAtFile: number | null; | ||||
|   public timeoutSeconds: number | null; | ||||
|   private timeoutWarningTimer: NodeJS.Timeout | null = null; | ||||
|  | ||||
|   public smartshellInstance = new plugins.smartshell.Smartshell({ | ||||
|     executor: 'bash', | ||||
| @@ -45,15 +44,6 @@ export class TsTest { | ||||
|       await this.movePreviousLogFiles(); | ||||
|     } | ||||
|      | ||||
|     // Start timeout warning timer if no timeout was specified | ||||
|     if (this.timeoutSeconds === null) { | ||||
|       this.timeoutWarningTimer = setTimeout(() => { | ||||
|         this.logger.warning('Test is running for more than 1 minute.'); | ||||
|         this.logger.warning('Consider using --timeout option to set a timeout for test files.'); | ||||
|         this.logger.warning('Example: tstest test --timeout=300 (for 5 minutes)'); | ||||
|       }, 60000); // 1 minute | ||||
|     } | ||||
|      | ||||
|     const testGroups = await this.testDir.getTestFileGroups(); | ||||
|     const allFiles = [...testGroups.serial, ...Object.values(testGroups.parallelGroups).flat()]; | ||||
|      | ||||
| @@ -92,12 +82,6 @@ export class TsTest { | ||||
|       } | ||||
|     } | ||||
|      | ||||
|     // Clear the timeout warning timer if it was set | ||||
|     if (this.timeoutWarningTimer) { | ||||
|       clearTimeout(this.timeoutWarningTimer); | ||||
|       this.timeoutWarningTimer = null; | ||||
|     } | ||||
|      | ||||
|     tapCombinator.evaluate(); | ||||
|   } | ||||
|    | ||||
| @@ -272,6 +256,19 @@ import '${absoluteTestFile.replace(/\\/g, '/')}'; | ||||
|       execResultStreaming.childProcess.on('error', cleanup); | ||||
|     } | ||||
|      | ||||
|     // Start warning timer if no timeout was specified | ||||
|     let warningTimer: NodeJS.Timeout | null = null; | ||||
|     if (this.timeoutSeconds === null) { | ||||
|       warningTimer = setTimeout(() => { | ||||
|         console.error(''); | ||||
|         console.error(cs('⚠️  WARNING: Test file is running for more than 1 minute', 'orange')); | ||||
|         console.error(cs(`   File: ${fileNameArg}`, 'orange')); | ||||
|         console.error(cs('   Consider using --timeout option to set a timeout for test files.', 'orange')); | ||||
|         console.error(cs('   Example: tstest test --timeout=300 (for 5 minutes)', 'orange')); | ||||
|         console.error(''); | ||||
|       }, 60000); // 1 minute | ||||
|     } | ||||
|      | ||||
|     // Handle timeout if specified | ||||
|     if (this.timeoutSeconds !== null) { | ||||
|       const timeoutMs = this.timeoutSeconds * 1000; | ||||
| @@ -293,6 +290,10 @@ import '${absoluteTestFile.replace(/\\/g, '/')}'; | ||||
|         // Clear timeout if test completed successfully | ||||
|         clearTimeout(timeoutId); | ||||
|       } catch (error) { | ||||
|         // Clear warning timer if it was set | ||||
|         if (warningTimer) { | ||||
|           clearTimeout(warningTimer); | ||||
|         } | ||||
|         // Handle timeout error | ||||
|         tapParser.handleTimeout(this.timeoutSeconds); | ||||
|         // Ensure entire process tree is killed if still running | ||||
| @@ -307,6 +308,11 @@ import '${absoluteTestFile.replace(/\\/g, '/')}'; | ||||
|       await tapParser.handleTapProcess(execResultStreaming.childProcess); | ||||
|     } | ||||
|      | ||||
|     // Clear warning timer if it was set | ||||
|     if (warningTimer) { | ||||
|       clearTimeout(warningTimer); | ||||
|     } | ||||
|      | ||||
|     return tapParser; | ||||
|   } | ||||
|  | ||||
| @@ -425,6 +431,19 @@ import '${absoluteTestFile.replace(/\\/g, '/')}'; | ||||
|       } | ||||
|     ); | ||||
|      | ||||
|     // Start warning timer if no timeout was specified | ||||
|     let warningTimer: NodeJS.Timeout | null = null; | ||||
|     if (this.timeoutSeconds === null) { | ||||
|       warningTimer = setTimeout(() => { | ||||
|         console.error(''); | ||||
|         console.error(cs('⚠️  WARNING: Test file is running for more than 1 minute', 'orange')); | ||||
|         console.error(cs(`   File: ${fileNameArg}`, 'orange')); | ||||
|         console.error(cs('   Consider using --timeout option to set a timeout for test files.', 'orange')); | ||||
|         console.error(cs('   Example: tstest test --timeout=300 (for 5 minutes)', 'orange')); | ||||
|         console.error(''); | ||||
|       }, 60000); // 1 minute | ||||
|     } | ||||
|      | ||||
|     // Handle timeout if specified | ||||
|     if (this.timeoutSeconds !== null) { | ||||
|       const timeoutMs = this.timeoutSeconds * 1000; | ||||
| @@ -444,6 +463,10 @@ import '${absoluteTestFile.replace(/\\/g, '/')}'; | ||||
|         // Clear timeout if test completed successfully | ||||
|         clearTimeout(timeoutId); | ||||
|       } catch (error) { | ||||
|         // Clear warning timer if it was set | ||||
|         if (warningTimer) { | ||||
|           clearTimeout(warningTimer); | ||||
|         } | ||||
|         // Handle timeout error | ||||
|         tapParser.handleTimeout(this.timeoutSeconds); | ||||
|       } | ||||
| @@ -451,6 +474,11 @@ import '${absoluteTestFile.replace(/\\/g, '/')}'; | ||||
|       await evaluatePromise; | ||||
|     } | ||||
|      | ||||
|     // Clear warning timer if it was set | ||||
|     if (warningTimer) { | ||||
|       clearTimeout(warningTimer); | ||||
|     } | ||||
|      | ||||
|     // Always clean up resources, even on timeout | ||||
|     try { | ||||
|       await this.smartbrowserInstance.stop(); | ||||
| @@ -488,10 +516,10 @@ import '${absoluteTestFile.replace(/\\/g, '/')}'; | ||||
|      | ||||
|     try { | ||||
|       // Delete 00err and 00diff directories if they exist | ||||
|       if (await plugins.smartfile.fs.isDirectory(errDir)) { | ||||
|       if (plugins.smartfile.fs.isDirectorySync(errDir)) { | ||||
|         plugins.smartfile.fs.removeSync(errDir); | ||||
|       } | ||||
|       if (await plugins.smartfile.fs.isDirectory(diffDir)) { | ||||
|       if (plugins.smartfile.fs.isDirectorySync(diffDir)) { | ||||
|         plugins.smartfile.fs.removeSync(diffDir); | ||||
|       } | ||||
|        | ||||
|   | ||||
| @@ -242,9 +242,11 @@ export class TsTestLogger { | ||||
|      | ||||
|     if (!this.options.quiet) { | ||||
|       const total = passed + failed; | ||||
|       const status = failed === 0 ? 'PASSED' : 'FAILED'; | ||||
|       const color = failed === 0 ? 'green' : 'red'; | ||||
|       this.log(this.format(`   Summary: ${passed}/${total} ${status}`, color)); | ||||
|       if (failed === 0) { | ||||
|         this.log(this.format(`   Summary: ${passed}/${total} PASSED`, 'green')); | ||||
|       } else { | ||||
|         this.log(this.format(`   Summary: ${passed} passed, ${failed} failed of ${total} tests`, 'red')); | ||||
|       } | ||||
|     } | ||||
|      | ||||
|     // If using --logfile, handle error copy and diff detection | ||||
| @@ -390,7 +392,11 @@ export class TsTestLogger { | ||||
|      | ||||
|     if (this.options.quiet) { | ||||
|       const status = summary.totalFailed === 0 ? 'PASSED' : 'FAILED'; | ||||
|       this.log(`\nSummary: ${summary.totalPassed}/${summary.totalTests} | ${totalDuration}ms | ${status}`); | ||||
|       if (summary.totalFailed === 0) { | ||||
|         this.log(`\nSummary: ${summary.totalPassed}/${summary.totalTests} | ${totalDuration}ms | ${status}`); | ||||
|       } else { | ||||
|         this.log(`\nSummary: ${summary.totalPassed} passed, ${summary.totalFailed} failed of ${summary.totalTests} tests | ${totalDuration}ms | ${status}`); | ||||
|       } | ||||
|       return; | ||||
|     } | ||||
|      | ||||
|   | ||||
		Reference in New Issue
	
	Block a user