-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
test(eslint-plugin): install multiple eslint versions to test matrix #2464
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
test(eslint-plugin): install multiple eslint versions to test matrix #2464
Conversation
Thanks for the PR, @esetnik! typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community. The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately. Thanks again! 🙏 Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint. As a thank you, your profile/company logo will be added to our main README which receives thousands of unique visitors per day. |
@bradzacher note that a maintainer will need to update the list of expected tests to include the new matrix |
0c46d01
to
dddb051
Compare
870cb82
to
7f54ae3
Compare
@bradzacher I'm not sure why the tests are still failing. I rebased off master which should have fixed the eslint v5/v6 compatibility issues. |
Our extension rules build on top of the underlying rule. In some rules we just short-circuit branches for TS usecases, in other cases we do our own reporting for TS usecases. The latter is why our Our tests include a number of tests to ensure that cases our extensions don't touch remain unaffected. These tests are using message IDs. So this is failing because the tests are expecting that the rule reports using a message ID. |
@bradzacher do you have a proposal of how to solve this? One idea I had is to implement some detection logic in the rule tester which will detect eslint version under test and change the expectations accordingly. But maybe it's not the best solution. It'd be nice if the tests themselves were stable across supported eslint versions and didn't rely on any specific implementation details of eslint v7. |
There's two solutions I can think of. First would be to use The eslint tester will resolve the reported Note that this will work across versions if and only if the content of the message remains consistent across versions. This is the danger of Second would be to have the test be aware of the eslint version and update the assertions appropriately (I.e. Wrap every error object in a helper function to do a transform). The eslint package exports the current version as a property on the [ This would probably be my preferred option, as it will ensure the tests remain stable for all future versions. |
👋 @esetnik, just checking in! Do you still have interest & time to work on this PR? No worries if not! |
Closing for housekeeping purposes. |
No description provided.