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

chore: migrate to RuleTester v9 #207

Merged

Conversation

MikeMcC399
Copy link
Collaborator

@MikeMcC399 MikeMcC399 commented May 7, 2024

Situation

Tests in the tests/lib/rules which run internally under ESLint 7.x and 8.x are not compatible with ESLint 9.x.

The guide Migrate to 9.0 > FlatRuleTester is now RuleTester explains:

As announced in our blog post, the temporary FlatRuleTester class has been renamed to RuleTester, while the RuleTester class from v8.x has been removed. Additionally, the FlatRuleTester export from eslint/use-at-your-own-risk has been removed.

This is a breaking change for the tests.

Goal

The enhancement goal is to be able to test the repo rules using each of the supported versions of ESLint: 7.x, 8.x and 9.x.

Migration steps

Achieving the goal requires separating the tests into a legacy copy, for use with the RuleTester class of ESLint 7.x and 8.x, and another copy which will be migrated to use the RuleTester of ESLint 9.x.

The migration is done in two steps:

  1. Create the legacy copy of tests, a script to use this copy and a modification to the CircleCI workflow. Done in PR tests: save legacy tests #205.
  2. Modify the current copy of tests for compatibility with the new RuleTester class in ESLint 9.x. (This PR.)

Changes

This PR deals with the second step of modifying the current copy of tests for compatibility with the new RuleTester class in ESLint 9.x.

For all tests/lib/rules *.js test files:

  1. harmonize tests to use the code block:
    const RuleTester = require('eslint').RuleTester
    const ruleTester = new RuleTester()
    according to the v9 > RuleTester documentation.
  2. Harmonize other formating.
  3. Change Remove unnecessary parserOptions to languageOptions. Edit: See chore: migrate to RuleTester v9 #207 (comment)
  4. For tests/lib/rules/no-unnecessary-waiting.js remove the section which attempts to test disabling the rule. This is not compatible with the v9 RuleTester and it is attempting to test a function of ESLint rather than the rule itself.
  5. For tests/lib/rules/unsafe-to-chain-command.js modify an incompatible option syntax in one rule.

For the CircleCI workflow

  1. Create a new test in CircleCI for ESLint 9.x to use the modified current tests.

References

@cypress-app-bot
Copy link

@MikeMcC399 MikeMcC399 marked this pull request as ready for review May 7, 2024 14:54
@MikeMcC399 MikeMcC399 self-assigned this May 7, 2024
@MikeMcC399
Copy link
Collaborator Author

@jennifer-shehane

Since this PR is successful ci/circleci: test-v9 could now be added to the branch protection rules as "Required".

@MikeMcC399 MikeMcC399 marked this pull request as draft May 8, 2024 08:54
@MikeMcC399
Copy link
Collaborator Author

MikeMcC399 commented May 8, 2024

After the initial submission of this PR I realized that it is not necessary to migrate parserOptions to languageOptions for RuleTester v9 compatibility.

According to ESLint v9 Configure Language Options the defaults are:

  • ecmaVersion: "latest"
  • sourceType: "module"

and so it is not necessary to specify these explicitly in the tests.

I have updated the PR accordingly by removing the parserOptions to simplify the tests.

There were also some instances where an errors parameter was incorrectly applied to a valid case, so I removed these, although they were not negatively affecting the results of running the tests.

@MikeMcC399 MikeMcC399 marked this pull request as ready for review May 8, 2024 10:39
@jennifer-shehane jennifer-shehane merged commit aa5ce37 into cypress-io:master May 9, 2024
7 checks passed
@MikeMcC399 MikeMcC399 deleted the migrate/ruletester-v9 branch May 9, 2024 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants