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

feat: Implement FlatRuleTester #15519

Merged
merged 11 commits into from Feb 2, 2022
Merged

feat: Implement FlatRuleTester #15519

merged 11 commits into from Feb 2, 2022

Conversation

nzakas
Copy link
Member

@nzakas nzakas commented Jan 13, 2022

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[x] Add something to the core
[ ] Other, please explain:

What changes did you make? (Give an overview)

I implemented FlatRuleTester to allow folks to test their rules using flat config instead of eslintrc.

I also implemented tests for FlatRuleTester.

This class is not currently exported but will be when we do a developer preview.

Note: I copied over most of the RuleTester tests, but I did delete a block that was testing ecmaVersion normalization because it was basically just duplicating all of the Linter tests that already did that.

Is there anything you'd like reviewers to focus on?

Does it all look good?

@nzakas nzakas added the core Relates to ESLint's core APIs and features label Jan 13, 2022
@eslint-github-bot eslint-github-bot bot added the feature This change adds a new feature to ESLint label Jan 13, 2022
@nzakas nzakas linked an issue Jan 13, 2022 that may be closed by this pull request
69 tasks
@nzakas
Copy link
Member Author

nzakas commented Jan 14, 2022

Looks like I need to incorporate https://github.com/eslint/eslint/pull/15507/files before merging.

@nzakas nzakas marked this pull request as draft January 14, 2022 00:48
Implements FlatRuleTester as a way to allow devs to test their rules
against flat config.

Refs #13481
@nzakas nzakas marked this pull request as ready for review January 14, 2022 18:49
@@ -58,7 +58,11 @@ class FlatConfigArray extends ConfigArray {
schema: flatConfigSchema
});

this.unshift(...baseConfig);
if (baseConfig[Symbol.iterator]) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Convenience.

const assertionDescribe = assertEmitted(ruleTesterTestEmitter, "custom describe", "this-is-a-rule-name");
const assertionIt = assertEmitted(ruleTesterTestEmitter, "custom it", "valid(code);");
const assertionItOnly = assertEmitted(ruleTesterTestEmitter, "custom itOnly", "validOnly(code);");
describe("Subclassing", () => {
Copy link
Member Author

Choose a reason for hiding this comment

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

While copying over I noticed this wasn't inside of a describe block so I added one.

Copy link
Member

@aladdin-add aladdin-add left a comment

Choose a reason for hiding this comment

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

LGTM.

lib/rule-tester/flat-rule-tester.js Outdated Show resolved Hide resolved
lib/rule-tester/flat-rule-tester.js Outdated Show resolved Hide resolved
lib/rule-tester/flat-rule-tester.js Outdated Show resolved Hide resolved
@mdjermanovic mdjermanovic added the accepted There is consensus among the team that this change meets the criteria for inclusion label Jan 20, 2022
lib/rule-tester/flat-rule-tester.js Outdated Show resolved Hide resolved
lib/rule-tester/flat-rule-tester.js Outdated Show resolved Hide resolved
@nzakas
Copy link
Member Author

nzakas commented Jan 21, 2022

Okay, I think I got everything. 👍

lib/rule-tester/flat-rule-tester.js Outdated Show resolved Hide resolved
lib/rule-tester/flat-rule-tester.js Outdated Show resolved Hide resolved
lib/rule-tester/flat-rule-tester.js Outdated Show resolved Hide resolved
@nzakas
Copy link
Member Author

nzakas commented Jan 24, 2022

Okay, I think I got everything. 👍

Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

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

A few minor suggestions.

  • Examples (here) should be updated as they are using some properties that are now invalid (for example, string parser and top-level globals).
  • The same for typedefs (here).

tests/lib/rule-tester/flat-rule-tester.js Outdated Show resolved Hide resolved
lib/rule-tester/flat-rule-tester.js Show resolved Hide resolved
@nzakas
Copy link
Member Author

nzakas commented Jan 27, 2022

I'm actually just going to remove the examples at the top of the file. They are already woefully out of date and I don't think they provide any real value since we have actual documentation.

@nzakas
Copy link
Member Author

nzakas commented Jan 27, 2022

Updated the typedefs 👍

@nzakas
Copy link
Member Author

nzakas commented Feb 1, 2022

Ensured that all parsers are wrapped and added tests to verify.

Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

I managed to successfully run all rule tests we have (except for those with /*eslint-env*/ comments in the code) with FlatRuleTester, by automatically converting configurations to the new format: mdjermanovic@4746376

@mdjermanovic mdjermanovic removed a link to an issue Feb 2, 2022
69 tasks
@mdjermanovic
Copy link
Member

I removed the link to #13481 issue so that it doesn't get closed automatically after merging this PR.

@mdjermanovic mdjermanovic merged commit 6f940c3 into main Feb 2, 2022
@mdjermanovic mdjermanovic deleted the flat-config-rule-tester branch February 2, 2022 12:16
crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this pull request Feb 17, 2022
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [eslint](https://eslint.org) ([source](https://github.com/eslint/eslint)) | devDependencies | minor | [`8.8.0` -> `8.9.0`](https://renovatebot.com/diffs/npm/eslint/8.8.0/8.9.0) |

---

### Release Notes

<details>
<summary>eslint/eslint</summary>

### [`v8.9.0`](https://github.com/eslint/eslint/releases/v8.9.0)

[Compare Source](eslint/eslint@v8.8.0...v8.9.0)

#### Features

-   [`68f64a9`](eslint/eslint@68f64a9) feat: update eslint-scope to ignore `"use strict"` directives in ES3 ([#&#8203;15595](eslint/eslint#15595)) (Milos Djermanovic)
-   [`db57639`](eslint/eslint@db57639) feat: add `es2016`, `es2018`, `es2019`, and `es2022` environments ([#&#8203;15587](eslint/eslint#15587)) (Milos Djermanovic)
-   [`2dc38aa`](eslint/eslint@2dc38aa) feat: fix bug with arrow function return types in function-paren-newline ([#&#8203;15541](eslint/eslint#15541)) (Milos Djermanovic)
-   [`6f940c3`](eslint/eslint@6f940c3) feat: Implement FlatRuleTester ([#&#8203;15519](eslint/eslint#15519)) (Nicholas C. Zakas)

#### Documentation

-   [`570a036`](eslint/eslint@570a036) docs: add `one-var` example with `for-loop` initializer ([#&#8203;15596](eslint/eslint#15596)) (Milos Djermanovic)
-   [`417191d`](eslint/eslint@417191d) docs: Remove the $ prefix in terminal commands ([#&#8203;15565](eslint/eslint#15565)) (Andreas Lewis)
-   [`389ff34`](eslint/eslint@389ff34) docs: add missing `Variable#scope` property in the scope manager docs ([#&#8203;15571](eslint/eslint#15571)) (Milos Djermanovic)
-   [`f63795d`](eslint/eslint@f63795d) docs: no-eval replace dead link with working one ([#&#8203;15568](eslint/eslint#15568)) (rasenplanscher)
-   [`0383591`](eslint/eslint@0383591) docs: Remove old Markdown issue template ([#&#8203;15556](eslint/eslint#15556)) (Brandon Mills)
-   [`a8dd5a2`](eslint/eslint@a8dd5a2) docs: add 'when not to use it' section in no-duplicate-case docs ([#&#8203;15563](eslint/eslint#15563)) (Milos Djermanovic)
-   [`1ad439e`](eslint/eslint@1ad439e) docs: add missed verb in docs ([#&#8203;15550](eslint/eslint#15550)) (Jeff Mosawy)

#### Chores

-   [`586d45c`](eslint/eslint@586d45c) chore: Upgrade to espree@9.3.1 ([#&#8203;15600](eslint/eslint#15600)) (Milos Djermanovic)
-   [`623e1e2`](eslint/eslint@623e1e2) chore: Upgrade to eslint-visitor-keys@3.3.0 ([#&#8203;15599](eslint/eslint#15599)) (Milos Djermanovic)
-   [`355b23d`](eslint/eslint@355b23d) chore: fix outdated link to Code of Conduct in PR template ([#&#8203;15578](eslint/eslint#15578)) (Rich Trott)
-   [`b10fef2`](eslint/eslint@b10fef2) ci: use Node 16 for browser test ([#&#8203;15569](eslint/eslint#15569)) (Milos Djermanovic)
-   [`92f89fb`](eslint/eslint@92f89fb) chore: suggest demo link in bug report template ([#&#8203;15557](eslint/eslint#15557)) (Brandon Mills)

</details>

---

### Configuration

📅 **Schedule**: At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox.

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1173
Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org>
Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
@eslint-github-bot eslint-github-bot bot locked and limited conversation to collaborators Aug 2, 2022
@eslint-github-bot eslint-github-bot bot added the archived due to age This issue has been archived; please open a new issue for any further discussion label Aug 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted There is consensus among the team that this change meets the criteria for inclusion archived due to age This issue has been archived; please open a new issue for any further discussion core Relates to ESLint's core APIs and features feature This change adds a new feature to ESLint
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants