Skip to content

Log temp-dir contents when test teardown fails to delete it #368

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
chrisdp wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Log temp-dir contents when test teardown fails to delete it #368

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/managers/BreakpointManager.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ let n = fileUtils.standardizePath.bind(fileUtils);
import type { SourceLocation } from '../managers/LocationManager';
import { LocationManager } from '../managers/LocationManager';
import { SourceMapManager } from './SourceMapManager';
import { expectPickEquals, pickArray } from '../testHelpers.spec';
import { expectPickEquals, pickArray, removeTempDir } from '../testHelpers.spec';
import { createSandbox } from 'sinon';
const sinon = createSandbox();

Expand Down Expand Up @@ -103,7 +103,7 @@ describe('BreakpointManager', () => {

afterEach(() => {
sinon.restore();
fsExtra.removeSync(tmpDir);
removeTempDir(tmpDir, 'BreakpointManager');
});

describe('reset', () => {
Expand Down Expand Up @@ -415,7 +415,7 @@ describe('BreakpointManager', () => {
});

afterEach(() => {
fsExtra.removeSync(tmpDir);
removeTempDir(tmpDir, 'BreakpointManager');
});

it('works with normal flow', async () => {
Expand Down
5 changes: 3 additions & 2 deletions src/managers/LocationManager.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { SourceMapConsumer, SourceNode } from 'source-map';
import { standardizePath as s } from '../FileUtils';
import { LocationManager } from './LocationManager';
import { SourceMapManager } from './SourceMapManager';
import { removeTempDir } from '../testHelpers.spec';

let tempDir = s`${process.cwd()}/.tmp`;
const rootDir = s`${tempDir}/rootDir`;
Expand All @@ -20,15 +21,15 @@ describe('LocationManager', () => {
beforeEach(() => {
sourceMapManager = new SourceMapManager();
locationManager = new LocationManager(sourceMapManager);
fsExtra.removeSync(tempDir);
removeTempDir(tempDir, 'LocationManager');
fsExtra.ensureDirSync(`${rootDir}/source`);
fsExtra.ensureDirSync(`${stagingDir}/source`);
for (let sourceDir of sourceDirs) {
fsExtra.ensureDirSync(`${sourceDir}/source`);
}
});
afterEach(() => {
fsExtra.removeSync(tempDir);
removeTempDir(tempDir, 'LocationManager');
});
describe('getSourceLocation', () => {

Expand Down
29 changes: 15 additions & 14 deletions src/managers/ProjectManager.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { BreakpointManager } from './BreakpointManager';
import { SourceMapManager } from './SourceMapManager';
import { LocationManager } from './LocationManager';
import * as decompress from 'decompress';
import { removeTempDir, emptyTempDir } from '../testHelpers.spec';

let sinon = sinonActual.createSandbox();
let n = fileUtils.standardizePath.bind(fileUtils);
Expand All @@ -25,12 +26,12 @@ let compLibstagingDir = s`${rootDir}/component-libraries/CompLibA`;

beforeEach(() => {
fsExtra.ensureDirSync(tempPath);
fsExtra.emptyDirSync(tempPath);
emptyTempDir(tempPath, 'ProjectManager');
sinon.restore();
});
afterEach(() => {
fsExtra.ensureDirSync(tempPath);
fsExtra.emptyDirSync(tempPath);
emptyTempDir(tempPath, 'ProjectManager');
});

describe('ProjectManager', () => {
Expand Down Expand Up @@ -432,7 +433,7 @@ describe('Project', () => {
describe('stage', () => {
afterEach(() => {
try {
fsExtra.removeSync(tempPath);
removeTempDir(tempPath, 'ProjectManager');
} catch (e) { }
});
it('actually stages the project', async () => {
Expand Down Expand Up @@ -542,7 +543,7 @@ describe('Project', () => {
describe('preprocessStagingFiles', () => {
afterEach(() => {
try {
fsExtra.removeSync(tempPath);
removeTempDir(tempPath, 'ProjectManager');
} catch (e) { }
});

Expand Down Expand Up @@ -1324,7 +1325,7 @@ describe('Project', () => {
describe('scriptReferencedFiles', () => {
afterEach(() => {
try {
fsExtra.removeSync(tempPath);
removeTempDir(tempPath, 'ProjectManager');
} catch (e) { }
});

Expand Down Expand Up @@ -1488,16 +1489,16 @@ describe('Project', () => {
fsExtra.writeFileSync(raleTrackerTaskFileLocation, `<!--dummy contents-->`);
});
after(() => {
fsExtra.removeSync(tempPath);
removeTempDir(tempPath, 'ProjectManager');
fsExtra.removeSync(raleTrackerTaskFileLocation);
});
afterEach(() => {
fsExtra.emptyDirSync(tempPath);
emptyTempDir(tempPath, 'ProjectManager');
fsExtra.rmdirSync(tempPath);
});

async function doTest(fileContents: string, expectedContents: string, fileExt = 'brs') {
fsExtra.emptyDirSync(tempPath);
emptyTempDir(tempPath, 'ProjectManager');
let folder = s`${tempPath}/findMainFunctionTests/`;
fsExtra.mkdirSync(folder);

Expand Down Expand Up @@ -1600,17 +1601,17 @@ describe('Project', () => {
fsExtra.writeFileSync(componentsFilePath, `' ${componentsFilePath}`);
});
after(() => {
fsExtra.removeSync(tempPath);
removeTempDir(tempPath, 'ProjectManager');
fsExtra.emptyDirSync(rdbFilesBasePath);
fsExtra.rmdirSync(rdbFilesBasePath);
});
afterEach(() => {
fsExtra.emptyDirSync(tempPath);
emptyTempDir(tempPath, 'ProjectManager');
fsExtra.rmdirSync(tempPath);
});

async function doTest(fileContents: string, expectedContents: string, fileExt = 'brs', injectRdbOnDeviceComponent = true) {
fsExtra.emptyDirSync(tempPath);
emptyTempDir(tempPath, 'ProjectManager');
let folder = s`${tempPath}/findMainFunctionTests/`;
fsExtra.mkdirSync(folder);

Expand All @@ -1636,7 +1637,7 @@ describe('Project', () => {
//which fast-glob treats as escape characters. Without normalization, the glob
//matches nothing and no files get copied.
it('copies the RDB files when rdbFilesBasePath is an absolute path with native separators', async () => {
fsExtra.emptyDirSync(tempPath);
emptyTempDir(tempPath, 'ProjectManager');
let folder = s`${tempPath}/copyAndTransformRDBTests/`;
fsExtra.mkdirSync(folder);
let filePath = s`${folder}/main.brs`;
Expand All @@ -1658,7 +1659,7 @@ describe('Project', () => {
});

it('does not copy files when injectRdbOnDeviceComponent is false', async () => {
fsExtra.emptyDirSync(tempPath);
emptyTempDir(tempPath, 'ProjectManager');
let folder = s`${tempPath}/copyAndTransformRDBTests/`;
fsExtra.mkdirSync(folder);
let filePath = s`${folder}/main.brs`;
Expand All @@ -1680,7 +1681,7 @@ describe('Project', () => {
});

it('does not copy files when rdbFilesBasePath is not set', async () => {
fsExtra.emptyDirSync(tempPath);
emptyTempDir(tempPath, 'ProjectManager');
let folder = s`${tempPath}/copyAndTransformRDBTests/`;
fsExtra.mkdirSync(folder);
let filePath = s`${folder}/main.brs`;
Expand Down
3 changes: 2 additions & 1 deletion src/managers/SourceMapManager.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import * as fsExtra from 'fs-extra';
import * as path from 'path';
import { standardizePath as s } from '../FileUtils';
import { SourceMapManager } from './SourceMapManager';
import { removeTempDir } from '../testHelpers.spec';
let tmpPath = s`${process.cwd()}/.tmp`;
describe('SourceMapManager', () => {
let manager: SourceMapManager;
Expand All @@ -13,7 +14,7 @@ describe('SourceMapManager', () => {
manager = new SourceMapManager();
});
afterEach(() => {
fsExtra.removeSync(tmpPath);
removeTempDir(tmpPath, 'SourceMapManager');
});

it('constructs', () => {
Expand Down
65 changes: 63 additions & 2 deletions src/testHelpers.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,73 @@ export const tempDir = s`${__dirname}/../.tmp`;
export const rootDir = s`${tempDir}/rootDir`;
export const stagingDir = s`${tempDir}/stagingDir`;

/**
* List every path remaining inside `dir` (recursively).
*/
function listTempLeftovers(dir: string): string[] {
const result: string[] = [];
const walk = (current: string) => {
let entries: string[];
try {
entries = fsExtra.readdirSync(current);
} catch {
return;
}
for (const entry of entries) {
const fullPath = `${current}/${entry}`;
result.push(fullPath);
try {
if (fsExtra.statSync(fullPath).isDirectory()) {
walk(fullPath);
}
} catch {
//entry may have been removed concurrently; ignore
}
}
};
walk(dir);
return result.sort();
}

/**
* Run a temp-dir cleanup operation. If it throws (most often Windows `ENOTEMPTY`, which happens when an
* earlier test left a file or handle open in the shared temp dir), log exactly what is still present so CI
* tells us which leftovers locked the dir, then re-throw. This diagnoses the failure; it does not hide it.
*/
function cleanupTempDir(label: string, dir: string, action: () => void) {
try {
action();
} catch (error) {
const leftovers = listTempLeftovers(dir);
console.error(`[temp-teardown] ${label}: ${(error as Error).message}`);
console.error(`[temp-teardown] ${label}: ${leftovers.length} path(s) still present under ${dir}:`);
for (const leftover of leftovers) {
console.error(`[temp-teardown] ${leftover}`);
}
throw error;
}
}

/**
* `fsExtra.removeSync` for a temp dir, with diagnostics logged if the removal fails.
*/
export function removeTempDir(dir: string, label: string) {
cleanupTempDir(label, dir, () => fsExtra.removeSync(dir));
}

/**
* `fsExtra.emptyDirSync` for a temp dir, with diagnostics logged if the empty fails.
*/
export function emptyTempDir(dir: string, label: string) {
cleanupTempDir(label, dir, () => fsExtra.emptyDirSync(dir));
}

beforeEach(() => {
fsExtra.emptyDirSync(tempDir);
emptyTempDir(tempDir, 'global beforeEach');
});

afterEach(() => {
fsExtra.removeSync(tempDir);
removeTempDir(tempDir, 'global afterEach');
});

/**
Expand Down
Loading