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

Centralize @babel/eslint-* tests #11106

Merged
merged 3 commits into from Feb 8, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
3 changes: 1 addition & 2 deletions .eslintignore
Expand Up @@ -26,5 +26,4 @@ packages/babel-parser/test/expressions

eslint/*/lib
eslint/*/node_modules
eslint/*/test
eslint/*/tests
eslint/*/test/fixtures
6 changes: 3 additions & 3 deletions eslint/babel-eslint-parser/package.json
Expand Up @@ -28,10 +28,10 @@
"semver": "^6.3.0"
},
"devDependencies": {
"@babel/eslint-shared-fixtures": "*",
"@babel/core": "^7.2.0",
"dedent": "^0.7.0",
"@babel/eslint-shared-fixtures": "*",
"eslint": "^6.0.1",
"espree": "^6.0.0"
"espree": "^6.0.0",
"lodash.clonedeep": "^4.5.0"
}
}

This file was deleted.

5 changes: 0 additions & 5 deletions eslint/babel-eslint-plugin/test/rules/no-invalid-this.js
Expand Up @@ -77,11 +77,6 @@ function extractPatterns(patterns, type) {
// Tests
//------------------------------------------------------------------------------

const errors = [
{ message: "Unexpected 'this'.", type: "ThisExpression" },
{ message: "Unexpected 'this'.", type: "ThisExpression" },
];

const patterns = [
// Class private fields
{
Expand Down
15 changes: 15 additions & 0 deletions eslint/babel-eslint-tests/package.json
@@ -0,0 +1,15 @@
{
"name": "@babel/eslint-tests",
"version": "0.0.0",
"description": "Tests for babel/eslint-* packages",
"license": "MIT",
"private": true,
"devDependencies": {
"@babel/eslint-parser": "*",
"@babel/eslint-plugin": "*",
"@babel/eslint-shared-fixtures": "*",
"dedent": "^0.7.0",
"eslint": "^6.0.0",
"eslint-plugin-import": "^2.20.1"
}
}
@@ -1,9 +1,5 @@
root: true

# babel-eslint
parser: ../../../lib/index.js

# use eslint-plugin-import
parser: "@babel/eslint-parser"
plugins:
- import
rules:
Expand Down
69 changes: 69 additions & 0 deletions eslint/babel-eslint-tests/test/helpers/verifyAndAssertMessages.js
@@ -0,0 +1,69 @@
import eslint from "eslint";
import unpad from "dedent";
import * as parser from "@babel/eslint-parser";

export default function verifyAndAssertMessages(
code,
rules = {},
expectedMessages = [],
sourceType,
overrideConfig,
) {
const linter = new eslint.Linter();
linter.defineParser("@babel/eslint-parser", parser);

const messages = linter.verify(unpad(`${code}`), {
parser: "@babel/eslint-parser",
rules,
env: {
node: true,
es6: true,
},
...overrideConfig,
parserOptions: {
sourceType,
requireConfigFile: false,
babelOptions: {
configFile: require.resolve(
"@babel/eslint-shared-fixtures/config/babel.config.js",
),
},
...overrideConfig?.parserOptions,
},
});

if (messages.length !== expectedMessages.length) {
throw new Error(
`Expected ${expectedMessages.length} message(s), got ${
messages.length
}\n${JSON.stringify(messages, null, 2)}`,
);
}

messages.forEach((message, i) => {
const formattedMessage = `${message.line}:${message.column} ${
message.message
}${message.ruleId ? ` ${message.ruleId}` : ""}`;
const expectedMessage = expectedMessages[i];

if (expectedMessage instanceof RegExp) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Added the ability to pass this function a RegExp so we can enable the xited test below.

Copy link
Member

Choose a reason for hiding this comment

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

Can we use jest's methods instead of writing our own errors checking logic?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that would be great. Mind if we do that in a follow up PR?

Copy link
Member

Choose a reason for hiding this comment

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

It's ok!

if (!expectedMessage.test(formattedMessage)) {
throw new Error(
`
Message ${i} does not pass RegExp test:
Test: ${expectedMessage}
Actual: ${formattedMessage}
`,
);
}
} else if (formattedMessage !== expectedMessage) {
throw new Error(
`
Message ${i} does not match:
Expected: ${expectedMessage}
Actual: ${formattedMessage}
`,
);
}
});
}
13 changes: 13 additions & 0 deletions eslint/babel-eslint-tests/test/integration/eslint-plugin-import.js
@@ -0,0 +1,13 @@
import eslint from "eslint";
import path from "path";

describe("https://github.com/babel/babel-eslint/issues/558", () => {
it("doesn't crash with eslint-plugin-import", () => {
const engine = new eslint.CLIEngine({ ignore: false });
engine.executeOnFiles(
["a.js", "b.js", "c.js"].map(file =>
path.resolve(__dirname, `../fixtures/eslint-plugin-import/${file}`),
),
);
});
});
@@ -1,81 +1,4 @@
import eslint from "eslint";
import path from "path";
import unpad from "dedent";
import * as parser from "../src";

function verifyAndAssertMessagesWithSpecificESLint(
code,
rules,
expectedMessages,
sourceType,
overrideConfig,
linter,
) {
const config = {
parser: "current-babel-eslint",
rules,
env: {
node: true,
es6: true,
},
...overrideConfig,
parserOptions: {
sourceType,
requireConfigFile: false,
babelOptions: {
configFile: require.resolve(
"@babel/eslint-shared-fixtures/config/babel.config.js",
),
},
...overrideConfig?.parserOptions,
},
};

const messages = linter.verify(code, config);

if (messages.length !== expectedMessages.length) {
throw new Error(
`Expected ${expectedMessages.length} message(s), got ${
messages.length
}\n${JSON.stringify(messages, null, 2)}`,
);
}

messages.forEach((message, i) => {
const formatedMessage = `${message.line}:${message.column} ${
message.message
}${message.ruleId ? ` ${message.ruleId}` : ""}`;
if (formatedMessage !== expectedMessages[i]) {
throw new Error(
`
Message ${i} does not match:
Expected: ${expectedMessages[i]}
Actual: ${formatedMessage}
`,
);
}
});
}

function verifyAndAssertMessages(
code,
rules,
expectedMessages,
sourceType,
overrideConfig,
) {
const linter = new eslint.Linter();
linter.defineParser("current-babel-eslint", parser);

verifyAndAssertMessagesWithSpecificESLint(
unpad(`${code}`),
rules || {},
expectedMessages || [],
sourceType,
overrideConfig,
linter,
);
}
import verifyAndAssertMessages from "../../helpers/verifyAndAssertMessages";

describe("verify", () => {
it("arrow function support (issue #1)", () => {
Expand All @@ -90,9 +13,9 @@ describe("verify", () => {
);
});

xit("Readable error messages (issue #3)", () => {
it("Readable error messages (issue #3)", () => {
verifyAndAssertMessages("{ , res }", {}, [
"1:3 Parsing error: Unexpected token",
/1:2 Parsing error:.*Unexpected token \(1:2\)/,
Copy link
Member Author

Choose a reason for hiding this comment

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

This error reports an absolute file path, which is why I'm assuming it was disabled. With RegExp support we can now enable this!

]);
});

Expand Down Expand Up @@ -150,6 +73,7 @@ describe("verify", () => {
{ strict: [1, "global"] },
[],
"script",
{ sourceType: "script" },
);
});

Expand Down Expand Up @@ -1154,7 +1078,6 @@ describe("verify", () => {
) {
const overrideConfig = {
parserOptions: {
sourceType,
babelOptions: {
configFile: require.resolve(
"@babel/eslint-shared-fixtures/config/babel.config.decorators-legacy.js",
Expand Down
@@ -1,23 +1,23 @@
import eslint from "eslint";
import fs from "fs";
import path from "path";
import * as parser from "../src";
import * as parser from "@babel/eslint-parser";

eslint.linter.defineParser("current-babel-eslint", parser);
eslint.linter.defineParser("@babel/eslint-parser", parser);

const paths = {
fixtures: path.join(__dirname, "fixtures", "rules"),
fixtures: path.join(__dirname, "../../..", "fixtures", "rules"),
};

const encoding = "utf8";
const errorLevel = 2;

const baseEslintOpts = {
parser: "current-babel-eslint",
parser: "@babel/eslint-parser",
parserOptions: {
sourceType: "script",
requireConfigFile: false,
babelOptions: { configFile: false }
babelOptions: { configFile: false },
},
};

Expand Down Expand Up @@ -49,7 +49,6 @@ function readFixture(id, done) {
describe("Rules:", () => {
describe("`strict`", strictSuite);
});
// describe

function strictSuite() {
const ruleId = "strict";
Expand All @@ -74,10 +73,8 @@ function strictSuite() {
},
);
});
// it
});
});
// describe

describe("when set to 'global'", () => {
const eslintOpts = Object.assign({}, baseEslintOpts, {
Expand All @@ -98,7 +95,6 @@ function strictSuite() {
},
);
});
// it

it("should error twice on global directive: no and function directive: yes", done => {
lint(
Expand All @@ -108,14 +104,11 @@ function strictSuite() {
},
(err, report) => {
if (err) return done(err);
[0, 1].forEach(i => {
expect(report[0].ruleId).toBe(ruleId);
});
expect(report[0].ruleId).toBe(ruleId);
done();
},
);
});
// it

it("should error on function directive", done => {
lint(
Expand All @@ -135,7 +128,6 @@ function strictSuite() {
},
);
});
// it

it("should error on no directive", done => {
lint(
Expand All @@ -150,9 +142,7 @@ function strictSuite() {
},
);
});
// it
});
// describe

describe("when set to 'function'", () => {
const eslintOpts = Object.assign({}, baseEslintOpts, {
Expand All @@ -173,7 +163,6 @@ function strictSuite() {
},
);
});
// it

it("should error twice on function directive: no and global directive: yes", done => {
lint(
Expand All @@ -190,7 +179,6 @@ function strictSuite() {
},
);
});
// it

it("should error on only global directive", done => {
lint(
Expand All @@ -205,7 +193,6 @@ function strictSuite() {
},
);
});
// it

it("should error on extraneous global directive", done => {
lint(
Expand All @@ -221,17 +208,5 @@ function strictSuite() {
},
);
});
// it
});
}

describe("https://github.com/babel/babel-eslint/issues/558", () => {
it("doesn't crash with eslint-plugin-import", () => {
const engine = new eslint.CLIEngine({ ignore: false });
const files = ["a.js", "b.js", "c.js"];
let fileWithPath = files.map(file =>
path.resolve(__dirname, `./fixtures/eslint-plugin-import/${file}`),
);
engine.executeOnFiles(fileWithPath);
});
});