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

Tune eslint packages test configuration #10848

Merged
merged 3 commits into from Dec 10, 2019

Conversation

JLHwung
Copy link
Contributor

@JLHwung JLHwung commented Dec 9, 2019

Q                       A
Fixed Issues? ESLint integration test should not load root babel configuration
License MIT

This PR is extracted from a messy working branch on the upcoming Yarn 2 support: https://github.com/JLHwung/babel/commits/standalone-rollup-bundle

As a follow up to #10705 , the integration test should not read root babel configuration.

@JLHwung JLHwung added the PR: Internal 🏠 A type of pull request used for our changelog categories label Dec 9, 2019
nicolo-ribaudo
nicolo-ribaudo previously approved these changes Dec 9, 2019
@nicolo-ribaudo nicolo-ribaudo dismissed their stale review December 9, 2019 20:19

The failing test is related to this PR

@JLHwung
Copy link
Contributor Author

JLHwung commented Dec 9, 2019

@nicolo-ribaudo The test error is expected, because now ecmaFeatrues.globalReturn is preserved. It aligns with ESLint behaviour: https://eslint.org/demo#eyJ0ZXh0IjoidmFyIGxlYWtlZEdsb2JhbCA9IDE7Iiwib3B0aW9ucyI6eyJwYXJzZXJPcHRpb25zIjp7ImVjbWFWZXJzaW9uIjo2LCJzb3VyY2VUeXBlIjoic2NyaXB0IiwiZWNtYUZlYXR1cmVzIjp7fX0sInJ1bGVzIjp7Im5vLWltcGxpY2l0LWdsb2JhbHMiOjJ9LCJlbnYiOnt9fX0=

I have added another test case when ecmaFeatures.globalReturn is false.

@@ -18,26 +18,23 @@ function verifyAndAssertMessagesWithSpecificESLint(
node: true,
es6: true,
},
...overrideConfig,
parserOptions: {
sourceType,
ecmaFeatures: {
globalReturn: true,
Copy link
Member

Choose a reason for hiding this comment

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

Can we remove this? It's better to set it explicitly in the only test where we need it rather than having a default value.

@nicolo-ribaudo
Copy link
Member

The Travis failures are caused by network errors

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Internal 🏠 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants