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.

Sign up for GitHub

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

Jump to bottom

Fix: newBasePath should be an absolute path (fixes #12850) #13078

Merged
merged 3 commits into from Apr 8, 2020
Merged

Fix: newBasePath should be an absolute path (fixes #12850) #13078

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0e25381
Fix: newBasePath should be an absolute path (fixes #12850)
nickharris Mar 24, 2020
e149f99
review feedback
nickharris Mar 25, 2020
2ba15ba
PR feeedback, add comments to test
nickharris Apr 3, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 7 additions & 1 deletion lib/cli-engine/config-array/ignore-pattern.js
View file
Open in desktop
Expand Up @@ -71,7 +71,13 @@ function getCommonAncestorPath(sourcePaths) {
}
}

return result || path.sep;
let resolvedResult = result || path.sep;

// if Windows common ancestor is root of drive must have trailing slash to be absolute.
if (resolvedResult && resolvedResult.endsWith(":") && process.platform === "win32") {
resolvedResult += path.sep;
}
return resolvedResult;
}

/**
Expand Down
178 changes: 116 additions & 62 deletions tests/lib/cli-engine/config-array/ignore-pattern.js
View file
Open in desktop
Expand Up @@ -6,6 +6,7 @@

const assert = require("assert");
const path = require("path");
const sinon = require("sinon");
const { IgnorePattern } = require("../../../../lib/cli-engine/config-array/ignore-pattern");

describe("IgnorePattern", () => {
Expand Down Expand Up @@ -52,74 +53,127 @@ describe("IgnorePattern", () => {

describe("static createIgnore(ignorePatterns)", () => {
describe("with two patterns should return a function, and the function", () => {
const cwd = process.cwd();
const basePath1 = path.join(cwd, "foo/bar");
const basePath2 = path.join(cwd, "abc/");
const ignores = IgnorePattern.createIgnore([
new IgnorePattern(["*.js", "/*.ts", "!a.*", "!/b.*"], basePath1),
new IgnorePattern(["*.js", "/*.ts", "!a.*", "!/b.*"], basePath2)
]);
const patterns = [
["a.js", false],
["a.ts", false],
["b.js", false],
["b.ts", false],
["c.js", false],
["c.ts", false],
["dir/a.js", false],
["dir/a.ts", false],
["dir/b.js", false],
["dir/b.ts", false],
["dir/c.js", false],
["dir/c.ts", false],
["foo/bar/a.js", false],
["foo/bar/a.ts", false],
["foo/bar/b.js", false],
["foo/bar/b.ts", false],
["foo/bar/c.js", true],
["foo/bar/c.ts", true],
["foo/bar/dir/a.js", false],
["foo/bar/dir/a.ts", false],
["foo/bar/dir/b.js", true],
["foo/bar/dir/b.ts", false],
["foo/bar/dir/c.js", true],
["foo/bar/dir/c.ts", false],
["abc/a.js", false],
["abc/a.ts", false],
["abc/b.js", false],
["abc/b.ts", false],
["abc/c.js", true],
["abc/c.ts", true],
["abc/dir/a.js", false],
["abc/dir/a.ts", false],
["abc/dir/b.js", true],
["abc/dir/b.ts", false],
["abc/dir/c.js", true],
["abc/dir/c.ts", false]
];

for (const [filename, expected] of patterns) {
it(`should return ${expected} if '${filename}' was given.`, () => {
assert.strictEqual(ignores(path.join(cwd, filename)), expected);

/**
* performs static createIgnre assertions against the cwd.
* @param {string} cwd cwd to be the base path for assertions
* @returns {void}
*/
function assertions(cwd) {
const basePath1 = path.join(cwd, "foo/bar");
const basePath2 = path.join(cwd, "abc/");
const ignores = IgnorePattern.createIgnore([
new IgnorePattern(["*.js", "/*.ts", "!a.*", "!/b.*"], basePath1),
new IgnorePattern(["*.js", "/*.ts", "!a.*", "!/b.*"], basePath2)
]);
const patterns = [
["a.js", false],
["a.ts", false],
["b.js", false],
["b.ts", false],
["c.js", false],
["c.ts", false],
["dir/a.js", false],
["dir/a.ts", false],
["dir/b.js", false],
["dir/b.ts", false],
["dir/c.js", false],
["dir/c.ts", false],
["foo/bar/a.js", false],
["foo/bar/a.ts", false],
["foo/bar/b.js", false],
["foo/bar/b.ts", false],
["foo/bar/c.js", true],
["foo/bar/c.ts", true],
["foo/bar/dir/a.js", false],
["foo/bar/dir/a.ts", false],
["foo/bar/dir/b.js", true],
["foo/bar/dir/b.ts", false],
["foo/bar/dir/c.js", true],
["foo/bar/dir/c.ts", false],
["abc/a.js", false],
["abc/a.ts", false],
["abc/b.js", false],
["abc/b.ts", false],
["abc/c.js", true],
["abc/c.ts", true],
["abc/dir/a.js", false],
["abc/dir/a.ts", false],
["abc/dir/b.js", true],
["abc/dir/b.ts", false],
["abc/dir/c.js", true],
["abc/dir/c.ts", false]
];

for (const [filename, expected] of patterns) {
it(`should return ${expected} if '${filename}' was given.`, () => {
assert.strictEqual(ignores(path.join(cwd, filename)), expected);
});
}

it("should return false if '.dot.js' and false was given.", () => {
assert.strictEqual(ignores(path.join(cwd, ".dot.js"), false), true);
});
}

it("should return false if '.dot.js' and false was given.", () => {
assert.strictEqual(ignores(path.join(cwd, ".dot.js"), false), true);
});
it("should return true if '.dot.js' and true were given.", () => {
assert.strictEqual(ignores(path.join(cwd, ".dot.js"), true), false);
});

it("should return true if '.dot.js' and true were given.", () => {
assert.strictEqual(ignores(path.join(cwd, ".dot.js"), true), false);
});
it("should return false if '.dot/foo.js' and false was given.", () => {
assert.strictEqual(ignores(path.join(cwd, ".dot/foo.js"), false), true);
});

it("should return true if '.dot/foo.js' and true were given.", () => {
assert.strictEqual(ignores(path.join(cwd, ".dot/foo.js"), true), false);
});
}

it("should return false if '.dot/foo.js' and false was given.", () => {
assert.strictEqual(ignores(path.join(cwd, ".dot/foo.js"), false), true);
});
assertions(process.cwd());
nickharris marked this conversation as resolved.
Show resolved Hide resolved

/*
* This will catch regressions of Windows specific issue #12850 when run on CI nodes.
* This runs the full set of assertions for the function returned from IgnorePattern.createIgnore.
* When run on Windows CI nodes the .root drive i.e C:\ will be supplied
* forcing getCommonAncestors to resolve to the root of the drive thus catching any regrssion of 12850.
* When run on *nix CI nodes provides additional coverage on this OS too.
* assertions when run on Windows CI nodes and / on *nix OS
*/
assertions(path.parse(process.cwd()).root);
});
});

it("should return true if '.dot/foo.js' and true were given.", () => {
assert.strictEqual(ignores(path.join(cwd, ".dot/foo.js"), true), false);
});
describe("static createIgnore(ignorePatterns)", () => {

/*
* This test will catch regressions of Windows specific issue #12850 when run on your local dev box
* irrespective of if you are running a Windows or *nix style OS.
* When running on *nix sinon is used to emulate Windows behaviors of path and platform APIs
* thus ensuring that the Windows specific fix is exercised and any regression is caught.
*/
it("with common ancestor of drive root on windows should not throw", () => {
try {

/*
* When not on Windows return win32 values so local runs on *nix hit the same code path as on Windows
* thus enabling developers with *nix style OS to catch and debug any future regression of #12850 without
* requiring a Windows based OS.
*/
if (process.platform !== "win32") {
nickharris marked this conversation as resolved.
Show resolved Hide resolved
sinon.stub(process, "platform").value("win32");
sinon.stub(path, "sep").value(path.win32.sep);
sinon.replace(path, "isAbsolute", path.win32.isAbsolute);
}

const ignores = IgnorePattern.createIgnore([
new IgnorePattern(["*.js"], "C:\\foo\\bar"),
new IgnorePattern(["*.js"], "C:\\abc\\")
]);

// calls to this should not throw when getCommonAncestor returns root of drive
ignores("C:\\abc\\contract.d.ts");
} finally {
sinon.restore();
}
});
});
});