Skip to content

test: add tests for eslint rules #16138

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
Closed
not-an-aardvark wants to merge 1 commit into from
Closed

test: add tests for eslint rules #16138

not-an-aardvark wants to merge 1 commit into from

Conversation

@not-an-aardvark
Copy link
Contributor

@not-an-aardvark not-an-aardvark commented Oct 11, 2017 •
edited by richardlau
Loading

This adds tests for the custom eslint rules in this repository, using
the RuleTester test utility bundled with eslint.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

tools

@not-an-aardvark
test: add tests for eslint rules
266dadc 
This adds tests for the custom eslint rules in this repository, using
the `RuleTester` test utility bundled with eslint.
@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Oct 11, 2017
@not-an-aardvark
Copy link
Contributor Author

not-an-aardvark commented Oct 11, 2017

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

Trott
Trott approved these changes Oct 11, 2017
View reviewed changes
Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

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

😍 LGTM if CI is green. 😍

@not-an-aardvark
Copy link
Contributor Author

not-an-aardvark commented Oct 14, 2017

Landed in cd5ee52

@not-an-aardvark not-an-aardvark deleted the eslint-tests branch October 14, 2017 07:03
not-an-aardvark added a commit that referenced this pull request Oct 14, 2017
@not-an-aardvark
test: add tests for eslint rules
cd5ee52 
This adds tests for the custom eslint rules in this repository, using
the `RuleTester` test utility bundled with eslint.

PR-URL: #16138
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
addaleax pushed a commit to ayojs/ayo that referenced this pull request Oct 15, 2017
@not-an-aardvark @addaleax
test: add tests for eslint rules
7699238 
This adds tests for the custom eslint rules in this repository, using
the `RuleTester` test utility bundled with eslint.

PR-URL: nodejs/node#16138
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@MylesBorins
Copy link
Contributor

MylesBorins commented Oct 23, 2017

Looks like this patch breaks some tests on v8.x
Would someone be willing to backport?

@Trott
Copy link
Member

Trott commented Oct 25, 2017

Looks like this patch breaks some tests on v8.x
Would someone be willing to backport?

@not-an-aardvark Are you up for opening a PR for this against the v8.x-staging branch?

@not-an-aardvark not-an-aardvark self-assigned this Oct 28, 2017
@not-an-aardvark
Copy link
Contributor Author

not-an-aardvark commented Oct 29, 2017

I think the tests will pass after 5f46944 is backported to v8.x-staging. It looks like it was backported at one point, but was removed from the branch because it caused build errors (see #15083 (comment)). Those errors seem to have been fixed in #15307, so I think it should be safe to reapply 5f46944 again, and then this PR will backport cleanly.

@not-an-aardvark not-an-aardvark removed their assignment Oct 29, 2017
@Trott
Copy link
Member

Trott commented Oct 30, 2017

@MylesBorins Does what @not-an-aardvark suggests above work for you?

@jasnell jasnell mentioned this pull request Oct 30, 2017
Merged
@MylesBorins
Copy link
Contributor

MylesBorins commented Nov 16, 2017

This should likely be bundled in a general eslint update for v6.x

@joyeecheung joyeecheung mentioned this pull request Nov 17, 2017
Closed
@MylesBorins
Copy link
Contributor

MylesBorins commented Dec 19, 2017 •
edited
Loading

ping regarding backport to v8.x

edit: got it to work yay

MylesBorins pushed a commit that referenced this pull request Dec 19, 2017
@not-an-aardvark @MylesBorins
test: add tests for eslint rules
131cf42 
This adds tests for the custom eslint rules in this repository, using
the `RuleTester` test utility bundled with eslint.

PR-URL: #16138
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@gibfahn gibfahn mentioned this pull request Dec 20, 2017
Closed
gibfahn pushed a commit that referenced this pull request Dec 20, 2017
@not-an-aardvark @gibfahn
test: add tests for eslint rules
5a1a567 
This adds tests for the custom eslint rules in this repository, using
the `RuleTester` test utility bundled with eslint.

PR-URL: #16138
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@gibfahn gibfahn mentioned this pull request Dec 20, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell approved these changes

@Trott Trott Trott approved these changes

+1 more reviewer

@hiroppy hiroppy hiroppy approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

test Issues and PRs related to the tests.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@not-an-aardvark @MylesBorins @Trott @jasnell @hiroppy @nodejs-github-bot