Centralize @babel/eslint-* tests #11106
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
kaicataldo
force-pushed
the
centralize-babel-eslint-tests
branch
from
40bf094
to
eb3f5d8
Compare
kaicataldo
commented
Feb 6, 2020
| @@ -1,6 +0,0 @@ | |||
| class ClassName { | |||
There was a problem hiding this comment.
I don't think this is actually being used anywhere?
kaicataldo
commented
Feb 6, 2020
| }${message.ruleId ? ` ${message.ruleId}` : ""}`; | ||
| const expectedMessage = expectedMessages[i]; | ||
|
|
||
| if (expectedMessage instanceof RegExp) { |
There was a problem hiding this comment.
Added the ability to pass this function a RegExp so we can enable the xited test below.
There was a problem hiding this comment.
Can we use jest's methods instead of writing our own errors checking logic?
There was a problem hiding this comment.
I think that would be great. Mind if we do that in a follow up PR?
kaicataldo
force-pushed
the
centralize-babel-eslint-tests
branch
from
eb3f5d8
to
4ce484d
Compare
kaicataldo
commented
Feb 6, 2020
| verifyAndAssertMessages("{ , res }", {}, [ | ||
| "1:3 Parsing error: Unexpected token", | ||
| /1:2 Parsing error:.*Unexpected token \(1:2\)/, |
There was a problem hiding this comment.
This error reports an absolute file path, which is why I'm assuming it was disabled. With RegExp support we can now enable this!
|
|
kaicataldo
added
the
PR: Internal 🏠
nicolo-ribaudo
approved these changes
Feb 8, 2020
existentialism
approved these changes
Feb 8, 2020
rajasekarm
pushed a commit
to rajasekarm/babel
that referenced
this pull request
Feb 17, 2020
* Centralize @babel/eslint-* tests * Enable linting of @babel/eslint-* test files * Add missing sourceType
github-actions
bot
added
the
outdated
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I've been trying to figure out how to structure tests (and which packages should be responsible for which tests), and have decided to advocate for the following:
@babel/eslint-parser: Only tests the shape of the AST (and asserts that it matches Espree's)@babel/eslint-plugin: Only test core rules that we have to create an@babel/*copy of@babel/eslint-tests: Integration/performance tests (typescript-eslint has a similar setup). This includes testing with ESLint itself, the ESLint core rules that we don't have to override, popular plugins, etc.I've moved all of the integration-y (by the definition above) tests into
@babel/eslint-teststo centralize them there. Just to be clear,@babel/eslint-testsshould never be published. I made it a package because we need to install deps for testing.Thoughts/concerns?
I'm not sure why we weren't linting any of the
@babel/eslint-*test files, but I enabled that as well.