Skip to content

Commit

Permalink
mocha runner: better logs around test cleanup (#4993)
Browse files Browse the repository at this point in the history
### Intent
When running a large number of tests in parallel, the script cleans up
the test data generated during the run, which can sometimes take a
while. To save time, there’s an option to skip this step, commonly used
in CICD pipelines. This option was already available, but the recent
update improves its visibility to make it clearer to the person running
the tests.
Note: This does not delete traces, but rather temporary files created by
the runner.

### Approach
Enhanced the logging to provide better visibility, highlighting when the
cleanup is being skipped to reduce confusion and improve monitoring.

#### Without skipping clean up
<img width="583" alt="Screenshot 2024-10-10 at 2 20 32 PM"
src="https://github.com/user-attachments/assets/5522e993-1f6d-4e16-8ec1-0d18ad654e76">

#### With skipping clean up  (`--skip-cleanup`)
<img width="220" alt="Screenshot 2024-10-10 at 2 20 02 PM"
src="https://github.com/user-attachments/assets/5b909df4-535f-433b-9c3a-390953fe7a59">
  • Loading branch information
midleman authored Oct 10, 2024
1 parent e8329dd commit 87fc7fd
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 30 deletions.
4 changes: 2 additions & 2 deletions test/smoke/run-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ const minimist = require('minimist');
const { prepareTestEnv, cloneTestRepo, runMochaTests } = require('./out/test-runner');
const OPTS = minimist(process.argv.slice(2));

(function main() {
(async function main() {
prepareTestEnv();
cloneTestRepo();
runMochaTests(OPTS);
await runMochaTests(OPTS);
})();
54 changes: 26 additions & 28 deletions test/smoke/src/test-runner/mocha-runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
*--------------------------------------------------------------------------------------------*/

import * as path from 'path';
import * as fs from 'fs';
const rimraf = require('rimraf');
// eslint-disable-next-line local/code-import-patterns
import * as fs from 'fs/promises';
const Mocha = require('mocha');

const TEST_DATA_PATH = process.env.TEST_DATA_PATH || 'TEST_DATA_PATH not set';
Expand All @@ -14,7 +14,7 @@ const REPORT_PATH = process.env.REPORT_PATH || 'REPORT_PATH not set';
/**
* Runs Mocha tests.
*/
export function runMochaTests(OPTS) {
export async function runMochaTests(OPTS: any) {
const mocha = new Mocha({
color: true,
timeout: 1 * 60 * 1000, // 1 minute
Expand All @@ -34,23 +34,19 @@ export function runMochaTests(OPTS) {
applyTestFilters(mocha);

// Add test files to the Mocha runner
const testFiles = findTestFilesRecursive(path.resolve('out/areas/positron'));
const testFiles = await findTestFilesRecursive(path.resolve('out/areas/positron'));
testFiles.forEach(file => mocha.addFile(file));

// Run the Mocha tests
const runner = mocha.run(failures => {
const runner = mocha.run(async failures => {
if (failures) {
console.log(getFailureLogs());
} else {
console.log('All tests passed.');
}
cleanupTestData(err => {
if (err) {
console.log('Error cleaning up test data:', err);
} else {
process.exit(failures ? 1 : 0);
}
});

await cleanupTestData();
process.exit(failures ? 1 : 0);
});

// Attach the 'retry' event listener to the runner
Expand Down Expand Up @@ -86,39 +82,41 @@ function applyTestFilters(mocha: Mocha): void {
/**
* Recursively finds all test files in child directories.
*/
function findTestFilesRecursive(dirPath: string): string[] {
async function findTestFilesRecursive(dirPath: string): Promise<string[]> {
let testFiles: string[] = [];
const entries = fs.readdirSync(dirPath, { withFileTypes: true });

entries.forEach(entry => {
const entries = await fs.readdir(dirPath, { withFileTypes: true });

for (const entry of entries) {
const fullPath = path.join(dirPath, entry.name);
if (entry.isDirectory()) {
// If it's a directory, recursively search within it
testFiles = testFiles.concat(findTestFilesRecursive(fullPath));
const subDirFiles = await findTestFilesRecursive(fullPath);
testFiles = testFiles.concat(subDirFiles);
} else if (entry.isFile() && entry.name.endsWith('.js') && !entry.name.startsWith('example.test.js')) {
// If it's a file, add it if it matches the criteria
testFiles.push(fullPath);
}
});
}

return testFiles;
}

/**
* Cleans up the test data directory.
*/
function cleanupTestData(callback: (error: Error | null) => void) {
async function cleanupTestData(): Promise<void> {
if (process.env.SKIP_CLEANUP) {
callback(null);
} else {
rimraf(TEST_DATA_PATH, { maxBusyTries: 10 }, error => {
if (error) {
console.error('Error cleaning up test data:', error);
return callback(error);
}
console.log('Test data cleaned up successfully.');
callback(null);
});
console.log('Skipping test data cleanup.');
return;
}

try {
console.log('Cleaning up test data directory. FYI: This can be bypassed with --skip-cleanup');
await fs.rm(TEST_DATA_PATH, { recursive: true, force: true });
console.log('Cleanup completed successfully.');
} catch (error) {
console.error(`Error cleaning up test data: ${error}`);
}
}

Expand Down

0 comments on commit 87fc7fd

Please sign in to comment.