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: fix repl-function-redefinition-edge-case #11772

Closed
wants to merge 1 commit into from
Closed

test: fix repl-function-redefinition-edge-case #11772

wants to merge 1 commit into from

Conversation

aqrln
Copy link
Contributor

@aqrln aqrln commented Mar 9, 2017

test/known_issues/test-repl-function-redefinition-edge-case.js had been introduced as a part of #7624 but the meat of the test became fixed in 007386e. Despite that, the test continued to fail since it was broken itself: there was a missing colon in the expected output.

This commit adds the missing colon and moves the test from test/known_issues to test/parallel.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines
Affected core subsystem(s)

test

@aqrln
test: fix repl-function-redefinition-edge-case
500e84a 
`test/known_issues/test-repl-function-redefinition-edge-case.js` had
been introduced as a part of nodejs#7624
but the meat of the test became fixed in
007386e. Despite that, the test
continued to fail since it was broken itself: there was a missing colon
in the expected output.

This commit adds the missing colon and moves the test from
`test/known_issues` to `test/parallel`.
@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Mar 9, 2017
addaleax
addaleax approved these changes Mar 9, 2017
View reviewed changes
Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

Awesome! LGTM if CI is green

CI: https://ci.nodejs.org/job/node-test-commit/8341/

@addaleax addaleax added the repl Issues and PRs related to the REPL subsystem. label Mar 9, 2017
mhdawson
mhdawson approved these changes Mar 10, 2017
View reviewed changes
Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

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

LGTM

jasnell pushed a commit that referenced this pull request Mar 15, 2017
@aqrln @jasnell
test: fix repl-function-redefinition-edge-case
205f4e5 
`test/known_issues/test-repl-function-redefinition-edge-case.js` had
been introduced as a part of #7624
but the meat of the test became fixed in
007386e. Despite that, the test
continued to fail since it was broken itself: there was a missing colon
in the expected output.

This commit adds the missing colon and moves the test from
`test/known_issues` to `test/parallel`.

PR-URL: #11772
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@jasnell
Copy link
Member

jasnell commented Mar 15, 2017

Landed in 205f4e5

@jasnell jasnell closed this Mar 15, 2017
@aqrln aqrln deleted the fix-repl-known-issue branch March 15, 2017 00:32
italoacasas pushed a commit to italoacasas/node that referenced this pull request Mar 20, 2017
@aqrln @italoacasas
test: fix repl-function-redefinition-edge-case
d2c9111 
`test/known_issues/test-repl-function-redefinition-edge-case.js` had
been introduced as a part of nodejs#7624
but the meat of the test became fixed in
007386e. Despite that, the test
continued to fail since it was broken itself: there was a missing colon
in the expected output.

This commit adds the missing colon and moves the test from
`test/known_issues` to `test/parallel`.

PR-URL: nodejs#11772
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@italoacasas italoacasas mentioned this pull request Mar 20, 2017
Merged
jungx098 pushed a commit to jungx098/node that referenced this pull request Mar 21, 2017
@aqrln @jungx098
test: fix repl-function-redefinition-edge-case
2f8ecfe 
`test/known_issues/test-repl-function-redefinition-edge-case.js` had
been introduced as a part of nodejs#7624
but the meat of the test became fixed in
007386e. Despite that, the test
continued to fail since it was broken itself: there was a missing colon
in the expected output.

This commit adds the missing colon and moves the test from
`test/known_issues` to `test/parallel`.

PR-URL: nodejs#11772
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@MylesBorins
Copy link
Contributor

does not look like this applies to v6.x.

please feel free to change label if this is incorrect

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mhdawson mhdawson mhdawson approved these changes

@cjihrig cjihrig cjihrig approved these changes

@targos targos targos approved these changes

@addaleax addaleax addaleax approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
repl Issues and PRs related to the REPL subsystem. test Issues and PRs related to the tests.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
8 participants
@aqrln @jasnell @MylesBorins @addaleax @targos @cjihrig @mhdawson @nodejs-github-bot