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.

Sign up for GitHub

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

Jump to bottom

test: add common.hasEslint #18807

Closed
wants to merge 1 commit into from
Closed

test: add common.hasEslint #18807

wants to merge 1 commit into from

Conversation

MylesBorins
Copy link
Contributor

We've added a number of tests that hook into ESLint which can error
when running the test suite with the distributed tarball. This PR
adds a new test helper common.hasEslint and will skip ESLint related
tests when ESLint is not available at tools/node_modules/eslint

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Feb 15, 2018
@MylesBorins
Copy link
Contributor Author

CI: https://ci.nodejs.org/job/node-test-pull-request/13188/

@devsnek
Copy link
Member

devsnek commented Feb 16, 2018

might wanna also abstract requiring the rules since they might move around, otherwise lgtm

richardlau
richardlau approved these changes Feb 16, 2018
View reviewed changes
Copy link
Member

@richardlau richardlau left a comment

Choose a reason for hiding this comment

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

Do we envisage any uses of hasEslint that are not immediately followed by skip? If not perhaps this could be like skipIfInspectorDisabled instead.

@MylesBorins
Copy link
Contributor Author

@richardlau good call updated

@MylesBorins
test: add common.skipIfEslintMissing
70190f9 
We've added a number of tests that hook into ESLint which can error
when running the test suite with the distributed tarball. This PR
adds a new test helper `common.skipIfEslintMissing` and will skip
remaining tests in a file when `ESLint` is not available at
`tools/node_modules/eslint`
@MylesBorins
Copy link
Contributor Author

new ci: https://ci.nodejs.org/job/node-test-pull-request/13191/

@BridgeAR BridgeAR added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. fast-track PRs that do not need to wait for 48 hours to land. labels Feb 16, 2018
@MylesBorins
Copy link
Contributor Author

CI failures are unrelated +1 for fast track, anyone else open to it?

jasnell pushed a commit that referenced this pull request Feb 17, 2018
@MylesBorins @jasnell
test: add common.skipIfEslintMissing
1fd6987 
We've added a number of tests that hook into ESLint which can error
when running the test suite with the distributed tarball. This PR
adds a new test helper `common.skipIfEslintMissing` and will skip
remaining tests in a file when `ESLint` is not available at
`tools/node_modules/eslint`

PR-URL: #18807
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@jasnell
Copy link
Member

jasnell commented Feb 17, 2018

Landed in 1fd6987

@jasnell jasnell closed this Feb 17, 2018
MylesBorins added a commit that referenced this pull request Feb 21, 2018
@MylesBorins
test: add common.skipIfEslintMissing
6f37c4d 
We've added a number of tests that hook into ESLint which can error
when running the test suite with the distributed tarball. This PR
adds a new test helper `common.skipIfEslintMissing` and will skip
remaining tests in a file when `ESLint` is not available at
`tools/node_modules/eslint`

PR-URL: #18807
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@MylesBorins MylesBorins mentioned this pull request Feb 21, 2018
Merged
MylesBorins added a commit that referenced this pull request Feb 21, 2018
@MylesBorins
test: add common.skipIfEslintMissing
0eead8c 
We've added a number of tests that hook into ESLint which can error
when running the test suite with the distributed tarball. This PR
adds a new test helper `common.skipIfEslintMissing` and will skip
remaining tests in a file when `ESLint` is not available at
`tools/node_modules/eslint`

PR-URL: #18807
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins added a commit that referenced this pull request Feb 21, 2018
@MylesBorins
test: add common.skipIfEslintMissing
f960ad4 
We've added a number of tests that hook into ESLint which can error
when running the test suite with the distributed tarball. This PR
adds a new test helper `common.skipIfEslintMissing` and will skip
remaining tests in a file when `ESLint` is not available at
`tools/node_modules/eslint`

PR-URL: #18807
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
MylesBorins added a commit that referenced this pull request Feb 26, 2018
@MylesBorins
test: add common.skipIfEslintMissing
474d776 
We've added a number of tests that hook into ESLint which can error
when running the test suite with the distributed tarball. This PR
adds a new test helper `common.skipIfEslintMissing` and will skip
remaining tests in a file when `ESLint` is not available at
`tools/node_modules/eslint`

PR-URL: #18807
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@kasicka kasicka mentioned this pull request Mar 8, 2018
Closed
@richardlau richardlau mentioned this pull request Apr 27, 2018
Closed
MayaLekova pushed a commit to MayaLekova/node that referenced this pull request May 8, 2018
@MylesBorins @MayaLekova
test: add common.skipIfEslintMissing
0b7c7c9 
We've added a number of tests that hook into ESLint which can error
when running the test suite with the distributed tarball. This PR
adds a new test helper `common.skipIfEslintMissing` and will skip
remaining tests in a file when `ESLint` is not available at
`tools/node_modules/eslint`

PR-URL: nodejs#18807
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BridgeAR BridgeAR BridgeAR approved these changes

@richardlau richardlau richardlau approved these changes

@hiroppy hiroppy hiroppy approved these changes

@cjihrig cjihrig cjihrig approved these changes

@Trott Trott Trott approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. fast-track PRs that do not need to wait for 48 hours to land. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@MylesBorins @devsnek @jasnell @Trott @cjihrig @hiroppy @richardlau @BridgeAR @nodejs-github-bot