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 DISABLED_ prefix to commented out test #14317

Closed
wants to merge 1 commit into from
Closed

test: add DISABLED_ prefix to commented out test #14317

wants to merge 1 commit into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Jul 17, 2017

Commit 95ab966 ("test: disable
MultipleEnvironmentsPerIsolate") commented out
MultipleEnvironmentsPerIsolate but it migth be better to disable
the test so that this gets reported and not forgotten:

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

test

@nodejs-github-bot nodejs-github-bot added the test Issues and PRs related to the tests. label Jul 17, 2017
@gibfahn
Copy link
Member

gibfahn commented Jul 17, 2017 •
edited
Loading

Ahh okay, that's how you mark a test as SKIP with cctest.

Link to docs: https://github.com/google/googletest/blob/a44bbab9bdb2ac80336504a373aab9109163a808/googletest/docs/AdvancedGuide.md#temporarily-disabling-tests

@danbev
test: add DISABLED_ prefix to commented out test
ad57ec9 
Commit 95ab966 ("test: disable
MultipleEnvironmentsPerIsolate") commented out
MultipleEnvironmentsPerIsolate but it migth be better to disable
the test so that this gets reported and not forgotten:

   YOU HAVE 1 DISABLED TEST
@danbev danbev force-pushed the disable-test-environment branch from 545bb06 to ad57ec9 Compare July 29, 2017 05:00
@danbev
Copy link
Contributor Author

danbev commented Jul 29, 2017

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

danbev added a commit to danbev/node that referenced this pull request Jul 29, 2017
@danbev
test: add DISABLED_ prefix to commented out test
75bf8a9 
Commit 95ab966 ("test: disable
MultipleEnvironmentsPerIsolate") commented out
MultipleEnvironmentsPerIsolate but it migth be better to disable
the test so that this gets reported and not forgotten:

   YOU HAVE 1 DISABLED TEST

PR-URL: nodejs#14317
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@danbev
Copy link
Contributor Author

danbev commented Jul 29, 2017

Landed in 75bf8a9

@danbev danbev closed this Jul 29, 2017
addaleax pushed a commit that referenced this pull request Aug 1, 2017
@danbev @addaleax
test: add DISABLED_ prefix to commented out test
8fea174 
Commit 95ab966 ("test: disable
MultipleEnvironmentsPerIsolate") commented out
MultipleEnvironmentsPerIsolate but it migth be better to disable
the test so that this gets reported and not forgotten:

   YOU HAVE 1 DISABLED TEST

PR-URL: #14317
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
@addaleax addaleax mentioned this pull request Aug 2, 2017
Merged
addaleax added a commit to addaleax/node that referenced this pull request Aug 10, 2017
@addaleax
Revert "test: add DISABLED_ prefix to commented out test"
3f95be9 
This reverts commit 75bf8a9.

Ref: nodejs#14206
Ref: nodejs#14317
addaleax added a commit that referenced this pull request Aug 10, 2017
@addaleax
Revert "test: add DISABLED_ prefix to commented out test"
ef7da42 
This reverts commit 75bf8a9.

Ref: #14206
Ref: #14317
PR-URL: #14749
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
addaleax added a commit that referenced this pull request Aug 10, 2017
@addaleax
Revert "test: add DISABLED_ prefix to commented out test"
8ff2a5c 
This reverts commit 75bf8a9.

Ref: #14206
Ref: #14317
PR-URL: #14749
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
@danbev danbev deleted the disable-test-environment branch August 16, 2017 07:41
@MylesBorins
Copy link
Contributor

this does not appear to be relevant to v6.x, please lmk if I'm wrong

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

@gibfahn gibfahn gibfahn approved these changes

@tniessen tniessen tniessen approved these changes

@cjihrig cjihrig cjihrig approved these changes

@jasnell jasnell jasnell approved these changes

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.
7 participants
@danbev @gibfahn @MylesBorins @jasnell @cjihrig @tniessen @nodejs-github-bot