Merge pull request microsoft#23972 from Microsoft/batchTestConfigurationsForBrowser

rbuckton · web-flow · commit 5af7e06f72c7 · 2018-05-09T14:15:42.000-07:00
Batch enumerateFiles into a single web request
diff --git a/src/harness/compilerRunner.ts b/src/harness/compilerRunner.ts
@@ -12,6 +12,10 @@ const enum CompilerTestType {
     Test262
 }
 
+interface CompilerFileBasedTest extends Harness.FileBasedTest {
+    payload?: Harness.TestCaseParser.TestCaseContent;
+}
+
 class CompilerBaselineRunner extends RunnerBase {
     private basePath = "tests/cases";
     private testSuiteName: TestRunnerKind;
@@ -42,7 +46,8 @@ class CompilerBaselineRunner extends RunnerBase {
     }
 
     public enumerateTestFiles() {
-        return this.enumerateFiles(this.basePath, /\.tsx?$/, { recursive: true });
+        // see also: `enumerateTestFiles` in tests/webTestServer.ts
+        return this.enumerateFiles(this.basePath, /\.tsx?$/, { recursive: true }).map(CompilerTest.getConfigurations);
     }
 
     public initializeTests() {
@@ -52,24 +57,32 @@ 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 : this.enumerateTestFiles();
-            files.forEach(file => { this.checkTestCodeOutput(vpath.normalizeSeparators(file)); });
+            const files = this.tests.length > 0 ? this.tests : Harness.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);
+            });
         });
     }
 
-    public checkTestCodeOutput(fileName: string) {
-        for (const { name, payload } of CompilerTest.getConfigurations(fileName)) {
-            describe(`${this.testSuiteName} tests for ${fileName}${name ? ` (${name})` : ``}`, () => {
-                this.runSuite(fileName, payload);
+    public checkTestCodeOutput(fileName: string, test?: CompilerFileBasedTest) {
+        if (test && test.configurations) {
+            test.configurations.forEach(configuration => {
+                describe(`${this.testSuiteName} tests for ${fileName}${configuration ? ` (${Harness.getFileBasedTestConfigurationDescription(configuration)})` : ``}`, () => {
+                    this.runSuite(fileName, test, configuration);
+                });
             });
         }
+        describe(`${this.testSuiteName} tests for ${fileName}}`, () => {
+            this.runSuite(fileName, test);
+        });
     }
 
-    private runSuite(fileName: string, testCaseContent: Harness.TestCaseParser.TestCaseContent) {
+    private runSuite(fileName: string, test?: CompilerFileBasedTest, configuration?: Harness.FileBasedTestConfiguration) {
         // Mocha holds onto the closure environment of the describe callback even after the test is done.
         // Everything declared here should be cleared out in the "after" callback.
         let compilerTest: CompilerTest | undefined;
-        before(() => { compilerTest = new CompilerTest(fileName, testCaseContent); });
+        before(() => { compilerTest = new CompilerTest(fileName, test && test.payload, configuration); });
         it(`Correct errors for ${fileName}`, () => { compilerTest.verifyDiagnostics(); });
         it(`Correct module resolution tracing for ${fileName}`, () => { compilerTest.verifyModuleResolution(); });
         it(`Correct sourcemap content for ${fileName}`, () => { compilerTest.verifySourceMapRecord(); });
@@ -97,11 +110,6 @@ class CompilerBaselineRunner extends RunnerBase {
     }
 }
 
-interface CompilerTestConfiguration {
-    name: string;
-    payload: Harness.TestCaseParser.TestCaseContent;
-}
-
 class CompilerTest {
     private fileName: string;
     private justName: string;
@@ -116,10 +124,20 @@ class CompilerTest {
     // equivalent to other files on the file system not directly passed to the compiler (ie things that are referenced by other files)
     private otherFiles: Harness.Compiler.TestFile[];
 
-    constructor(fileName: string, testCaseContent: Harness.TestCaseParser.TestCaseContent) {
+    constructor(fileName: string, testCaseContent?: Harness.TestCaseParser.TestCaseContent, configurationOverrides?: Harness.TestCaseParser.CompilerSettings) {
         this.fileName = fileName;
         this.justName = vpath.basename(fileName);
+
         const rootDir = fileName.indexOf("conformance") === -1 ? "tests/cases/compiler/" : ts.getDirectoryPath(fileName) + "/";
+
+        if (testCaseContent === undefined) {
+            testCaseContent = Harness.TestCaseParser.makeUnitsFromTest(Harness.IO.readFile(fileName), fileName, rootDir);
+        }
+
+        if (configurationOverrides) {
+            testCaseContent = { ...testCaseContent, settings: { ...testCaseContent.settings, ...configurationOverrides } };
+        }
+
         const units = testCaseContent.testUnitData;
         this.harnessSettings = testCaseContent.settings;
         let tsConfigOptions: ts.CompilerOptions;
@@ -174,32 +192,14 @@ class CompilerTest {
         this.options = this.result.options;
     }
 
-    public static getConfigurations(fileName: string) {
-        const content = Harness.IO.readFile(fileName);
-        const rootDir = fileName.indexOf("conformance") === -1 ? "tests/cases/compiler/" : ts.getDirectoryPath(fileName) + "/";
-        const testCaseContent = Harness.TestCaseParser.makeUnitsFromTest(content, fileName, rootDir);
-        const configurations: CompilerTestConfiguration[] = [];
-        const scriptTargets = this._split(testCaseContent.settings.target);
-        const moduleKinds = this._split(testCaseContent.settings.module);
-        for (const scriptTarget of scriptTargets) {
-            for (const moduleKind of moduleKinds) {
-                let name = "";
-                if (moduleKinds.length > 1) {
-                    name += `@module: ${moduleKind || "none"}`;
-                }
-                if (scriptTargets.length > 1) {
-                    if (name) name += ", ";
-                    name += `@target: ${scriptTarget || "none"}`;
-                }
-
-                const settings = { ...testCaseContent.settings };
-                if (scriptTarget) settings.target = scriptTarget;
-                if (moduleKind) settings.module = moduleKind;
-                configurations.push({ name, payload: { ...testCaseContent, settings } });
-            }
-        }
-
-        return configurations;
+    public static getConfigurations(file: string): CompilerFileBasedTest {
+        // also see `parseCompilerTestConfigurations` in tests/webTestServer.ts
+        const content = Harness.IO.readFile(file);
+        const rootDir = file.indexOf("conformance") === -1 ? "tests/cases/compiler/" : ts.getDirectoryPath(file) + "/";
+        const payload = Harness.TestCaseParser.makeUnitsFromTest(content, file, rootDir);
+        const settings = Harness.TestCaseParser.extractCompilerSettings(content);
+        const configurations = Harness.getFileBasedTestConfigurations(settings, /*varyBy*/ ["module", "target"]);
+        return { file, payload, configurations };
     }
 
     public verifyDiagnostics() {
@@ -267,11 +267,6 @@ class CompilerTest {
             this.toBeCompiled.concat(this.otherFiles).filter(file => !!this.result.program.getSourceFile(file.unitName)));
     }
 
-    private static _split(text: string) {
-        const entries = text && text.split(",").map(s => s.toLowerCase().trim()).filter(s => s.length > 0);
-        return entries && entries.length > 0 ? entries : [""];
-    }
-
     private makeUnitName(name: string, root: string) {
         const path = ts.toPath(name, root, ts.identity);
         const pathStart = ts.toPath(Harness.IO.getCurrentDirectory(), "", ts.identity);
diff --git a/src/harness/externalCompileRunner.ts b/src/harness/externalCompileRunner.ts
@@ -28,7 +28,7 @@ abstract class ExternalCompileRunnerBase extends RunnerBase {
 
         describe(`${this.kind()} code samples`, () => {
             for (const test of testList) {
-                this.runTest(test);
+                this.runTest(typeof test === "string" ? test : test.file);
             }
         });
     }
diff --git a/src/harness/fourslashRunner.ts b/src/harness/fourslashRunner.ts
@@ -36,6 +36,7 @@ class FourSlashRunner extends RunnerBase {
     }
 
     public enumerateTestFiles() {
+        // see also: `enumerateTestFiles` in tests/webTestServer.ts
         return this.enumerateFiles(this.basePath, /\.ts/i, { recursive: false });
     }
 
@@ -45,22 +46,23 @@ class FourSlashRunner extends RunnerBase {
 
     public initializeTests() {
         if (this.tests.length === 0) {
-            this.tests = this.enumerateTestFiles();
+            this.tests = Harness.IO.enumerateTestFiles(this);
         }
 
         describe(this.testSuiteName + " tests", () => {
-            this.tests.forEach((fn: string) => {
-                 describe(fn, () => {
-                       fn = ts.normalizeSlashes(fn);
-                        const justName = fn.replace(/^.*[\\\/]/, "");
+            this.tests.forEach(test => {
+                const file = typeof test === "string" ? test : test.file;
+                describe(file, () => {
+                    let fn = ts.normalizeSlashes(file);
+                    const justName = fn.replace(/^.*[\\\/]/, "");
 
-                        // Convert to relative path
-                        const testIndex = fn.indexOf("tests/");
-                        if (testIndex >= 0) fn = fn.substr(testIndex);
+                    // Convert to relative path
+                    const testIndex = fn.indexOf("tests/");
+                    if (testIndex >= 0) fn = fn.substr(testIndex);
 
-                        if (justName && !justName.match(/fourslash\.ts$/i) && !justName.match(/\.d\.ts$/i)) {
-                            it(this.testSuiteName + " test " + justName + " runs correctly", () => {
-                                FourSlash.runFourSlashTest(this.basePath, this.testType, fn);
+                    if (justName && !justName.match(/fourslash\.ts$/i) && !justName.match(/\.d\.ts$/i)) {
+                        it(this.testSuiteName + " test " + justName + " runs correctly", () => {
+                            FourSlash.runFourSlashTest(this.basePath, this.testType, fn);
                         });
                     }
                 });
diff --git a/src/harness/harness.ts b/src/harness/harness.ts
@@ -514,6 +514,7 @@ namespace Harness {
         fileExists(fileName: string): boolean;
         directoryExists(path: string): boolean;
         deleteFile(fileName: string): void;
+        enumerateTestFiles(runner: RunnerBase): (string | FileBasedTest)[];
         listFiles(path: string, filter?: RegExp, options?: { recursive?: boolean }): string[];
         log(text: string): void;
         args(): string[];
@@ -559,6 +560,10 @@ namespace Harness {
             return dirPath === path ? undefined : dirPath;
         }
 
+        function enumerateTestFiles(runner: RunnerBase) {
+            return runner.enumerateTestFiles();
+        }
+
         function listFiles(path: string, spec: RegExp, options?: { recursive?: boolean }) {
             options = options || {};
 
@@ -639,6 +644,7 @@ namespace Harness {
             directoryExists: path => ts.sys.directoryExists(path),
             deleteFile,
             listFiles,
+            enumerateTestFiles,
             log: s => console.log(s),
             args: () => ts.sys.args,
             getExecutingFilePath: () => ts.sys.getExecutingFilePath(),
@@ -913,6 +919,11 @@ namespace Harness {
             return ts.getDirectoryPath(ts.normalizeSlashes(url.pathname || "/"));
         }
 
+        function enumerateTestFiles(runner: RunnerBase): (string | FileBasedTest)[] {
+            const response = send(HttpRequestMessage.post(new URL("/api/enumerateTestFiles", serverRoot), HttpContent.text(runner.kind())));
+            return hasJsonContent(response) ? JSON.parse(response.content.content) : [];
+        }
+
         function listFiles(dirname: string, spec?: RegExp, options?: { recursive?: boolean }): string[] {
             if (spec || (options && !options.recursive)) {
                 let results = IO.listFiles(dirname);
@@ -959,6 +970,7 @@ namespace Harness {
             directoryExists,
             deleteFile,
             listFiles: Utils.memoize(listFiles, (path, spec, options) => `${path}|${spec}|${options ? options.recursive === true : true}`),
+            enumerateTestFiles: Utils.memoize(enumerateTestFiles, runner => runner.kind()),
             log: s => console.log(s),
             args: () => [],
             getExecutingFilePath: () => "",
@@ -1761,6 +1773,74 @@ namespace Harness {
         }
     }
 
+    export interface FileBasedTest {
+        file: string;
+        configurations?: FileBasedTestConfiguration[];
+    }
+
+    export interface FileBasedTestConfiguration {
+        [key: string]: string;
+    }
+
+    function splitVaryBySettingValue(text: string): string[] | undefined {
+        if (!text) return undefined;
+        const entries = text.split(/,/).map(s => s.trim().toLowerCase()).filter(s => s.length > 0);
+        return entries && entries.length > 1 ? entries : undefined;
+    }
+
+    function computeFileBasedTestConfigurationVariations(configurations: FileBasedTestConfiguration[], variationState: FileBasedTestConfiguration, varyByEntries: [string, string[]][], offset: number) {
+        if (offset >= varyByEntries.length) {
+            // make a copy of the current variation state
+            configurations.push({ ...variationState });
+            return;
+        }
+
+        const [varyBy, entries] = varyByEntries[offset];
+        for (const entry of entries) {
+            // set or overwrite the variation, then compute the next variation
+            variationState[varyBy] = entry;
+            computeFileBasedTestConfigurationVariations(configurations, variationState, varyByEntries, offset + 1);
+        }
+    }
+
+    /**
+     * Compute FileBasedTestConfiguration variations based on a supplied list of variable settings.
+     */
+    export function getFileBasedTestConfigurations(settings: TestCaseParser.CompilerSettings, varyBy: string[]): FileBasedTestConfiguration[] | undefined {
+        let varyByEntries: [string, string[]][] | undefined;
+        for (const varyByKey of varyBy) {
+            if (ts.hasProperty(settings, varyByKey)) {
+                // we only consider variations when there are 2 or more variable entries.
+                const entries = splitVaryBySettingValue(settings[varyByKey]);
+                if (entries) {
+                    if (!varyByEntries) varyByEntries = [];
+                    varyByEntries.push([varyByKey, entries]);
+                }
+            }
+        }
+
+        if (!varyByEntries) return undefined;
+
+        const configurations: FileBasedTestConfiguration[] = [];
+        computeFileBasedTestConfigurationVariations(configurations, /*variationState*/ {}, varyByEntries, /*offset*/ 0);
+        return configurations;
+    }
+
+    /**
+     * Compute a description for this configuration based on its entries
+     */
+    export function getFileBasedTestConfigurationDescription(configuration: FileBasedTestConfiguration) {
+        let name = "";
+        if (configuration) {
+            const keys = Object.keys(configuration).sort();
+            for (const key of keys) {
+                if (name) name += ", ";
+                name += `@${key}: ${configuration[key]}`;
+            }
+        }
+        return name;
+    }
+
     export namespace TestCaseParser {
         /** all the necessary information to set the right compiler settings */
         export interface CompilerSettings {
@@ -1779,7 +1859,7 @@ namespace Harness {
         // Regex for parsing options in the format "@Alpha: Value of any sort"
         const optionRegex = /^[\/]{2}\s*@(\w+)\s*:\s*([^\r\n]*)/gm;  // multiple matches on multiple lines
 
-        function extractCompilerSettings(content: string): CompilerSettings {
+        export function extractCompilerSettings(content: string): CompilerSettings {
             const opts: CompilerSettings = {};
 
             let match: RegExpExecArray;
@@ -1800,9 +1880,7 @@ namespace Harness {
         }
 
         /** Given a test file containing // @FileName directives, return an array of named units of code to be added to an existing compiler instance */
-        export function makeUnitsFromTest(code: string, fileName: string, rootDir?: string): TestCaseContent {
-            const settings = extractCompilerSettings(code);
-
+        export function makeUnitsFromTest(code: string, fileName: string, rootDir?: string, settings = extractCompilerSettings(code)): TestCaseContent {
             // List of all the subfiles we've parsed out
             const testUnitData: TestUnitData[] = [];
 
diff --git a/src/harness/parallel/host.ts b/src/harness/parallel/host.ts
@@ -81,7 +81,8 @@ namespace Harness.Parallel.Host {
         const { statSync }: { statSync(path: string): { size: number }; } = require("fs");
         const path: { join: (...args: string[]) => string } = require("path");
         for (const runner of runners) {
-            for (const file of runner.enumerateTestFiles()) {
+            for (const test of runner.enumerateTestFiles()) {
+                const file = typeof test === "string" ? test : test.file;
                 let size: number;
                 if (!perfData) {
                     try {
diff --git a/src/harness/projectsRunner.ts b/src/harness/projectsRunner.ts
@@ -51,7 +51,7 @@ namespace project {
             describe("projects tests", () => {
                 const tests = this.tests.length === 0 ? this.enumerateTestFiles() : this.tests;
                 for (const test of tests) {
-                    this.runProjectTestCase(test);
+                    this.runProjectTestCase(typeof test === "string" ? test : test.file);
                 }
             });
         }
diff --git a/src/harness/runnerbase.ts b/src/harness/runnerbase.ts
@@ -1,13 +1,10 @@
-/// <reference path="harness.ts" />
-
-
 type TestRunnerKind = CompilerTestKind | FourslashTestKind | "project" | "rwc" | "test262" | "user" | "dt";
 type CompilerTestKind = "conformance" | "compiler";
 type FourslashTestKind = "fourslash" | "fourslash-shims" | "fourslash-shims-pp" | "fourslash-server";
 
 abstract class RunnerBase {
     // contains the tests to run
-    public tests: string[] = [];
+    public tests: (string | Harness.FileBasedTest)[] = [];
 
     /** Add a source file to the runner's list of tests that need to be initialized with initializeTests */
     public addTest(fileName: string) {
@@ -20,7 +17,7 @@ abstract class RunnerBase {
 
     abstract kind(): TestRunnerKind;
 
-    abstract enumerateTestFiles(): string[];
+    abstract enumerateTestFiles(): (string | Harness.FileBasedTest)[];
 
     /** The working directory where tests are found. Needed for batch testing where the input path will differ from the output path inside baselines */
     public workingDirectory = "";
diff --git a/src/harness/rwcRunner.ts b/src/harness/rwcRunner.ts
diff --git a/src/harness/test262Runner.ts b/src/harness/test262Runner.ts
diff --git a/tests/webTestServer.ts b/tests/webTestServer.ts

Original file line number	Diff line number	Diff line change
`@@ -28,7 +28,7 @@ abstract class ExternalCompileRunnerBase extends RunnerBase {`
`28`	`28`
`29`	`29`	describe(`${this.kind()} code samples`, () => {
`30`	`30`	`for (const test of testList) {`
`31`		`- this.runTest(test);`
	`31`	`+ this.runTest(typeof test === "string" ? test : test.file);`
`32`	`32`	`}`
`33`	`33`	`});`
`34`	`34`	`}`
Original file line number	Diff line number	Diff line change
`@@ -51,7 +51,7 @@ namespace project {`
`51`	`51`	`describe("projects tests", () => {`
`52`	`52`	`const tests = this.tests.length === 0 ? this.enumerateTestFiles() : this.tests;`
`53`	`53`	`for (const test of tests) {`
`54`		`- this.runProjectTestCase(test);`
	`54`	`+ this.runProjectTestCase(typeof test === "string" ? test : test.file);`
`55`	`55`	`}`
`56`	`56`	`});`
`57`	`57`	`}`