Fix: verification of incremental correctness that was not working because of using wrote writeFile (microsoft#48751)

* Use fixed time for vfs so baselining is consistent

* Baseline buildinfos

* Write new file text in baseline even if the file wasnt read on the shadow

* Remove unnecessary debugger statement

* Make sure that incremental correctness is checked with correct writeFile so we know buildInfo was written
Also baseline these so its easy to verify the changes

* More baselines for the tsbuildinfo

* Renames and test fixes after dts Signature change merge

* COmment

Author: sheetalkamat

src/testRunner/unittests/tsbuild/helpers.ts

@@ -8,40 +8,81 @@ namespace ts {
             projFs = undefined!;
         });
 
-        function verifyNoEmitOnError(subScenario: string, fixModifyFs: TestTscEdit["modifyFs"], modifyFs?: TestTscEdit["modifyFs"]) {
-            verifyTscWithEdits({
-                scenario: "noEmitOnError",
-                subScenario,
-                fs: () => projFs,
-                modifyFs,
-                commandLineArgs: ["--b", "/src/tsconfig.json"],
-                edits: [
-                    noChangeRun,
-                    {
-                        subScenario: "Fix error",
-                        modifyFs: fixModifyFs,
-                    },
-                    noChangeRun,
-                ],
-                baselinePrograms: true,
-                baselineIncremental: true
-            });
-        }
+        verifyTscWithEdits({
+            scenario: "noEmitOnError",
+            subScenario: "syntax errors",
+            fs: () => projFs,
+            commandLineArgs: ["--b", "/src/tsconfig.json"],
+            edits: [
+                noChangeRun,
+                {
+                    subScenario: "Fix error",
+                    modifyFs: fs => fs.writeFileSync("/src/src/main.ts", `import { A } from "../shared/types/db";
+const a = {
+    lastName: 'sdsd'
+};`, "utf-8"),
+                },
+                noChangeRun,
+            ],
+            baselinePrograms: true,
+        });
 
-        verifyNoEmitOnError(
-            "syntax errors",
-            fs => fs.writeFileSync("/src/src/main.ts", `import { A } from "../shared/types/db";
+        verifyTscWithEdits({
+            scenario: "noEmitOnError",
+            subScenario: "syntax errors with incremental",
+            fs: () => projFs,
+            commandLineArgs: ["--b", "/src/tsconfig.json", "--incremental"],
+            edits: [
+                noChangeRun,
+                {
+                    subScenario: "Fix error",
+                    modifyFs: fs => fs.writeFileSync("/src/src/main.ts", `import { A } from "../shared/types/db";
 const a = {
     lastName: 'sdsd'
-};`, "utf-8")
-        );
+};`, "utf-8"),
+                    discrepancyExplanation: noChangeWithExportsDiscrepancyRun.discrepancyExplanation,
+                },
+                noChangeWithExportsDiscrepancyRun,
+            ],
+            baselinePrograms: true,
+        });
 
-        verifyNoEmitOnError(
-            "semantic errors",
-            fs => fs.writeFileSync("/src/src/main.ts", `import { A } from "../shared/types/db";
+        verifyTscWithEdits({
+            scenario: "noEmitOnError",
+            subScenario: "semantic errors",
+            fs: () => projFs,
+            modifyFs: fs => fs.writeFileSync("/src/src/main.ts", `import { A } from "../shared/types/db";
+const a: string = 10;`, "utf-8"),
+            commandLineArgs: ["--b", "/src/tsconfig.json"],
+            edits: [
+                noChangeRun,
+                {
+                    subScenario: "Fix error",
+                    modifyFs: fs => fs.writeFileSync("/src/src/main.ts", `import { A } from "../shared/types/db";
 const a: string = "hello";`, "utf-8"),
-            fs => fs.writeFileSync("/src/src/main.ts", `import { A } from "../shared/types/db";
-const a: string = 10;`, "utf-8")
-        );
+                },
+                noChangeRun,
+            ],
+            baselinePrograms: true,
+        });
+
+        verifyTscWithEdits({
+            scenario: "noEmitOnError",
+            subScenario: "semantic errors with incremental",
+            fs: () => projFs,
+            modifyFs: fs => fs.writeFileSync("/src/src/main.ts", `import { A } from "../shared/types/db";
+const a: string = 10;`, "utf-8"),
+            commandLineArgs: ["--b", "/src/tsconfig.json", "--incremental"],
+            edits: [
+                noChangeWithExportsDiscrepancyRun,
+                {
+                    subScenario: "Fix error",
+                    modifyFs: fs => fs.writeFileSync("/src/src/main.ts", `import { A } from "../shared/types/db";
+const a: string = "hello";`, "utf-8"),
+                },
+                noChangeRun,
+            ],
+            baselinePrograms: true,
+        });
     });
 }

src/testRunner/unittests/tsbuild/noEmitOnError.ts

@@ -46,7 +46,7 @@ export function f22() { } // trailing`,
                 writtenFiles.add(path);
                 return originalWriteFile.call(sys, fileName, content, writeByteOrderMark);
             };
-            const { cb, getPrograms } = commandLineCallbacks(sys, /*originalReadCall*/ undefined, originalWriteFile);
+            const { cb, getPrograms } = commandLineCallbacks(sys, /*originalReadCall*/ undefined);
             const buildHost = createSolutionBuilderHost(
                 sys,
                     /*createProgram*/ undefined,

src/testRunner/unittests/tsbuild/publicApi.ts

@@ -246,7 +246,7 @@ namespace ts {
                     )
                 );
 
-                const host = createSolutionBuilderHost(system);
+                const host = createSolutionBuilderHostForBaseline(system);
                 const builder = createSolutionBuilder(host, [testsConfig.path], {});
                 baseline.push("Input::");
                 baselineState();
@@ -317,7 +317,7 @@ namespace ts {
                     )
                 );
 
-                const host = createSolutionBuilderHost(system);
+                const host = createSolutionBuilderHostForBaseline(system);
                 const builder = createSolutionBuilder(host, [testsConfig.path], { dry: false, force: false, verbose: false });
                 builder.build();
                 baselineState("Build of project");

src/testRunner/unittests/tsbuild/sample.ts

@@ -10,7 +10,15 @@ namespace ts {
         subScenario: "no-change-run",
         modifyFs: noop
     };
+    export const noChangeWithExportsDiscrepancyRun: TestTscEdit = {
+        ...noChangeRun,
+        discrepancyExplanation: () => [
+            "Incremental build did not emit and has .ts as signature so exports has all imported modules/referenced files",
+            "Clean build always uses d.ts for signature for testing thus does not contain non exported modules/referenced files that arent needed"
+        ]
+    };
     export const noChangeOnlyRuns = [noChangeRun];
+    export const noChangeWithExportsDiscrepancyOnlyRuns = [noChangeWithExportsDiscrepancyRun];
 
     export interface TestTscCompile extends TestTscCompileLikeBase {
         baselineSourceMap?: boolean;
@@ -29,23 +37,22 @@ namespace ts {
         return !!(program as Program | BuilderProgram).getCompilerOptions;
     }
     export function commandLineCallbacks(
-        sys: System & { writtenFiles: ReadonlyCollection<Path>; },
+        sys: TscCompileSystem | tscWatch.WatchedSystem,
         originalReadCall?: System["readFile"],
-        originalWriteFile?: System["writeFile"],
     ): CommandLineCallbacks {
         let programs: CommandLineProgram[] | undefined;
 
         return {
             cb: program => {
                 if (isAnyProgram(program)) {
-                    baselineBuildInfo(program.getCompilerOptions(), sys, originalReadCall, originalWriteFile);
+                    baselineBuildInfo(program.getCompilerOptions(), sys, originalReadCall);
                     (programs || (programs = [])).push(isBuilderProgram(program) ?
                         [program.getProgram(), program] :
                         [program]
                     );
                 }
                 else {
-                    baselineBuildInfo(program.options, sys, originalReadCall, originalWriteFile);
+                    baselineBuildInfo(program.options, sys, originalReadCall);
                 }
             },
             getPrograms: () => {
@@ -122,16 +129,22 @@ ${patch ? vfs.formatPatch(patch) : ""}`
         const originalWriteFile = sys.writeFile;
         sys.writeFile = (fileName, content, writeByteOrderMark) => {
             const path = toPathWithSystem(sys, fileName);
-            assert.isFalse(writtenFiles.has(path));
+            // When buildinfo is same for two projects,
+            // it gives error and doesnt write buildinfo but because buildInfo is written for one project,
+            // readable baseline will be written two times for those two projects with same contents and is ok
+            Debug.assert(!writtenFiles.has(path) || endsWith(path, "baseline.txt"));
             writtenFiles.add(path);
             return originalWriteFile.call(sys, fileName, content, writeByteOrderMark);
         };
-        return originalWriteFile;
     }
 
-    export function createSolutionBuilderHostForBaseline(sys: TscCompileSystem, versionToWrite?: string) {
-        makeSystemReadyForBaseline(sys, versionToWrite);
-        const { cb } = commandLineCallbacks(sys);
+    export function createSolutionBuilderHostForBaseline(
+        sys: TscCompileSystem | tscWatch.WatchedSystem,
+        versionToWrite?: string,
+        originalRead?: (TscCompileSystem | tscWatch.WatchedSystem)["readFile"]
+    ) {
+        if (sys instanceof fakes.System) makeSystemReadyForBaseline(sys, versionToWrite);
+        const { cb } = commandLineCallbacks(sys, originalRead);
         const host = createSolutionBuilderHost(sys,
             /*createProgram*/ undefined,
             createDiagnosticReporter(sys, /*pretty*/ true),
@@ -155,7 +168,7 @@ ${patch ? vfs.formatPatch(patch) : ""}`
         });
 
         function commandLineCompile(sys: TscCompileSystem) {
-            const originalWriteFile = makeSystemReadyForBaseline(sys);
+            makeSystemReadyForBaseline(sys);
             actualReadFileMap = {};
             const originalReadFile = sys.readFile;
             sys.readFile = path => {
@@ -166,7 +179,7 @@ ${patch ? vfs.formatPatch(patch) : ""}`
                 return originalReadFile.call(sys, path);
             };
 
-            const result = commandLineCallbacks(sys, originalReadFile, originalWriteFile);
+            const result = commandLineCallbacks(sys, originalReadFile);
             executeCommandLine(
                 sys,
                 result.cb,

src/testRunner/unittests/tsc/helpers.ts

@@ -90,7 +90,7 @@ namespace ts {
                     commandLineArgs: ["--incremental", "-p", "src"],
                     modifyFs,
                     edits: [
-                        noChangeRun,
+                        noChangeWithExportsDiscrepancyRun,
                         {
                             subScenario: "incremental-declaration-doesnt-change",
                             modifyFs: fixModifyFs
@@ -123,15 +123,20 @@ const a: string = 10;`, "utf-8"),
             verifyNoEmitChanges({ composite: true });
 
             function verifyNoEmitChanges(compilerOptions: CompilerOptions) {
+                const discrepancyIfNoDtsEmit = getEmitDeclarations(compilerOptions) ?
+                    undefined :
+                    noChangeWithExportsDiscrepancyRun.discrepancyExplanation;
                 const noChangeRunWithNoEmit: TestTscEdit = {
                     ...noChangeRun,
                     subScenario: "No Change run with noEmit",
                     commandLineArgs: ["--p", "src/project", "--noEmit"],
+                    discrepancyExplanation: discrepancyIfNoDtsEmit,
                 };
                 const noChangeRunWithEmit: TestTscEdit = {
                     ...noChangeRun,
                     subScenario: "No Change run with emit",
                     commandLineArgs: ["--p", "src/project"],
+                    discrepancyExplanation: discrepancyIfNoDtsEmit,
                 };
                 let optionsString = "";
                 for (const key in compilerOptions) {
@@ -152,10 +157,12 @@ const a: string = 10;`, "utf-8"),
                             subScenario: "Introduce error but still noEmit",
                             commandLineArgs: ["--p", "src/project", "--noEmit"],
                             modifyFs: fs => replaceText(fs, "/src/project/src/class.ts", "prop", "prop1"),
+                            discrepancyExplanation: getEmitDeclarations(compilerOptions) ? noChangeWithExportsDiscrepancyRun.discrepancyExplanation : undefined,
                         },
                         {
                             subScenario: "Fix error and emit",
                             modifyFs: fs => replaceText(fs, "/src/project/src/class.ts", "prop1", "prop"),
+                            discrepancyExplanation: discrepancyIfNoDtsEmit
                         },
                         noChangeRunWithEmit,
                         noChangeRunWithNoEmit,
@@ -164,6 +171,7 @@ const a: string = 10;`, "utf-8"),
                         {
                             subScenario: "Introduce error and emit",
                             modifyFs: fs => replaceText(fs, "/src/project/src/class.ts", "prop", "prop1"),
+                            discrepancyExplanation: discrepancyIfNoDtsEmit
                         },
                         noChangeRunWithEmit,
                         noChangeRunWithNoEmit,
@@ -173,6 +181,7 @@ const a: string = 10;`, "utf-8"),
                             subScenario: "Fix error and no emit",
                             commandLineArgs: ["--p", "src/project", "--noEmit"],
                             modifyFs: fs => replaceText(fs, "/src/project/src/class.ts", "prop1", "prop"),
+                            discrepancyExplanation: noChangeWithExportsDiscrepancyRun.discrepancyExplanation,
                         },
                         noChangeRunWithEmit,
                         noChangeRunWithNoEmit,
@@ -196,6 +205,7 @@ const a: string = 10;`, "utf-8"),
                         {
                             subScenario: "Fix error and no emit",
                             modifyFs: fs => replaceText(fs, "/src/project/src/class.ts", "prop1", "prop"),
+                            discrepancyExplanation: noChangeWithExportsDiscrepancyRun.discrepancyExplanation
                         },
                         noChangeRunWithEmit,
                     ],
@@ -352,11 +362,10 @@ declare global {
                 {
                     subScenario: "Add class3 to project1 and build it",
                     modifyFs: fs => fs.writeFileSync("/src/projects/project1/class3.ts", `class class3 {}`, "utf-8"),
-                    cleanBuildDiscrepancies: () => new Map<string, CleanBuildDescrepancy>([
-                        // Ts buildinfo will not be updated in incremental build so it will have semantic diagnostics cached from previous build
-                        // But in clean build because of global diagnostics, semantic diagnostics are not queried so not cached in tsbuildinfo
-                        ["/src/projects/project2/tsconfig.tsbuildinfo", CleanBuildDescrepancy.CleanFileTextDifferent]
-                    ]),
+                    discrepancyExplanation: () => [
+                        "Ts buildinfo will not be updated in incremental build so it will have semantic diagnostics cached from previous build",
+                        "But in clean build because of global diagnostics, semantic diagnostics are not queried so not cached in tsbuildinfo",
+                    ],
                 },
                 {
                     subScenario: "Add output of class3",
@@ -372,11 +381,10 @@ declare global {
                 {
                     subScenario: "Delete output for class3",
                     modifyFs: fs => fs.unlinkSync("/src/projects/project1/class3.d.ts"),
-                    cleanBuildDiscrepancies: () => new Map<string, CleanBuildDescrepancy>([
-                        // Ts buildinfo willbe updated but will retain lib file errors from previous build and not others because they are emitted because of change which results in clearing their semantic diagnostics cache
-                        // But in clean build because of global diagnostics, semantic diagnostics are not queried so not cached in tsbuildinfo
-                        ["/src/projects/project2/tsconfig.tsbuildinfo", CleanBuildDescrepancy.CleanFileTextDifferent]
-                    ]),
+                    discrepancyExplanation: () => [
+                        "Ts buildinfo will be updated but will retain lib file errors from previous build and not others because they are emitted because of change which results in clearing their semantic diagnostics cache",
+                        "But in clean build because of global diagnostics, semantic diagnostics are not queried so not cached in tsbuildinfo",
+                    ],
                 },
                 {
                     subScenario: "Create output for class3",

src/testRunner/unittests/tsc/incremental.ts

@@ -171,9 +171,10 @@ namespace ts.tscWatch {
     export interface Baseline extends BaselineBase, CommandLineCallbacks {
     }
 
-    export function createBaseline(system: WatchedSystem, modifySystem?: (sys: WatchedSystem) => void): Baseline {
+    export function createBaseline(system: WatchedSystem, modifySystem?: (sys: WatchedSystem, originalRead: WatchedSystem["readFile"]) => void): Baseline {
+        const originalRead = system.readFile;
         const initialSys = fakes.patchHostForBuildInfoReadWrite(system);
-        modifySystem?.(initialSys);
+        modifySystem?.(initialSys, originalRead);
         const sys = TestFSWithWatch.changeToHostTrackingWrittenFiles(initialSys);
         const baseline: string[] = [];
         baseline.push("Input::");
@@ -410,30 +411,32 @@ namespace ts.tscWatch {
         sys.writeFile(file, content.replace(searchValue, replaceValue));
     }
 
-    export function createSolutionBuilder(system: WatchedSystem, rootNames: readonly string[], defaultOptions?: BuildOptions) {
-        const host = createSolutionBuilderHost(system);
-        return ts.createSolutionBuilder(host, rootNames, defaultOptions || {});
+    export function createSolutionBuilder(system: WatchedSystem, rootNames: readonly string[], originalRead?: WatchedSystem["readFile"]) {
+        const host = createSolutionBuilderHostForBaseline(system, /*versionToWrite*/ undefined, originalRead);
+        return ts.createSolutionBuilder(host, rootNames, {});
     }
 
     export function ensureErrorFreeBuild(host: WatchedSystem, rootNames: readonly string[]) {
         // ts build should succeed
-        const solutionBuilder = createSolutionBuilder(host, rootNames, {});
-        solutionBuilder.build();
+        solutionBuildWithBaseline(host, rootNames);
         assert.equal(host.getOutput().length, 0, JSON.stringify(host.getOutput(), /*replacer*/ undefined, " "));
     }
 
-    export function createSystemWithSolutionBuild(solutionRoots: readonly string[], files: readonly TestFSWithWatch.FileOrFolderOrSymLink[], params?: TestFSWithWatch.TestServerHostCreationParameters) {
-        const sys = createWatchedSystem(files, params);
+    export function solutionBuildWithBaseline(sys: WatchedSystem, solutionRoots: readonly string[], originalRead?: WatchedSystem["readFile"]) {
         const originalReadFile = sys.readFile;
         const originalWrite = sys.write;
         const originalWriteFile = sys.writeFile;
         const solutionBuilder = createSolutionBuilder(TestFSWithWatch.changeToHostTrackingWrittenFiles(
             fakes.patchHostForBuildInfoReadWrite(sys)
-        ), solutionRoots, {});
+        ), solutionRoots, originalRead);
         solutionBuilder.build();
         sys.readFile = originalReadFile;
         sys.write = originalWrite;
         sys.writeFile = originalWriteFile;
         return sys;
     }
+
+    export function createSystemWithSolutionBuild(solutionRoots: readonly string[], files: readonly TestFSWithWatch.FileOrFolderOrSymLink[], params?: TestFSWithWatch.TestServerHostCreationParameters) {
+        return solutionBuildWithBaseline(createWatchedSystem(files, params), solutionRoots);
+    }
 }