Skip to content

Commit

Permalink
Reduce file system calls during test setup. (#56791)
Browse files Browse the repository at this point in the history
  • Loading branch information
dragomirtitian authored Jan 11, 2024
1 parent e769725 commit 9a0c7b1
Show file tree
Hide file tree
Showing 8 changed files with 53 additions and 62 deletions.
2 changes: 2 additions & 0 deletions src/compiler/sys.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1445,6 +1445,7 @@ export interface System {
realpath?(path: string): string;
/** @internal */ getEnvironmentVariable(name: string): string;
/** @internal */ tryEnableSourceMapsForHost?(): void;
/** @internal */ getAccessibleFileSystemEntries?(path: string): FileSystemEntries;
/** @internal */ debugMode?: boolean;
setTimeout?(callback: (...args: any[]) => void, ms: number, ...args: any[]): any;
clearTimeout?(timeoutId: any): void;
Expand Down Expand Up @@ -1553,6 +1554,7 @@ export let sys: System = (() => {
resolvePath: path => _path.resolve(path),
fileExists,
directoryExists,
getAccessibleFileSystemEntries,
createDirectory(directoryName: string) {
if (!nodeSystem.directoryExists(directoryName)) {
// Wrapped in a try-catch to prevent crashing if we are in a race
Expand Down
51 changes: 12 additions & 39 deletions src/harness/harnessIO.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export interface IO {
fileExists(fileName: string): boolean;
directoryExists(path: string): boolean;
deleteFile(fileName: string): void;
enumerateTestFiles(runner: RunnerBase): (string | FileBasedTest)[];
enumerateTestFiles(runner: RunnerBase): string[];
listFiles(path: string, filter?: RegExp, options?: { recursive?: boolean; }): string[];
log(text: string): void;
args(): string[];
Expand Down Expand Up @@ -84,55 +84,28 @@ function createNodeIO(): IO {
return runner.getTestFiles();
}

function listFiles(path: string, spec: RegExp, options: { recursive?: boolean; } = {}) {
function listFiles(path: string, spec: RegExp | undefined, options: { recursive?: boolean; } = {}) {
function filesInFolder(folder: string): string[] {
const { files, directories } = ts.sys.getAccessibleFileSystemEntries!(folder);
let paths: string[] = [];

for (const file of fs.readdirSync(folder)) {
for (const file of files) {
const pathToFile = pathModule.join(folder, file);
if (!fs.existsSync(pathToFile)) continue; // ignore invalid symlinks
const stat = fs.statSync(pathToFile);
if (options.recursive && stat.isDirectory()) {
paths = paths.concat(filesInFolder(pathToFile));
}
else if (stat.isFile() && (!spec || file.match(spec))) {
if (!spec || file.match(spec)) {
paths.push(pathToFile);
}
}

if (options.recursive) {
for (const dir of directories) {
const pathToDir = pathModule.join(folder, dir);
paths = paths.concat(filesInFolder(pathToDir));
}
}
return paths;
}

return filesInFolder(path);
}

function getAccessibleFileSystemEntries(dirname: string): ts.FileSystemEntries {
try {
const entries: string[] = fs.readdirSync(dirname || ".").sort(ts.sys.useCaseSensitiveFileNames ? ts.compareStringsCaseSensitive : ts.compareStringsCaseInsensitive);
const files: string[] = [];
const directories: string[] = [];
for (const entry of entries) {
if (entry === "." || entry === "..") continue;
const name = vpath.combine(dirname, entry);
try {
const stat = fs.statSync(name);
if (!stat) continue;
if (stat.isFile()) {
files.push(entry);
}
else if (stat.isDirectory()) {
directories.push(entry);
}
}
catch { /*ignore*/ }
}
return { files, directories };
}
catch (e) {
return { files: [], directories: [] };
}
}

function createDirectory(path: string) {
try {
fs.mkdirSync(path);
Expand Down Expand Up @@ -170,7 +143,7 @@ function createNodeIO(): IO {
getWorkspaceRoot: () => workspaceRoot,
exit: exitCode => ts.sys.exit(exitCode),
readDirectory: (path, extension, exclude, include, depth) => ts.sys.readDirectory(path, extension, exclude, include, depth),
getAccessibleFileSystemEntries,
getAccessibleFileSystemEntries: ts.sys.getAccessibleFileSystemEntries!,
tryEnableSourceMapsForHost: () => ts.sys.tryEnableSourceMapsForHost && ts.sys.tryEnableSourceMapsForHost(),
getMemoryUsage: () => ts.sys.getMemoryUsage && ts.sys.getMemoryUsage(),
getEnvironmentVariable: name => ts.sys.getEnvironmentVariable(name),
Expand Down
5 changes: 2 additions & 3 deletions src/harness/runnerbase.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import {
FileBasedTest,
IO,
userSpecifiedRoot,
} from "./_namespaces/Harness";
Expand All @@ -22,7 +21,7 @@ export function setShardId(id: number) {

export abstract class RunnerBase {
// contains the tests to run
public tests: (string | FileBasedTest)[] = [];
public tests: string[] = [];

/** Add a source file to the runner's list of tests that need to be initialized with initializeTests */
public addTest(fileName: string) {
Expand All @@ -35,7 +34,7 @@ export abstract class RunnerBase {

abstract kind(): TestRunnerKind;

abstract enumerateTestFiles(): (string | FileBasedTest)[];
abstract enumerateTestFiles(): string[];

getTestFiles(): ReturnType<this["enumerateTestFiles"]> {
const all = this.enumerateTestFiles();
Expand Down
8 changes: 4 additions & 4 deletions src/testRunner/compilerRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,10 @@ export class CompilerBaselineRunner extends RunnerBase {
return this.testSuiteName;
}

private testFiles: string[] | undefined;
public enumerateTestFiles() {
// see also: `enumerateTestFiles` in tests/webTestServer.ts
return this.enumerateFiles(this.basePath, /\.tsx?$/, { recursive: true }).map(CompilerTest.getConfigurations);
return this.testFiles ??= this.enumerateFiles(this.basePath, /\.tsx?$/, { recursive: true });
}

public initializeTests() {
Expand All @@ -64,9 +65,8 @@ export class CompilerBaselineRunner extends RunnerBase {

// this will set up a series of describe/it blocks to run between the setup and cleanup phases
const files = this.tests.length > 0 ? this.tests : IO.enumerateTestFiles(this);
files.forEach(test => {
const file = typeof test === "string" ? test : test.file;
this.checkTestCodeOutput(vpath.normalizeSeparators(file), typeof test === "string" ? CompilerTest.getConfigurations(test) : test);
files.forEach(file => {
this.checkTestCodeOutput(vpath.normalizeSeparators(file), CompilerTest.getConfigurations(file));
});
});
}
Expand Down
3 changes: 1 addition & 2 deletions src/testRunner/fourslashRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,7 @@ export class FourSlashRunner extends RunnerBase {
}

describe(this.testSuiteName + " tests", () => {
this.tests.forEach(test => {
const file = typeof test === "string" ? test : test.file;
this.tests.forEach(file => {
describe(file, () => {
let fn = ts.normalizeSlashes(file);
const justName = fn.replace(/^.*[\\/]/, "");
Expand Down
3 changes: 1 addition & 2 deletions src/testRunner/parallel/host.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,8 +217,7 @@ export function start() {
console.log("Discovering runner-based tests...");
const discoverStart = +(new Date());
for (const runner of runners) {
for (const test of runner.getTestFiles()) {
const file = typeof test === "string" ? test : test.file;
for (const file of runner.getTestFiles()) {
let size: number;
if (!perfData) {
try {
Expand Down
39 changes: 30 additions & 9 deletions src/testRunner/projectsRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export class ProjectRunner extends Harness.RunnerBase {
describe("projects tests", () => {
const tests = this.tests.length === 0 ? this.enumerateTestFiles() : this.tests;
for (const test of tests) {
this.runProjectTestCase(typeof test === "string" ? test : test.file);
this.runProjectTestCase(test);
}
});
}
Expand Down Expand Up @@ -202,15 +202,36 @@ class ProjectTestCase {
throw assert(false, "Testcase: " + testCaseFileName + " does not contain valid json format: " + e.message);
}

const fs = vfs.createFromFileSystem(Harness.IO, /*ignoreCase*/ false);
fs.mountSync(vpath.resolve(Harness.IO.getWorkspaceRoot(), "tests"), vpath.combine(vfs.srcFolder, "tests"), vfs.createResolver(Harness.IO));
fs.mkdirpSync(vpath.combine(vfs.srcFolder, testCase.projectRoot));
fs.chdir(vpath.combine(vfs.srcFolder, testCase.projectRoot));
fs.makeReadonly();

function makeFileSystem() {
const fs = vfs.createFromFileSystem(Harness.IO, /*ignoreCase*/ false);
fs.mountSync(vpath.resolve(Harness.IO.getWorkspaceRoot(), "tests"), vpath.combine(vfs.srcFolder, "tests"), vfs.createResolver(Harness.IO));
fs.mkdirpSync(vpath.combine(vfs.srcFolder, testCase.projectRoot));
fs.chdir(vpath.combine(vfs.srcFolder, testCase.projectRoot));
fs.makeReadonly();
return fs;
}
let fs: vfs.FileSystem | undefined;
return [
{ name: `@module: commonjs`, payload: { testCase, moduleKind: ts.ModuleKind.CommonJS, vfs: fs } },
{ name: `@module: amd`, payload: { testCase, moduleKind: ts.ModuleKind.AMD, vfs: fs } },
{
name: `@module: commonjs`,
payload: {
testCase,
moduleKind: ts.ModuleKind.CommonJS,
get vfs() {
return fs ??= makeFileSystem();
},
},
},
{
name: `@module: amd`,
payload: {
testCase,
moduleKind: ts.ModuleKind.AMD,
get vfs() {
return fs ??= makeFileSystem();
},
},
},
];
}

Expand Down
4 changes: 1 addition & 3 deletions src/testRunner/runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,7 @@ function runTests(runners: RunnerBase[]) {
const dupes: [string, string][] = [];
for (const runner of runners) {
if (runner instanceof CompilerBaselineRunner || runner instanceof FourSlashRunner) {
for (const sf of runner.enumerateTestFiles()) {
const full = typeof sf === "string" ? sf : sf.file;
for (const full of runner.enumerateTestFiles()) {
const base = vpath.basename(full).toLowerCase();
// allow existing dupes in fourslash/shims and fourslash/server
if (seen.has(base) && !/fourslash\/(shim|server)/.test(full)) {
Expand Down Expand Up @@ -191,7 +190,6 @@ function handleTestConfig() {
case "fourslash-generated":
runners.push(new GeneratedFourslashRunner(FourSlash.FourSlashTestType.Native));
break;
break;
}
}
}
Expand Down

0 comments on commit 9a0c7b1

Please sign in to comment.