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

Backport of 5528 #5785

Closed
wants to merge 2 commits into from
Closed

Backport of 5528 #5785

wants to merge 2 commits into from

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Mar 18, 2016

This is a backport of #5528 (test: add a place for reproducing known bugs) to the v4.x-staging branch, at @thealphanerd request.

cjihrig added 2 commits March 18, 2016 13:02
This commit adds a known_issues directory to the test directory
for scripts that reproduce known bugs. Since these scripts are
expected to fail, it also adds a --expect-fail flag to test.py
which reports tests as successful when they fail.

Refs: nodejs/testing#18
PR-URL: nodejs#5528
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Wyatt Preul <wpreul@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

Conflicts:
	tools/test.py
This commit adds a failing test case for the vm module.
Currently, if runInContext() defines a function, and a later call
to runInContext() redefines the same function, the original
function is not overwritten.

Refs: nodejs#548
PR-URL: nodejs#5528
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Wyatt Preul <wpreul@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@mscdex mscdex added test Issues and PRs related to the tests. v4.x labels Mar 18, 2016
@jasnell
Copy link
Member

jasnell commented Mar 18, 2016

LGTM if ci is green

@MylesBorins
Copy link
Contributor

@MylesBorins MylesBorins self-assigned this Mar 18, 2016
@bnoordhuis
Copy link
Member

LGTM. Unrelated test failure on ARM. How are we going to test this on the CI?

EDIT: By which I mean: how are we going to test the new Makefile target on the CI?

@Trott
Copy link
Member

Trott commented Mar 19, 2016

Don't want to hijack the comment thread here, but the flaky ARM failure above is fixed in #5728 but that PR still needs a LGTM from a collaborator. Feel free to review!

@cjihrig
Copy link
Contributor Author

cjihrig commented Mar 20, 2016

@joaocgreis would it be possible to get a CI job setup to run this, independent of any other jobs?

@Trott
Copy link
Member

Trott commented Mar 20, 2016

@cjihrig An alternative to getting a custom CI job could be to create a new branch from this one, change make test-ci (and equivalent in vcbuild.bat) to be the same as the target you want to run, and run a test job against that commit/branch.

@joaocgreis
Copy link
Member

@Trott that does not work for the fanned jobs. Because they have to archive binaries, test-ci is not used.

I created a new job (slow, no cross-compiling), here is the first run: https://ci.nodejs.org/job/node-test-known-issues/1/ . There seems to be something wrong with one win2008 machine, I'll have to take a look at it. Does not seem related to this.

@cjihrig
Copy link
Contributor Author

cjihrig commented Mar 21, 2016

Thanks @joaocgreis!

@jasnell
Copy link
Member

jasnell commented Mar 21, 2016

LGTM

MylesBorins pushed a commit that referenced this pull request Mar 21, 2016
This commit adds a known_issues directory to the test directory
for scripts that reproduce known bugs. Since these scripts are
expected to fail, it also adds a --expect-fail flag to test.py
which reports tests as successful when they fail.

Refs: nodejs/testing#18
Backport-URL: #5785
PR-URL: #5528
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Wyatt Preul <wpreul@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

Conflicts:
	tools/test.py
MylesBorins pushed a commit that referenced this pull request Mar 21, 2016
This commit adds a failing test case for the vm module.
Currently, if runInContext() defines a function, and a later call
to runInContext() redefines the same function, the original
function is not overwritten.

Refs: #548
Backport-URL: #5785
PR-URL: #5528
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Wyatt Preul <wpreul@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@MylesBorins
Copy link
Contributor

landed in 52e2141...7725cd3

@MylesBorins MylesBorins removed their assignment Mar 21, 2016
@cjihrig cjihrig deleted the backport branch March 21, 2016 17:49
MylesBorins pushed a commit that referenced this pull request Mar 21, 2016
This commit adds a known_issues directory to the test directory
for scripts that reproduce known bugs. Since these scripts are
expected to fail, it also adds a --expect-fail flag to test.py
which reports tests as successful when they fail.

Refs: nodejs/testing#18
Backport-URL: #5785
PR-URL: #5528
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Wyatt Preul <wpreul@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

Conflicts:
	tools/test.py
MylesBorins pushed a commit that referenced this pull request Mar 21, 2016
This commit adds a failing test case for the vm module.
Currently, if runInContext() defines a function, and a later call
to runInContext() redefines the same function, the original
function is not overwritten.

Refs: #548
Backport-URL: #5785
PR-URL: #5528
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Wyatt Preul <wpreul@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
MylesBorins pushed a commit that referenced this pull request Mar 21, 2016
This commit adds a known_issues directory to the test directory
for scripts that reproduce known bugs. Since these scripts are
expected to fail, it also adds a --expect-fail flag to test.py
which reports tests as successful when they fail.

Refs: nodejs/testing#18
Backport-URL: #5785
PR-URL: #5528
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Wyatt Preul <wpreul@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

Conflicts:
	tools/test.py
MylesBorins pushed a commit that referenced this pull request Mar 21, 2016
This commit adds a failing test case for the vm module.
Currently, if runInContext() defines a function, and a later call
to runInContext() redefines the same function, the original
function is not overwritten.

Refs: #548
Backport-URL: #5785
PR-URL: #5528
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Wyatt Preul <wpreul@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants