Skip to content

Add vfs diff, update tsbuild test#24920

Merged
rbuckton merged 3 commits into
masterfrom
vfsDiff
Jun 13, 2018
Merged

Add vfs diff, update tsbuild test#24920
rbuckton merged 3 commits into
masterfrom
vfsDiff

Conversation

@rbuckton
Copy link
Copy Markdown
Contributor

With the recent change to project references, @weswigham and I noticed a few issues with sourcemap emit. To better diagnose these, I've added the ability to generate a diff between two virtual filesystems so that we can get a better idea of what changes we are making. This change will help diagnose source map paths in #24917.

Copy link
Copy Markdown
Member

@weswigham weswigham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some nits; I like the new unified emit diff. It's nice - all the emit in one place, so I can associate all the outputs. We should consider swapping our other baselines to it, too (provided none of the test emit outputs are so large it's untenable).

Comment thread src/testRunner/unittests/tsbuild.ts Outdated
}
for (const dir of Harness.IO.getDirectories(localRoot)) {
loadFsMirror(vfs, localRoot + "/" + dir, virtualRoot + "/" + dir);
function patchResolver(resolver: vfs.FileSystemResolver): vfs.FileSystemResolver {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this absolutely necessary? The mounted files are lazily loaded now, right? And only diffs are in the output? What still depends on only these lib files being findable?

Comment thread package.json Outdated
"@types/minimist": "latest",
"@types/mkdirp": "latest",
"@types/mocha": "latest",
"@types/mocha": "^5.2.2",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably still be latest.

Comment thread src/harness/vfs.ts

private static symlinkDiff(container: FileSet, basename: string, changedNode: SymlinkInode, baseNode: SymlinkInode) {
// no difference if the nodes are the same reference
if (changedNode.symlink === baseNode.symlink) return false;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If symlinks are interpreted as relative paths, should a symlink at /lib/foo of ../bar be equivalent to one at the same location of ../../lib/bar? Or am I mistaken?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This compares symlinks in the same path, so you shouldn't run into this.

Comment thread src/harness/vfs.ts Outdated
private _unlinkBrand?: never; // brand necessary for proper type guards
}

// prevents compiler errors due to unused brand properties
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we have some _unlinkBrand?! or ["_unlinkBrand"] syntax variant that should more cleanly suppress that error? Or is that just definite assignment?

@rbuckton rbuckton merged commit 6e570e3 into master Jun 13, 2018
@weswigham weswigham deleted the vfsDiff branch June 13, 2018 18:28
@microsoft microsoft locked as resolved and limited conversation to collaborators Oct 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants