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

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

Closed
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

`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
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
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
`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
`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>
jungx098 pushed a commit to jungx098/node that referenced this pull request Mar 21, 2017
`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
Member

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
Labels
repl Issues and PRs related to the REPL subsystem. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants