Update harness to use single robust virtual file system for tests. #20763

rbuckton · 2017-12-18T20:42:12Z

To better support the lib reference PR (#15780) and the ES3 lib PR (#16077) we needed to make a number of changes to our test harness to improve and consolidate our various approaches to emulating a file system. To that end, this PR introduces a much more robust virtual file system implementation that supports additional features needed by our various test runners including:

Symbolic links
~~File/Directory watching~~
File stats
Resolving paths/files to the real file system
File/Directory metadata
VFS shadowing (allows the use of a shared VFS where changes in a shadowed VFS do not affect the real VFS)

As of this PR, the Compiler/Conformance, Project, and FourSlash runners all now use the VFS. ~~In addition all existing unit tests that used "virtualFileSystemWithWatch" have been migrated over to the single VFS implementation.~~

RyanCavanaugh · 2017-12-18T20:48:57Z

Can we do a one-time hack to make the 1,400ish baseline diffs go away, then do a separate PR to un-hack that so we can be sure no baselines are changing unexpectedly due to the VFS?

weswigham · 2017-12-18T21:38:13Z

src/harness/runner.ts

@@ -200,7 +196,7 @@ function handleTestConfig() {

        // TODO: project tests don"t work in the browser yet


Is this still the case? Both the projects runner and browser IO impl got major overhauls.

weswigham · 2017-12-18T21:42:23Z

src/harness/harness.ts

+                programFileNames,
+                options);
+
+            // const fileOutputs = compilation.outputs.map(output => output.asGeneratedFile());


Is the commented-out code here required in some way?

Not anymore, I'll remove it.

mhegazy · 2018-04-20T22:45:32Z

src/harness/compiler.ts

@@ -0,0 +1,256 @@
+/// <reference path="./harness.ts" />


we are removing all triple-slash references currently in favor of tsconfig.josn file list

mhegazy · 2018-04-20T22:47:25Z

src/harness/core/comparers.ts

+        return a < b ? -1 : +1;
+    }
+
+    export function compareStrings(a: string, b: string, ignoreCase: boolean): number {


how are these diffrent from the ones in src\compiler\core?

They aren't. Originally I intended to attempt to divorce the harness from the compiler to improve time-to-test by reducing compilation time, but with @RyanCavanaugh's forthcoming changes for project references this is no longer an issue. I am in the process of switching over to using existing functions, which should show up in the next few commits.

mhegazy · 2018-04-20T22:48:27Z

src/harness/core/functions.ts

@@ -0,0 +1,3 @@
+namespace core {
+    export function identity<T>(v: T): T { return v; }


i think we have this in core already.

Planning to remove. See #20763 (comment) for more information.

mhegazy · 2018-04-20T22:49:15Z

src/harness/core/strings.ts

+                    break;
+
+                // WhiteSpace
+                case 0x0009: // <TAB> tab


isWhiteSpaceSingleLine?

This function will be removed. See #20763 (comment) for more information.

mhegazy · 2018-04-20T22:49:43Z

src/harness/core/strings.ts

+        return lines;
+    }
+
+    export function computeLineStarts(text: string): LineStarts {


we should already have this in scanner?

Planning to remove. See #20763 (comment) for more information.

mhegazy · 2018-04-20T23:05:05Z

src/harness/core.ts

@@ -0,0 +1,4 @@
+/// <reference path="./core/functions.ts" />
+/// <reference path="./core/comparers.ts" />


i would put these in tsconfig.json instead.

mhegazy · 2018-04-20T23:10:08Z

src/harness/vpath.ts

+    /**
+     * Combines two or more paths. If a path is absolute, it replaces any previous path.
+     */
+    export function combine(path: string, ...paths: string[]) {


many of the helpers here have counterparts in core, can we reuse?

Yes, where possible. I like the logical grouping of path-related functionality in the vpath namespace (along with the more concise naming), and wonder if it would make sense to similarly consolidate related functionality in the compiler to reduce the volume of functions that show up in completions.

that is fine, but as we chatted offline, i think this restructuring should be done in the core, and not in the tests, and not by introducing a set of duplicate implementations.

I've removed most of the strict duplications already. What remains are places where the implementations are divergent. When you say "core", are you speaking of a new src/core folder for those shared dependencies or refactoring existing functions in src/compiler/core.ts to support the needs of both compiler and harness? I haven't made this change yet as I didn't want to bloat compiler with harness-specific functionality unnecessarily.

mhegazy · 2018-04-24T16:06:25Z

src/harness/core/collections.ts

+        }
+    }
+
+    export class SortedSet<T> {


i am not seeing this used anywhere.. is it needed?

It was originally used as part of the file watcher functionality.

rbuckton · 2018-04-24T21:13:33Z

@mhegazy, can you take another look?

sheetalkamat · 2018-04-27T19:13:50Z

src/harness/fakes.ts

+        private readonly _executingFilePath: string | undefined;
+        private readonly _env: Record<string, string> | undefined;
+
+        constructor(vfs: vfs.FileSystem, { executingFilePath, newLine = "\r\n", env }: SystemOptions = {}) {


Why do you have this another layer? Why not make vfs.FileSystem implement ts.System ?

I'd rather keep them separate so that FileSystem is purely a file system abstraction. Breaking things down into multiple components is more maintainable and better delineates functionality. We already have too many monolithic dependencies in the compiler, and we don't always need a full System implementation (which is why we have so many independent *Host types).

sheetalkamat · 2018-04-27T19:15:01Z

src/harness/fakes.ts

+     * A fake `ts.CompilerHost` that leverages a virtual file system.
+     */
+    export class CompilerHost implements ts.CompilerHost {
+        public readonly sys: System;


Also this one.. Seems like this should just use createCompilerHost API on system instead of having another abstraction.

Except we need to customize the compiler host for certain operations:

Handling lib resolution in getDefaultLibFileName

SourceFile caching for test performance in getSourceFile

Keep track of outputs in writeFile

Honestly, it'd be nice if fakes.CompilerHost could inherit from fakes.System except they differ in their definitions of useCaseSensitiveFileNames.

rbuckton · 2018-05-01T21:01:52Z

I have a few more changes pending, as I've almost finished unifying the path logic between vpath and core.

rbuckton · 2018-05-02T02:05:40Z

@mhegazy can you take one more look after the last set of changes. I've cleaned up our path handling logic and was able to unify most of the path handling features in vpath and core.

rbuckton added 24 commits October 10, 2017 15:53

Updated vfs

5b4553d

Cleanup, add watch capabilities

41aeaae

Minor cleanup

560cffd

Minor cleanup

db6794a

Merge branch 'master' into vfs

8ff9520

Cleanup after merge

8a70e17

Cache comparers

07bb677

Merge branch 'master' into vfs

7d08d74

Update Harness.compileFiles to use vfs

8245f62

Retire Harness.compileFiles

cf261c3

Add support for variations in module/target in compiler tests

4697ba3

Deprecate CompilerResult

cd0809e

Deprecate GeneratedFile

791c01e

Merge branch 'master' into vfs

646c32c

Partial deprecation of non-vfs TestServerHost

4fbc74e

Migrate compileOnSave tests to vfs

3d3977f

Migrate tscWatchMode to vfs

41567b2

Switch to function spies

fa42835

Migrate tsserverProjectSystem to vfs

c5e5020

Deprecate virtualFileSystemWithWatch

c3701df

Update projects tests to use vfs

1cb1088

Re-enable all tests

6727499

Merge branch 'master' into vfs

7944f50

Accept baselines

279b348

rbuckton requested review from weswigham, mhegazy and sheetalkamat December 18, 2017 20:42

weswigham reviewed Dec 18, 2017

View reviewed changes

mhegazy reviewed Apr 20, 2018

View reviewed changes

rbuckton added 2 commits April 23, 2018 21:52

Fix issues with runtests-browser

e422365

Merge branch 'master' into vfs

bb26ab5

mhegazy reviewed Apr 24, 2018

View reviewed changes

Reorganize and remove duplication

c9c562a

sheetalkamat reviewed Apr 27, 2018

View reviewed changes

rbuckton added 2 commits April 27, 2018 13:58

Partial migration of some shared vpath functionality to core

8424c4d

Merge branch 'master' into vfs

4863d55

mhegazy approved these changes May 1, 2018

View reviewed changes

rbuckton added 3 commits May 1, 2018 18:53

More path cleanup

1dff0af

Merge branch 'master' into vfs

d62a11f

Update documentation comments

860f9f8

Update completion order in test

66c11c5

rbuckton force-pushed the vfs branch from 6981da7 to 66c11c5 Compare May 2, 2018 17:51

mhegazy approved these changes May 3, 2018

View reviewed changes

rbuckton merged commit 56648ad into master May 3, 2018

rbuckton deleted the vfs branch May 3, 2018 17:25

ghost mentioned this pull request May 3, 2018

Delay in runtests-browser #23877

Closed

rbuckton mentioned this pull request May 3, 2018

New vfs baselines follow-up #23878

Merged

microsoft locked and limited conversation to collaborators Jul 31, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update harness to use single robust virtual file system for tests. #20763

Update harness to use single robust virtual file system for tests. #20763

rbuckton commented Dec 18, 2017 •

edited

Loading

RyanCavanaugh commented Dec 18, 2017

weswigham Dec 18, 2017

weswigham Dec 18, 2017

rbuckton Dec 19, 2017

mhegazy Apr 20, 2018

mhegazy Apr 20, 2018

rbuckton Apr 24, 2018

mhegazy Apr 20, 2018

rbuckton Apr 24, 2018

mhegazy Apr 20, 2018

rbuckton Apr 24, 2018

mhegazy Apr 20, 2018

rbuckton Apr 24, 2018

mhegazy Apr 20, 2018

mhegazy Apr 20, 2018

rbuckton Apr 24, 2018

mhegazy Apr 25, 2018

rbuckton Apr 25, 2018

mhegazy Apr 24, 2018

rbuckton Apr 24, 2018

rbuckton commented Apr 24, 2018

sheetalkamat Apr 27, 2018

rbuckton Apr 27, 2018

sheetalkamat Apr 27, 2018

rbuckton Apr 27, 2018 •

edited

Loading

rbuckton Apr 27, 2018

rbuckton commented May 1, 2018

rbuckton commented May 2, 2018

		@@ -200,7 +196,7 @@ function handleTestConfig() {

		// TODO: project tests don"t work in the browser yet

		@@ -0,0 +1,3 @@
		namespace core {
		export function identity<T>(v: T): T { return v; }

		@@ -0,0 +1,4 @@
		/// <reference path="./core/functions.ts" />
		/// <reference path="./core/comparers.ts" />

Update harness to use single robust virtual file system for tests. #20763

Update harness to use single robust virtual file system for tests. #20763

Conversation

rbuckton commented Dec 18, 2017 • edited Loading

RyanCavanaugh commented Dec 18, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rbuckton commented Apr 24, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rbuckton Apr 27, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rbuckton commented May 1, 2018

rbuckton commented May 2, 2018

rbuckton commented Dec 18, 2017 •

edited

Loading

rbuckton Apr 27, 2018 •

edited

Loading