Skip to content

Commit 94cb657

Browse files
sheetalkamatsheetalkamat
authored
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
1 parent af30c79 commit 94cb657

File tree

36 files changed

+3469
-323
lines changed

36 files changed

+3469
-323
lines changed

‎src/testRunner/unittests/tsbuild/helpers.ts

+198-164
Large diffs are not rendered by default.

‎src/testRunner/unittests/tsbuild/noEmitOnError.ts

+71-30
Original file line numberDiff line numberDiff line change
@@ -8,40 +8,81 @@ namespace ts {
88
projFs = undefined!;
99
});
1010

11-
function verifyNoEmitOnError(subScenario: string, fixModifyFs: TestTscEdit["modifyFs"], modifyFs?: TestTscEdit["modifyFs"]) {
12-
verifyTscWithEdits({
13-
scenario: "noEmitOnError",
14-
subScenario,
15-
fs: () => projFs,
16-
modifyFs,
17-
commandLineArgs: ["--b", "/src/tsconfig.json"],
18-
edits: [
19-
noChangeRun,
20-
{
21-
subScenario: "Fix error",
22-
modifyFs: fixModifyFs,
23-
},
24-
noChangeRun,
25-
],
26-
baselinePrograms: true,
27-
baselineIncremental: true
28-
});
29-
}
11+
verifyTscWithEdits({
12+
scenario: "noEmitOnError",
13+
subScenario: "syntax errors",
14+
fs: () => projFs,
15+
commandLineArgs: ["--b", "/src/tsconfig.json"],
16+
edits: [
17+
noChangeRun,
18+
{
19+
subScenario: "Fix error",
20+
modifyFs: fs => fs.writeFileSync("/src/src/main.ts", `import { A } from "../shared/types/db";
21+
const a = {
22+
lastName: 'sdsd'
23+
};`, "utf-8"),
24+
},
25+
noChangeRun,
26+
],
27+
baselinePrograms: true,
28+
});
3029

31-
verifyNoEmitOnError(
32-
"syntax errors",
33-
fs => fs.writeFileSync("/src/src/main.ts", `import { A } from "../shared/types/db";
30+
verifyTscWithEdits({
31+
scenario: "noEmitOnError",
32+
subScenario: "syntax errors with incremental",
33+
fs: () => projFs,
34+
commandLineArgs: ["--b", "/src/tsconfig.json", "--incremental"],
35+
edits: [
36+
noChangeRun,
37+
{
38+
subScenario: "Fix error",
39+
modifyFs: fs => fs.writeFileSync("/src/src/main.ts", `import { A } from "../shared/types/db";
3440
const a = {
3541
lastName: 'sdsd'
36-
};`, "utf-8")
37-
);
42+
};`, "utf-8"),
43+
discrepancyExplanation: noChangeWithExportsDiscrepancyRun.discrepancyExplanation,
44+
},
45+
noChangeWithExportsDiscrepancyRun,
46+
],
47+
baselinePrograms: true,
48+
});
3849

39-
verifyNoEmitOnError(
40-
"semantic errors",
41-
fs => fs.writeFileSync("/src/src/main.ts", `import { A } from "../shared/types/db";
50+
verifyTscWithEdits({
51+
scenario: "noEmitOnError",
52+
subScenario: "semantic errors",
53+
fs: () => projFs,
54+
modifyFs: fs => fs.writeFileSync("/src/src/main.ts", `import { A } from "../shared/types/db";
55+
const a: string = 10;`, "utf-8"),
56+
commandLineArgs: ["--b", "/src/tsconfig.json"],
57+
edits: [
58+
noChangeRun,
59+
{
60+
subScenario: "Fix error",
61+
modifyFs: fs => fs.writeFileSync("/src/src/main.ts", `import { A } from "../shared/types/db";
4262
const a: string = "hello";`, "utf-8"),
43-
fs => fs.writeFileSync("/src/src/main.ts", `import { A } from "../shared/types/db";
44-
const a: string = 10;`, "utf-8")
45-
);
63+
},
64+
noChangeRun,
65+
],
66+
baselinePrograms: true,
67+
});
68+
69+
verifyTscWithEdits({
70+
scenario: "noEmitOnError",
71+
subScenario: "semantic errors with incremental",
72+
fs: () => projFs,
73+
modifyFs: fs => fs.writeFileSync("/src/src/main.ts", `import { A } from "../shared/types/db";
74+
const a: string = 10;`, "utf-8"),
75+
commandLineArgs: ["--b", "/src/tsconfig.json", "--incremental"],
76+
edits: [
77+
noChangeWithExportsDiscrepancyRun,
78+
{
79+
subScenario: "Fix error",
80+
modifyFs: fs => fs.writeFileSync("/src/src/main.ts", `import { A } from "../shared/types/db";
81+
const a: string = "hello";`, "utf-8"),
82+
},
83+
noChangeRun,
84+
],
85+
baselinePrograms: true,
86+
});
4687
});
4788
}

‎src/testRunner/unittests/tsbuild/publicApi.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ export function f22() { } // trailing`,
4646
writtenFiles.add(path);
4747
return originalWriteFile.call(sys, fileName, content, writeByteOrderMark);
4848
};
49-
const { cb, getPrograms } = commandLineCallbacks(sys, /*originalReadCall*/ undefined, originalWriteFile);
49+
const { cb, getPrograms } = commandLineCallbacks(sys, /*originalReadCall*/ undefined);
5050
const buildHost = createSolutionBuilderHost(
5151
sys,
5252
/*createProgram*/ undefined,

‎src/testRunner/unittests/tsbuild/sample.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ namespace ts {
246246
)
247247
);
248248

249-
const host = createSolutionBuilderHost(system);
249+
const host = createSolutionBuilderHostForBaseline(system);
250250
const builder = createSolutionBuilder(host, [testsConfig.path], {});
251251
baseline.push("Input::");
252252
baselineState();
@@ -317,7 +317,7 @@ namespace ts {
317317
)
318318
);
319319

320-
const host = createSolutionBuilderHost(system);
320+
const host = createSolutionBuilderHostForBaseline(system);
321321
const builder = createSolutionBuilder(host, [testsConfig.path], { dry: false, force: false, verbose: false });
322322
builder.build();
323323
baselineState("Build of project");

‎src/testRunner/unittests/tsc/helpers.ts

+24-11
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,15 @@ namespace ts {
1010
subScenario: "no-change-run",
1111
modifyFs: noop
1212
};
13+
export const noChangeWithExportsDiscrepancyRun: TestTscEdit = {
14+
...noChangeRun,
15+
discrepancyExplanation: () => [
16+
"Incremental build did not emit and has .ts as signature so exports has all imported modules/referenced files",
17+
"Clean build always uses d.ts for signature for testing thus does not contain non exported modules/referenced files that arent needed"
18+
]
19+
};
1320
export const noChangeOnlyRuns = [noChangeRun];
21+
export const noChangeWithExportsDiscrepancyOnlyRuns = [noChangeWithExportsDiscrepancyRun];
1422

1523
export interface TestTscCompile extends TestTscCompileLikeBase {
1624
baselineSourceMap?: boolean;
@@ -29,23 +37,22 @@ namespace ts {
2937
return !!(program as Program | BuilderProgram).getCompilerOptions;
3038
}
3139
export function commandLineCallbacks(
32-
sys: System & { writtenFiles: ReadonlyCollection<Path>; },
40+
sys: TscCompileSystem | tscWatch.WatchedSystem,
3341
originalReadCall?: System["readFile"],
34-
originalWriteFile?: System["writeFile"],
3542
): CommandLineCallbacks {
3643
let programs: CommandLineProgram[] | undefined;
3744

3845
return {
3946
cb: program => {
4047
if (isAnyProgram(program)) {
41-
baselineBuildInfo(program.getCompilerOptions(), sys, originalReadCall, originalWriteFile);
48+
baselineBuildInfo(program.getCompilerOptions(), sys, originalReadCall);
4249
(programs || (programs = [])).push(isBuilderProgram(program) ?
4350
[program.getProgram(), program] :
4451
[program]
4552
);
4653
}
4754
else {
48-
baselineBuildInfo(program.options, sys, originalReadCall, originalWriteFile);
55+
baselineBuildInfo(program.options, sys, originalReadCall);
4956
}
5057
},
5158
getPrograms: () => {
@@ -122,16 +129,22 @@ ${patch ? vfs.formatPatch(patch) : ""}`
122129
const originalWriteFile = sys.writeFile;
123130
sys.writeFile = (fileName, content, writeByteOrderMark) => {
124131
const path = toPathWithSystem(sys, fileName);
125-
assert.isFalse(writtenFiles.has(path));
132+
// When buildinfo is same for two projects,
133+
// it gives error and doesnt write buildinfo but because buildInfo is written for one project,
134+
// readable baseline will be written two times for those two projects with same contents and is ok
135+
Debug.assert(!writtenFiles.has(path) || endsWith(path, "baseline.txt"));
126136
writtenFiles.add(path);
127137
return originalWriteFile.call(sys, fileName, content, writeByteOrderMark);
128138
};
129-
return originalWriteFile;
130139
}
131140

132-
export function createSolutionBuilderHostForBaseline(sys: TscCompileSystem, versionToWrite?: string) {
133-
makeSystemReadyForBaseline(sys, versionToWrite);
134-
const { cb } = commandLineCallbacks(sys);
141+
export function createSolutionBuilderHostForBaseline(
142+
sys: TscCompileSystem | tscWatch.WatchedSystem,
143+
versionToWrite?: string,
144+
originalRead?: (TscCompileSystem | tscWatch.WatchedSystem)["readFile"]
145+
) {
146+
if (sys instanceof fakes.System) makeSystemReadyForBaseline(sys, versionToWrite);
147+
const { cb } = commandLineCallbacks(sys, originalRead);
135148
const host = createSolutionBuilderHost(sys,
136149
/*createProgram*/ undefined,
137150
createDiagnosticReporter(sys, /*pretty*/ true),
@@ -155,7 +168,7 @@ ${patch ? vfs.formatPatch(patch) : ""}`
155168
});
156169

157170
function commandLineCompile(sys: TscCompileSystem) {
158-
const originalWriteFile = makeSystemReadyForBaseline(sys);
171+
makeSystemReadyForBaseline(sys);
159172
actualReadFileMap = {};
160173
const originalReadFile = sys.readFile;
161174
sys.readFile = path => {
@@ -166,7 +179,7 @@ ${patch ? vfs.formatPatch(patch) : ""}`
166179
return originalReadFile.call(sys, path);
167180
};
168181

169-
const result = commandLineCallbacks(sys, originalReadFile, originalWriteFile);
182+
const result = commandLineCallbacks(sys, originalReadFile);
170183
executeCommandLine(
171184
sys,
172185
result.cb,

‎src/testRunner/unittests/tsc/incremental.ts

+19-11
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ namespace ts {
9090
commandLineArgs: ["--incremental", "-p", "src"],
9191
modifyFs,
9292
edits: [
93-
noChangeRun,
93+
noChangeWithExportsDiscrepancyRun,
9494
{
9595
subScenario: "incremental-declaration-doesnt-change",
9696
modifyFs: fixModifyFs
@@ -123,15 +123,20 @@ const a: string = 10;`, "utf-8"),
123123
verifyNoEmitChanges({ composite: true });
124124

125125
function verifyNoEmitChanges(compilerOptions: CompilerOptions) {
126+
const discrepancyIfNoDtsEmit = getEmitDeclarations(compilerOptions) ?
127+
undefined :
128+
noChangeWithExportsDiscrepancyRun.discrepancyExplanation;
126129
const noChangeRunWithNoEmit: TestTscEdit = {
127130
...noChangeRun,
128131
subScenario: "No Change run with noEmit",
129132
commandLineArgs: ["--p", "src/project", "--noEmit"],
133+
discrepancyExplanation: discrepancyIfNoDtsEmit,
130134
};
131135
const noChangeRunWithEmit: TestTscEdit = {
132136
...noChangeRun,
133137
subScenario: "No Change run with emit",
134138
commandLineArgs: ["--p", "src/project"],
139+
discrepancyExplanation: discrepancyIfNoDtsEmit,
135140
};
136141
let optionsString = "";
137142
for (const key in compilerOptions) {
@@ -152,10 +157,12 @@ const a: string = 10;`, "utf-8"),
152157
subScenario: "Introduce error but still noEmit",
153158
commandLineArgs: ["--p", "src/project", "--noEmit"],
154159
modifyFs: fs => replaceText(fs, "/src/project/src/class.ts", "prop", "prop1"),
160+
discrepancyExplanation: getEmitDeclarations(compilerOptions) ? noChangeWithExportsDiscrepancyRun.discrepancyExplanation : undefined,
155161
},
156162
{
157163
subScenario: "Fix error and emit",
158164
modifyFs: fs => replaceText(fs, "/src/project/src/class.ts", "prop1", "prop"),
165+
discrepancyExplanation: discrepancyIfNoDtsEmit
159166
},
160167
noChangeRunWithEmit,
161168
noChangeRunWithNoEmit,
@@ -164,6 +171,7 @@ const a: string = 10;`, "utf-8"),
164171
{
165172
subScenario: "Introduce error and emit",
166173
modifyFs: fs => replaceText(fs, "/src/project/src/class.ts", "prop", "prop1"),
174+
discrepancyExplanation: discrepancyIfNoDtsEmit
167175
},
168176
noChangeRunWithEmit,
169177
noChangeRunWithNoEmit,
@@ -173,6 +181,7 @@ const a: string = 10;`, "utf-8"),
173181
subScenario: "Fix error and no emit",
174182
commandLineArgs: ["--p", "src/project", "--noEmit"],
175183
modifyFs: fs => replaceText(fs, "/src/project/src/class.ts", "prop1", "prop"),
184+
discrepancyExplanation: noChangeWithExportsDiscrepancyRun.discrepancyExplanation,
176185
},
177186
noChangeRunWithEmit,
178187
noChangeRunWithNoEmit,
@@ -196,6 +205,7 @@ const a: string = 10;`, "utf-8"),
196205
{
197206
subScenario: "Fix error and no emit",
198207
modifyFs: fs => replaceText(fs, "/src/project/src/class.ts", "prop1", "prop"),
208+
discrepancyExplanation: noChangeWithExportsDiscrepancyRun.discrepancyExplanation
199209
},
200210
noChangeRunWithEmit,
201211
],
@@ -352,11 +362,10 @@ declare global {
352362
{
353363
subScenario: "Add class3 to project1 and build it",
354364
modifyFs: fs => fs.writeFileSync("/src/projects/project1/class3.ts", `class class3 {}`, "utf-8"),
355-
cleanBuildDiscrepancies: () => new Map<string, CleanBuildDescrepancy>([
356-
// Ts buildinfo will not be updated in incremental build so it will have semantic diagnostics cached from previous build
357-
// But in clean build because of global diagnostics, semantic diagnostics are not queried so not cached in tsbuildinfo
358-
["/src/projects/project2/tsconfig.tsbuildinfo", CleanBuildDescrepancy.CleanFileTextDifferent]
359-
]),
365+
discrepancyExplanation: () => [
366+
"Ts buildinfo will not be updated in incremental build so it will have semantic diagnostics cached from previous build",
367+
"But in clean build because of global diagnostics, semantic diagnostics are not queried so not cached in tsbuildinfo",
368+
],
360369
},
361370
{
362371
subScenario: "Add output of class3",
@@ -372,11 +381,10 @@ declare global {
372381
{
373382
subScenario: "Delete output for class3",
374383
modifyFs: fs => fs.unlinkSync("/src/projects/project1/class3.d.ts"),
375-
cleanBuildDiscrepancies: () => new Map<string, CleanBuildDescrepancy>([
376-
// 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
377-
// But in clean build because of global diagnostics, semantic diagnostics are not queried so not cached in tsbuildinfo
378-
["/src/projects/project2/tsconfig.tsbuildinfo", CleanBuildDescrepancy.CleanFileTextDifferent]
379-
]),
384+
discrepancyExplanation: () => [
385+
"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",
386+
"But in clean build because of global diagnostics, semantic diagnostics are not queried so not cached in tsbuildinfo",
387+
],
380388
},
381389
{
382390
subScenario: "Create output for class3",

‎src/testRunner/unittests/tscWatch/helpers.ts

+13-10
Original file line numberDiff line numberDiff line change
@@ -171,9 +171,10 @@ namespace ts.tscWatch {
171171
export interface Baseline extends BaselineBase, CommandLineCallbacks {
172172
}
173173

174-
export function createBaseline(system: WatchedSystem, modifySystem?: (sys: WatchedSystem) => void): Baseline {
174+
export function createBaseline(system: WatchedSystem, modifySystem?: (sys: WatchedSystem, originalRead: WatchedSystem["readFile"]) => void): Baseline {
175+
const originalRead = system.readFile;
175176
const initialSys = fakes.patchHostForBuildInfoReadWrite(system);
176-
modifySystem?.(initialSys);
177+
modifySystem?.(initialSys, originalRead);
177178
const sys = TestFSWithWatch.changeToHostTrackingWrittenFiles(initialSys);
178179
const baseline: string[] = [];
179180
baseline.push("Input::");
@@ -410,30 +411,32 @@ namespace ts.tscWatch {
410411
sys.writeFile(file, content.replace(searchValue, replaceValue));
411412
}
412413

413-
export function createSolutionBuilder(system: WatchedSystem, rootNames: readonly string[], defaultOptions?: BuildOptions) {
414-
const host = createSolutionBuilderHost(system);
415-
return ts.createSolutionBuilder(host, rootNames, defaultOptions || {});
414+
export function createSolutionBuilder(system: WatchedSystem, rootNames: readonly string[], originalRead?: WatchedSystem["readFile"]) {
415+
const host = createSolutionBuilderHostForBaseline(system, /*versionToWrite*/ undefined, originalRead);
416+
return ts.createSolutionBuilder(host, rootNames, {});
416417
}
417418

418419
export function ensureErrorFreeBuild(host: WatchedSystem, rootNames: readonly string[]) {
419420
// ts build should succeed
420-
const solutionBuilder = createSolutionBuilder(host, rootNames, {});
421-
solutionBuilder.build();
421+
solutionBuildWithBaseline(host, rootNames);
422422
assert.equal(host.getOutput().length, 0, JSON.stringify(host.getOutput(), /*replacer*/ undefined, " "));
423423
}
424424

425-
export function createSystemWithSolutionBuild(solutionRoots: readonly string[], files: readonly TestFSWithWatch.FileOrFolderOrSymLink[], params?: TestFSWithWatch.TestServerHostCreationParameters) {
426-
const sys = createWatchedSystem(files, params);
425+
export function solutionBuildWithBaseline(sys: WatchedSystem, solutionRoots: readonly string[], originalRead?: WatchedSystem["readFile"]) {
427426
const originalReadFile = sys.readFile;
428427
const originalWrite = sys.write;
429428
const originalWriteFile = sys.writeFile;
430429
const solutionBuilder = createSolutionBuilder(TestFSWithWatch.changeToHostTrackingWrittenFiles(
431430
fakes.patchHostForBuildInfoReadWrite(sys)
432-
), solutionRoots, {});
431+
), solutionRoots, originalRead);
433432
solutionBuilder.build();
434433
sys.readFile = originalReadFile;
435434
sys.write = originalWrite;
436435
sys.writeFile = originalWriteFile;
437436
return sys;
438437
}
438+
439+
export function createSystemWithSolutionBuild(solutionRoots: readonly string[], files: readonly TestFSWithWatch.FileOrFolderOrSymLink[], params?: TestFSWithWatch.TestServerHostCreationParameters) {
440+
return solutionBuildWithBaseline(createWatchedSystem(files, params), solutionRoots);
441+
}
439442
}

0 commit comments

Comments
 (0)