-
Notifications
You must be signed in to change notification settings - Fork 89
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
tests: save legacy tests #205
tests: save legacy tests #205
Conversation
|
const errors = [{ messageId: 'unexpected' }] | ||
const parserOptions = { ecmaVersion: 6 } | ||
|
||
ruleTester.run('assertion-before-screenshot', rule, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MikeMcC399 I'm not sure how the new tester works, but if possible, it seems like there'd be utility to breaking out these rules so that they can be in a shared file used for both legacy and recent tests. Just thinking how manual it will be to update both sets of tests whenever an update it needed. Not really sure it's worth it though for just 2 files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is similar to the Cypress migration from 9.x
to 10.x
where it was necessary in Cypress GitHub Actions to have two sets of tests in parallel for a limited time, one set for Cypress 9.x
that was only updated as absolutely necessary and a new set of tests for Cypress 10.x
which were maintained and updated. At some stage, a new major version was declared and support for Cypress 9.x
was dropped.
It will be difficult to continue to support ESLint 7
, 8
and 9
all in one version of this plugin and at some stage support for earlier versions will need to be dropped. I will put this is a separate issue for discussion.
Thanks for your comments!
🎉 This PR is included in version 3.3.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Situation
Tests in the tests/lib/rules which run internally under ESLint
7.x
and8.x
are not compatible with ESLint9.x
.The guide Migrate to 9.0 > FlatRuleTester is now RuleTester explains:
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
and9.x
.Migration steps
Achieving the goal requires separating the tests into a legacy copy, for use with the
RuleTester
class of ESLint7.x
and8.x
, and another copy which will be migrated to use theRuleTester
of ESLint9.x
.The migration is done in two steps:
RuleTester
class in ESLint9.x
.Changes
This PR deals with creating the legacy copy.
A follow-on PR will take care of the second step of migrating tests to run under to ESLint
9.x
.tests
totests-legacy
jest.config-legacy.js
test:legacy
scriptcircle.yml
to test ESLint7
and8
withtest-legacy
Note that the original tests/lib/rules directory is temporarily unused in this PR.
References
Verification
Ubuntu
22.04.4
LTS, Node.jsv20.12.2
LTS and onWindows 11, Node.js
v20.12.2
At this point, ESLint 9 cannot be tested and would result in multiple errors. The resolution is left for a follow-on PR.