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 messages and use return to skip tests #2290

Closed

Conversation

thefourtheye
Copy link
Contributor

This is a followup of #2109.
The tests which didn't make it in #2109, are included in this patch.
The skip messages are supposed to follow the format

1..0 # Skipped: [Actual reason why the test is skipped]

and the tests should be skipped with the return statement.

cc @bnoordhuis @jbergstroem

This is a followup of nodejs#2109.
The tests which didn't make it in nodejs#2109, are included in this patch.
The skip messages are supposed to follow the format

    1..0 # Skipped: [Actual reason why the test is skipped]

and the tests should be skipped with the return statement.
@thefourtheye thefourtheye added the test Issues and PRs related to the tests. label Aug 2, 2015
@@ -5,8 +5,8 @@ var assert = require('assert');
try {
var crypto = require('crypto');
} catch (e) {
console.log('Not compiled with OPENSSL support.');
process.exit();
console.log('1..0 # Skipped: Not compiled with OPENSSL support.');
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we make sure the messages are consistent? :D

@Fishrock123
Copy link
Contributor

LGTM if the messages are consistent.

@jbergstroem
Copy link
Member

LGTM

@thefourtheye
Copy link
Contributor Author

CI Run: node-test-commit/80

thefourtheye added a commit that referenced this pull request Aug 3, 2015
This is a followup of #2109.
The tests which didn't make it in #2109, are included in this patch.
The skip messages are supposed to follow the format

    1..0 # Skipped: [Actual reason why the test is skipped]

and the tests should be skipped with the return statement.

PR-URL: #2290
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
@thefourtheye
Copy link
Contributor Author

Thanks guys, landed at 80a1cf7.

Since the linter job failed, I ran locally and it didn't raise any concerns.

@thefourtheye thefourtheye deleted the test-fixing-skip-messages-1 branch August 3, 2015 16:06
@rvagg rvagg mentioned this pull request Aug 4, 2015
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.

3 participants