Skip to content

Commit

Permalink
Fix: Windows path parsing for JUnit (fixes #12507) (#12509)
Browse files Browse the repository at this point in the history 
* Fix: Windows path parsing for JUnit (fixes #12507)

This commit aims to fix the issues described in #12507 by removing the
assumption that POSIX style filePaths are supplied by messages.

The tests for this are also updated to run on both Windows and POSIX
based systems. The results can be seen by running ESLint with the JUnit
formatter on a Windows system.

Full details can be seen in the issue mentioned above.

* Fix: use const instead of function

Addresses some comments.
  • Loading branch information
@thewalla07 @kaicataldo
thewalla07 authored and kaicataldo committed Dec 23, 2019
1 parent 985dac3 commit 00ddfff
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 21 deletions.
2 changes: 1 addition & 1 deletion lib/cli-engine/formatters/junit.js
View file
Expand Up @@ -32,7 +32,7 @@ function getMessageType(message) {
* @private
*/
function pathWithoutExt(filePath) {
return path.posix.join(path.posix.dirname(filePath), path.basename(filePath, path.extname(filePath)));
return path.join(path.dirname(filePath), path.basename(filePath, path.extname(filePath)));
}

//------------------------------------------------------------------------------
Expand Down
44 changes: 24 additions & 20 deletions tests/lib/cli-engine/formatters/junit.js
View file
Expand Up @@ -12,12 +12,16 @@
//------------------------------------------------------------------------------

const assert = require("chai").assert,
formatter = require("../../../../lib/cli-engine/formatters/junit");
formatter = require("../../../../lib/cli-engine/formatters/junit"),
process = require("process");

//------------------------------------------------------------------------------
// Tests
//------------------------------------------------------------------------------

const suppliedFilePath = (process.platform === "win32") ? "C:\\path\\to\\foo.js" : "/path/to/foo.js";
const expectedClassName = (process.platform === "win32") ? "C:\\path\\to\\foo" : "/path/to/foo";

describe("formatter:junit", () => {
describe("when there are no problems", () => {
const code = [];
Expand All @@ -31,7 +35,7 @@ describe("formatter:junit", () => {

describe("when passed a single message", () => {
const code = [{
filePath: "/path/to/foo.js",
filePath: suppliedFilePath,
messages: [{
message: "Unexpected foo.",
severity: 2,
Expand All @@ -44,20 +48,20 @@ describe("formatter:junit", () => {
it("should return a single <testcase> with a message and the line and col number in the body (error)", () => {
const result = formatter(code);

assert.strictEqual(result.replace(/\n/gu, ""), "<?xml version=\"1.0\" encoding=\"utf-8\"?><testsuites><testsuite package=\"org.eslint\" time=\"0\" tests=\"1\" errors=\"1\" name=\"/path/to/foo.js\"><testcase time=\"0\" name=\"org.eslint.foo\" classname=\"/path/to/foo\"><failure message=\"Unexpected foo.\"><![CDATA[line 5, col 10, Error - Unexpected foo. (foo)]]></failure></testcase></testsuite></testsuites>");
assert.strictEqual(result.replace(/\n/gu, ""), `<?xml version="1.0" encoding="utf-8"?><testsuites><testsuite package="org.eslint" time="0" tests="1" errors="1" name="${suppliedFilePath}"><testcase time="0" name="org.eslint.foo" classname="${expectedClassName}"><failure message="Unexpected foo."><![CDATA[line 5, col 10, Error - Unexpected foo. (foo)]]></failure></testcase></testsuite></testsuites>`);
});

it("should return a single <testcase> with a message and the line and col number in the body (warning)", () => {
code[0].messages[0].severity = 1;
const result = formatter(code);

assert.strictEqual(result.replace(/\n/gu, ""), "<?xml version=\"1.0\" encoding=\"utf-8\"?><testsuites><testsuite package=\"org.eslint\" time=\"0\" tests=\"1\" errors=\"1\" name=\"/path/to/foo.js\"><testcase time=\"0\" name=\"org.eslint.foo\" classname=\"/path/to/foo\"><failure message=\"Unexpected foo.\"><![CDATA[line 5, col 10, Warning - Unexpected foo. (foo)]]></failure></testcase></testsuite></testsuites>");
assert.strictEqual(result.replace(/\n/gu, ""), `<?xml version="1.0" encoding="utf-8"?><testsuites><testsuite package="org.eslint" time="0" tests="1" errors="1" name="${suppliedFilePath}"><testcase time="0" name="org.eslint.foo" classname="${expectedClassName}"><failure message="Unexpected foo."><![CDATA[line 5, col 10, Warning - Unexpected foo. (foo)]]></failure></testcase></testsuite></testsuites>`);
});
});

describe("when passed a fatal error message", () => {
const code = [{
filePath: "foo.js",
filePath: suppliedFilePath,
messages: [{
fatal: true,
message: "Unexpected foo.",
Expand All @@ -70,13 +74,13 @@ describe("formatter:junit", () => {
it("should return a single <testcase> and an <error>", () => {
const result = formatter(code);

assert.strictEqual(result.replace(/\n/gu, ""), "<?xml version=\"1.0\" encoding=\"utf-8\"?><testsuites><testsuite package=\"org.eslint\" time=\"0\" tests=\"1\" errors=\"1\" name=\"foo.js\"><testcase time=\"0\" name=\"org.eslint.foo\" classname=\"foo\"><error message=\"Unexpected foo.\"><![CDATA[line 5, col 10, Error - Unexpected foo. (foo)]]></error></testcase></testsuite></testsuites>");
assert.strictEqual(result.replace(/\n/gu, ""), `<?xml version="1.0" encoding="utf-8"?><testsuites><testsuite package="org.eslint" time="0" tests="1" errors="1" name="${suppliedFilePath}"><testcase time="0" name="org.eslint.foo" classname="${expectedClassName}"><error message="Unexpected foo."><![CDATA[line 5, col 10, Error - Unexpected foo. (foo)]]></error></testcase></testsuite></testsuites>`);
});
});

describe("when passed a fatal error message with no line or column", () => {
const code = [{
filePath: "foo.js",
filePath: suppliedFilePath,
messages: [{
fatal: true,
message: "Unexpected foo."
Expand All @@ -86,13 +90,13 @@ describe("formatter:junit", () => {
it("should return a single <testcase> and an <error>", () => {
const result = formatter(code);

assert.strictEqual(result.replace(/\n/gu, ""), "<?xml version=\"1.0\" encoding=\"utf-8\"?><testsuites><testsuite package=\"org.eslint\" time=\"0\" tests=\"1\" errors=\"1\" name=\"foo.js\"><testcase time=\"0\" name=\"org.eslint.unknown\" classname=\"foo\"><error message=\"Unexpected foo.\"><![CDATA[line 0, col 0, Error - Unexpected foo.]]></error></testcase></testsuite></testsuites>");
assert.strictEqual(result.replace(/\n/gu, ""), `<?xml version="1.0" encoding="utf-8"?><testsuites><testsuite package="org.eslint" time="0" tests="1" errors="1" name="${suppliedFilePath}"><testcase time="0" name="org.eslint.unknown" classname="${expectedClassName}"><error message="Unexpected foo."><![CDATA[line 0, col 0, Error - Unexpected foo.]]></error></testcase></testsuite></testsuites>`);
});
});

describe("when passed a fatal error message with no line, column, or message text", () => {
const code = [{
filePath: "foo.js",
filePath: suppliedFilePath,
messages: [{
fatal: true
}]
Expand All @@ -101,13 +105,13 @@ describe("formatter:junit", () => {
it("should return a single <testcase> and an <error>", () => {
const result = formatter(code);

assert.strictEqual(result.replace(/\n/gu, ""), "<?xml version=\"1.0\" encoding=\"utf-8\"?><testsuites><testsuite package=\"org.eslint\" time=\"0\" tests=\"1\" errors=\"1\" name=\"foo.js\"><testcase time=\"0\" name=\"org.eslint.unknown\" classname=\"foo\"><error message=\"\"><![CDATA[line 0, col 0, Error - ]]></error></testcase></testsuite></testsuites>");
assert.strictEqual(result.replace(/\n/gu, ""), `<?xml version="1.0" encoding="utf-8"?><testsuites><testsuite package="org.eslint" time="0" tests="1" errors="1" name="${suppliedFilePath}"><testcase time="0" name="org.eslint.unknown" classname="${expectedClassName}"><error message=""><![CDATA[line 0, col 0, Error - ]]></error></testcase></testsuite></testsuites>`);
});
});

describe("when passed multiple messages", () => {
const code = [{
filePath: "foo.js",
filePath: suppliedFilePath,
messages: [{
message: "Unexpected foo.",
severity: 2,
Expand All @@ -126,13 +130,13 @@ describe("formatter:junit", () => {
it("should return a multiple <testcase>'s", () => {
const result = formatter(code);

assert.strictEqual(result.replace(/\n/gu, ""), "<?xml version=\"1.0\" encoding=\"utf-8\"?><testsuites><testsuite package=\"org.eslint\" time=\"0\" tests=\"2\" errors=\"2\" name=\"foo.js\"><testcase time=\"0\" name=\"org.eslint.foo\" classname=\"foo\"><failure message=\"Unexpected foo.\"><![CDATA[line 5, col 10, Error - Unexpected foo. (foo)]]></failure></testcase><testcase time=\"0\" name=\"org.eslint.bar\" classname=\"foo\"><failure message=\"Unexpected bar.\"><![CDATA[line 6, col 11, Warning - Unexpected bar. (bar)]]></failure></testcase></testsuite></testsuites>");
assert.strictEqual(result.replace(/\n/gu, ""), `<?xml version="1.0" encoding="utf-8"?><testsuites><testsuite package="org.eslint" time="0" tests="2" errors="2" name="${suppliedFilePath}"><testcase time="0" name="org.eslint.foo" classname="${expectedClassName}"><failure message="Unexpected foo."><![CDATA[line 5, col 10, Error - Unexpected foo. (foo)]]></failure></testcase><testcase time="0" name="org.eslint.bar" classname="${expectedClassName}"><failure message="Unexpected bar."><![CDATA[line 6, col 11, Warning - Unexpected bar. (bar)]]></failure></testcase></testsuite></testsuites>`);
});
});

describe("when passed special characters", () => {
const code = [{
filePath: "foo.js",
filePath: suppliedFilePath,
messages: [{
message: "Unexpected <foo></foo>\b\t\n\f\r牛逼.",
severity: 1,
Expand All @@ -145,13 +149,13 @@ describe("formatter:junit", () => {
it("should make them go away", () => {
const result = formatter(code);

assert.strictEqual(result.replace(/\n/gu, ""), "<?xml version=\"1.0\" encoding=\"utf-8\"?><testsuites><testsuite package=\"org.eslint\" time=\"0\" tests=\"1\" errors=\"1\" name=\"foo.js\"><testcase time=\"0\" name=\"org.eslint.foo\" classname=\"foo\"><failure message=\"Unexpected &lt;foo&gt;&lt;/foo&gt;&#8;&#9;&#10;&#12;&#13;&#29275;&#36924;.\"><![CDATA[line 5, col 10, Warning - Unexpected &lt;foo&gt;&lt;/foo&gt;&#8;&#9;&#10;&#12;&#13;&#29275;&#36924;. (foo)]]></failure></testcase></testsuite></testsuites>");
assert.strictEqual(result.replace(/\n/gu, ""), `<?xml version="1.0" encoding="utf-8"?><testsuites><testsuite package="org.eslint" time="0" tests="1" errors="1" name="${suppliedFilePath}"><testcase time="0" name="org.eslint.foo" classname="${expectedClassName}"><failure message="Unexpected &lt;foo&gt;&lt;/foo&gt;&#8;&#9;&#10;&#12;&#13;&#29275;&#36924;."><![CDATA[line 5, col 10, Warning - Unexpected &lt;foo&gt;&lt;/foo&gt;&#8;&#9;&#10;&#12;&#13;&#29275;&#36924;. (foo)]]></failure></testcase></testsuite></testsuites>`);
});
});

describe("when passed multiple files with 1 message each", () => {
const code = [{
filePath: "foo.js",
filePath: suppliedFilePath,
messages: [{
message: "Unexpected foo.",
severity: 1,
Expand All @@ -173,13 +177,13 @@ describe("formatter:junit", () => {
it("should return 2 <testsuite>'s", () => {
const result = formatter(code);

assert.strictEqual(result.replace(/\n/gu, ""), "<?xml version=\"1.0\" encoding=\"utf-8\"?><testsuites><testsuite package=\"org.eslint\" time=\"0\" tests=\"1\" errors=\"1\" name=\"foo.js\"><testcase time=\"0\" name=\"org.eslint.foo\" classname=\"foo\"><failure message=\"Unexpected foo.\"><![CDATA[line 5, col 10, Warning - Unexpected foo. (foo)]]></failure></testcase></testsuite><testsuite package=\"org.eslint\" time=\"0\" tests=\"1\" errors=\"1\" name=\"bar.js\"><testcase time=\"0\" name=\"org.eslint.bar\" classname=\"bar\"><failure message=\"Unexpected bar.\"><![CDATA[line 6, col 11, Error - Unexpected bar. (bar)]]></failure></testcase></testsuite></testsuites>");
assert.strictEqual(result.replace(/\n/gu, ""), `<?xml version="1.0" encoding="utf-8"?><testsuites><testsuite package="org.eslint" time="0" tests="1" errors="1" name="${suppliedFilePath}"><testcase time="0" name="org.eslint.foo" classname="${expectedClassName}"><failure message="Unexpected foo."><![CDATA[line 5, col 10, Warning - Unexpected foo. (foo)]]></failure></testcase></testsuite><testsuite package="org.eslint" time="0" tests="1" errors="1" name="bar.js"><testcase time="0" name="org.eslint.bar" classname="bar"><failure message="Unexpected bar."><![CDATA[line 6, col 11, Error - Unexpected bar. (bar)]]></failure></testcase></testsuite></testsuites>`);
});
});

describe("when passed multiple files should print even if no errors", () => {
const code = [{
filePath: "foo.js",
filePath: suppliedFilePath,
messages: [{
message: "Unexpected foo.",
severity: 1,
Expand All @@ -195,20 +199,20 @@ describe("formatter:junit", () => {
it("should return 2 <testsuite>", () => {
const result = formatter(code);

assert.strictEqual(result.replace(/\n/gu, ""), "<?xml version=\"1.0\" encoding=\"utf-8\"?><testsuites><testsuite package=\"org.eslint\" time=\"0\" tests=\"1\" errors=\"1\" name=\"foo.js\"><testcase time=\"0\" name=\"org.eslint.foo\" classname=\"foo\"><failure message=\"Unexpected foo.\"><![CDATA[line 5, col 10, Warning - Unexpected foo. (foo)]]></failure></testcase></testsuite><testsuite package=\"org.eslint\" time=\"0\" tests=\"1\" errors=\"0\" name=\"bar.js\"><testcase time=\"0\" name=\"bar.js\" classname=\"bar\" /></testsuite></testsuites>");
assert.strictEqual(result.replace(/\n/gu, ""), `<?xml version="1.0" encoding="utf-8"?><testsuites><testsuite package="org.eslint" time="0" tests="1" errors="1" name="${suppliedFilePath}"><testcase time="0" name="org.eslint.foo" classname="${expectedClassName}"><failure message="Unexpected foo."><![CDATA[line 5, col 10, Warning - Unexpected foo. (foo)]]></failure></testcase></testsuite><testsuite package="org.eslint" time="0" tests="1" errors="0" name="bar.js"><testcase time="0" name="bar.js" classname="bar" /></testsuite></testsuites>`);
});
});

describe("when passed a file with no errors", () => {
const code = [{
filePath: "foo.js",
filePath: suppliedFilePath,
messages: []
}];

it("should print a passing <testcase>", () => {
const result = formatter(code);

assert.strictEqual(result.replace(/\n/gu, ""), "<?xml version=\"1.0\" encoding=\"utf-8\"?><testsuites><testsuite package=\"org.eslint\" time=\"0\" tests=\"1\" errors=\"0\" name=\"foo.js\"><testcase time=\"0\" name=\"foo.js\" classname=\"foo\" /></testsuite></testsuites>");
assert.strictEqual(result.replace(/\n/gu, ""), `<?xml version="1.0" encoding="utf-8"?><testsuites><testsuite package="org.eslint" time="0" tests="1" errors="0" name="${suppliedFilePath}"><testcase time="0" name="${suppliedFilePath}" classname="${expectedClassName}" /></testsuite></testsuites>`);
});
});
});

0 comments on commit 00ddfff

Please sign in to comment.