-
Notifications
You must be signed in to change notification settings - Fork 28.8k
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: skip test-icu-transcode if ICU is not present #10707
Conversation
const buffer = require('buffer'); | ||
const assert = require('assert'); | ||
const icu = getPunycode(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMHO I think it would be better to have the skip up here instead. Also, there is already a common.hasIntl
that we should be able to re-use:
// ...
const assert = require('assert');
if (!common.hasIntl) {
common.skip('missing intl... skipping test');
return;
}
// ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your comment! I've updated it.
use common.hasIntl to make sure Intl object is present or not. PR-URL:
02f75dd
to
bf731b9
Compare
const buffer = require('buffer'); | ||
const assert = require('assert'); | ||
|
||
if (!common.hasIntl) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should use this for other test cases. 👍
This way we can easily find the test cases that are Intl
dependent.
use common.hasIntl to make sure Intl object is present or not. PR-URL: #10707 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Italo A. Casas <me@italoacasas.com> Reviewed-By: Steven R Loomis <srloomis@us.ibm.com>
Landed in a0e13da |
We should use `common.hasIntl` in tests for test cases which are related to ICU. This way we can easily find the test cases that are Intl dependent. Plus, it will be able to make the tests a little faster if we check hasIntl first. Also, this tweaks the log messages to unify the message. Refs: nodejs#10707
use common.hasIntl to make sure Intl object is present or not. PR-URL: nodejs#10707 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Italo A. Casas <me@italoacasas.com> Reviewed-By: Steven R Loomis <srloomis@us.ibm.com>
We should use `common.hasIntl` in tests for test cases which are related to ICU. This way we can easily find the test cases that are Intl dependent. Plus, it will be able to make the tests a little faster if we check hasIntl first. Also, this tweaks the log messages to unify the message. Refs: #10707 PR-URL: #10841 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
use common.hasIntl to make sure Intl object is present or not. PR-URL: nodejs#10707 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Italo A. Casas <me@italoacasas.com> Reviewed-By: Steven R Loomis <srloomis@us.ibm.com>
use common.hasIntl to make sure Intl object is present or not. PR-URL: nodejs#10707 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Italo A. Casas <me@italoacasas.com> Reviewed-By: Steven R Loomis <srloomis@us.ibm.com>
We should use `common.hasIntl` in tests for test cases which are related to ICU. This way we can easily find the test cases that are Intl dependent. Plus, it will be able to make the tests a little faster if we check hasIntl first. Also, this tweaks the log messages to unify the message. Refs: nodejs#10707 PR-URL: nodejs#10841 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
use common.hasIntl to make sure Intl object is present or not. PR-URL: nodejs#10707 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Italo A. Casas <me@italoacasas.com> Reviewed-By: Steven R Loomis <srloomis@us.ibm.com>
We should use `common.hasIntl` in tests for test cases which are related to ICU. This way we can easily find the test cases that are Intl dependent. Plus, it will be able to make the tests a little faster if we check hasIntl first. Also, this tweaks the log messages to unify the message. Refs: nodejs#10707 PR-URL: nodejs#10841 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
This does not land cleanly in LTS. Please feel free to manually backport |
ping re: backport |
I just started working on the backport to |
use common.hasIntl to make sure Intl object is present or not. PR-URL: nodejs#10707 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Italo A. Casas <me@italoacasas.com> Reviewed-By: Steven R Loomis <srloomis@us.ibm.com>
@MylesBorins I'm going to remove the |
Fixes #9511. Use icu's punycode implementation to make sure ICU is present or not in a test.
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passesAffected core subsystem(s)