Skip to content
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.

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

Make sure virtual file system with watch behaves same way as sys/node so we have proper test coverage for symlinks #57607

Merged
merged 2 commits into from
Mar 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ tests/baselines/rwc/*
tests/baselines/reference/projectOutput/*
tests/baselines/local/projectOutput/*
tests/baselines/reference/testresults.tap
tests/baselines/symlinks/*
tests/services/baselines/prototyping/local/*
tests/services/browser/typescriptServices.js
src/harness/*.js
Expand Down
1 change: 1 addition & 0 deletions src/harness/harnessLanguageService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,7 @@ class SessionServerHost implements ts.server.ServerHost {
"watchedFiles",
"watchedDirectories",
ts.createGetCanonicalFileName(this.useCaseSensitiveFileNames),
this,
);

constructor(private host: NativeLanguageServiceHost) {
Expand Down
78 changes: 55 additions & 23 deletions src/harness/watchUtils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import {
addRange,
arrayFrom,
compareStringsCaseSensitive,
contains,
Expand All @@ -10,6 +9,7 @@ import {
GetCanonicalFileName,
MultiMap,
PollingInterval,
System,
} from "./_namespaces/ts";

export interface TestFileWatcher {
Expand All @@ -25,7 +25,7 @@ export interface TestFsWatcher<DirCallback> {
export interface Watches<Data> {
add(path: string, data: Data): void;
remove(path: string, data: Data): void;
forEach(path: string, cb: (data: Data) => void): void;
forEach(path: string, cb: (data: Data, path: string) => void): void;
serialize(baseline: string[]): void;
}

Expand All @@ -44,6 +44,7 @@ export function createWatchUtils<PollingWatcherData, FsWatcherData>(
pollingWatchesName: string,
fsWatchesName: string,
getCanonicalFileName: GetCanonicalFileName,
system: Required<Pick<System, "realpath">>,
): WatchUtils<PollingWatcherData, FsWatcherData> {
const pollingWatches = initializeWatches<PollingWatcherData>(pollingWatchesName);
const fsWatches = initializeWatches<FsWatcherData>(fsWatchesName);
Expand All @@ -64,6 +65,8 @@ export function createWatchUtils<PollingWatcherData, FsWatcherData>(
const actuals = createMultiMap<string, Data>();
let serialized: Map<string, Data[]> | undefined;
let canonicalPathsToStrings: Map<string, Set<string>> | undefined;
let realToLinked: MultiMap<string, string> | undefined;
let pathToReal: Map<string, string> | undefined;
return {
add,
remove,
Expand All @@ -73,40 +76,69 @@ export function createWatchUtils<PollingWatcherData, FsWatcherData>(

function add(path: string, data: Data) {
actuals.add(path, data);
if (actuals.get(path)!.length === 1) {
const canonicalPath = getCanonicalFileName(path);
if (canonicalPath !== path) {
(canonicalPathsToStrings ??= new Map()).set(
canonicalPath,
(canonicalPathsToStrings?.get(canonicalPath) ?? new Set()).add(path),
);
}
if (actuals.get(path)!.length !== 1) return;
const canonicalPath = getCanonicalFileName(path);
if (canonicalPath !== path) {
(canonicalPathsToStrings ??= new Map()).set(
canonicalPath,
(canonicalPathsToStrings?.get(canonicalPath) ?? new Set()).add(path),
);
}
const real = system.realpath(path);
(pathToReal ??= new Map()).set(path, real);
if (real === path) return;
const canonicalReal = getCanonicalFileName(real);
if (getCanonicalFileName(path) !== canonicalReal) {
(realToLinked ??= createMultiMap()).add(canonicalReal, path);
}
}

function remove(path: string, data: Data) {
actuals.remove(path, data);
if (!actuals.has(path)) {
const canonicalPath = getCanonicalFileName(path);
if (canonicalPath !== path) {
const existing = canonicalPathsToStrings!.get(canonicalPath);
if (existing!.size === 1) canonicalPathsToStrings!.delete(canonicalPath);
else existing!.delete(path);
}
if (actuals.has(path)) return;
const canonicalPath = getCanonicalFileName(path);
if (canonicalPath !== path) {
const existing = canonicalPathsToStrings!.get(canonicalPath);
if (existing!.size === 1) canonicalPathsToStrings!.delete(canonicalPath);
else existing!.delete(path);
}
const real = pathToReal?.get(path)!;
pathToReal!.delete(path);
if (real === path) return;
const canonicalReal = getCanonicalFileName(real);
if (getCanonicalFileName(path) !== canonicalReal) {
realToLinked!.remove(canonicalReal, path);
}
}

function forEach(path: string, cb: (data: Data) => void) {
let allData: Data[] | undefined;
allData = addRange(allData, actuals.get(path));
function getAllData(path: string) {
let allData: Map<string, Data[]> | undefined;
addData(path);
const canonicalPath = getCanonicalFileName(path);
if (canonicalPath !== path) allData = addRange(allData, actuals.get(canonicalPath));
if (canonicalPath !== path) addData(canonicalPath);
canonicalPathsToStrings?.get(canonicalPath)?.forEach(canonicalSamePath => {
if (canonicalSamePath !== path && canonicalSamePath !== canonicalPath) {
allData = addRange(allData, actuals.get(canonicalSamePath));
addData(canonicalSamePath);
}
});
allData?.forEach(cb);
return allData;
function addData(path: string) {
const data = actuals.get(path);
if (data) (allData ??= new Map()).set(path, data);
}
}

function forEach(path: string, cb: (data: Data, path: string) => void) {
const real = system.realpath(path);
const canonicalPath = getCanonicalFileName(path);
const canonicalReal = getCanonicalFileName(real);
let allData = canonicalPath === canonicalReal ? getAllData(path) : getAllData(real);
realToLinked?.get(canonicalReal)?.forEach(linked => {
if (allData?.has(linked)) return;
const data = actuals.get(linked);
if (data) (allData ??= new Map()).set(linked, data);
});
allData?.forEach((data, path) => data.forEach(d => cb(d, path)));
}

function serialize(baseline: string[]) {
Expand Down
1 change: 1 addition & 0 deletions src/testRunner/tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ import "./unittests/services/preProcessFile";
import "./unittests/services/textChanges";
import "./unittests/services/transpile";
import "./unittests/services/utilities";
import "./unittests/sys/symlinkWatching";
import "./unittests/tsbuild/amdModulesWithOut";
import "./unittests/tsbuild/clean";
import "./unittests/tsbuild/commandLine";
Expand Down
25 changes: 8 additions & 17 deletions src/testRunner/unittests/helpers/tscWatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ export interface TscWatchCompileChange<T extends ts.BuilderProgram = ts.EmitAndS
watchOrSolution: WatchOrSolution<T>,
) => void;
// TODO:: sheetal: Needing these fields are technically issues that need to be fixed later
symlinksNotReflected?: readonly string[];
skipStructureCheck?: true;
}
export interface TscWatchCheckOptions {
Expand Down Expand Up @@ -220,7 +219,7 @@ export function runWatchBaseline<T extends ts.BuilderProgram = ts.EmitAndSemanti
});

if (edits) {
for (const { caption, edit, timeouts, symlinksNotReflected, skipStructureCheck } of edits) {
for (const { caption, edit, timeouts, skipStructureCheck } of edits) {
applyEdit(sys, baseline, edit, caption);
timeouts(sys, programs, watchOrSolution);
programs = watchBaseline({
Expand All @@ -233,7 +232,6 @@ export function runWatchBaseline<T extends ts.BuilderProgram = ts.EmitAndSemanti
caption,
resolutionCache: !skipStructureCheck ? (watchOrSolution as ts.WatchOfConfigFile<T> | undefined)?.getResolutionCache?.() : undefined,
useSourceOfProjectReferenceRedirect,
symlinksNotReflected,
});
}
}
Expand All @@ -254,7 +252,6 @@ export interface WatchBaseline extends BaselineBase, TscWatchCheckOptions {
caption?: string;
resolutionCache?: ts.ResolutionCache;
useSourceOfProjectReferenceRedirect?: () => boolean;
symlinksNotReflected?: readonly string[];
}
export function watchBaseline({
baseline,
Expand All @@ -266,7 +263,6 @@ export function watchBaseline({
caption,
resolutionCache,
useSourceOfProjectReferenceRedirect,
symlinksNotReflected,
}: WatchBaseline) {
if (baselineSourceMap) generateSourceMapBaselineFiles(sys);
const programs = getPrograms();
Expand All @@ -279,7 +275,13 @@ export function watchBaseline({
// Verify program structure and resolution cache when incremental edit with tsc --watch (without build mode)
if (resolutionCache && programs.length) {
ts.Debug.assert(programs.length === 1);
verifyProgramStructureAndResolutionCache(caption!, sys, programs[0][0], resolutionCache, useSourceOfProjectReferenceRedirect, symlinksNotReflected);
verifyProgramStructureAndResolutionCache(
caption!,
sys,
programs[0][0],
resolutionCache,
useSourceOfProjectReferenceRedirect,
);
}
return programs;
}
Expand All @@ -289,23 +291,12 @@ function verifyProgramStructureAndResolutionCache(
program: ts.Program,
resolutionCache: ts.ResolutionCache,
useSourceOfProjectReferenceRedirect?: () => boolean,
symlinksNotReflected?: readonly string[],
) {
const options = program.getCompilerOptions();
const compilerHost = ts.createCompilerHostWorker(options, /*setParentNodes*/ undefined, sys);
compilerHost.trace = ts.noop;
compilerHost.writeFile = ts.notImplemented;
compilerHost.useSourceOfProjectReferenceRedirect = useSourceOfProjectReferenceRedirect;
const readFile = compilerHost.readFile;
compilerHost.readFile = fileName => {
const text = readFile.call(compilerHost, fileName);
if (!ts.contains(symlinksNotReflected, fileName)) return text;
// Handle symlinks that dont reflect the watch change
ts.Debug.assert(sys.toPath(sys.realpath(fileName)) !== sys.toPath(fileName));
const file = program.getSourceFile(fileName)!;
ts.Debug.assert(file.text !== text);
return file.text;
};
verifyProgramStructure(
ts.createProgram({
rootNames: program.getRootFileNames(),
Expand Down
Loading