test: Skip intl-no-icu-data test when not (Intl and smallICU)

[evantorrie]

The --icu-data-dir flag applies only when the code was originally
compiled with --with-intl=small-icu. Skip the test in all other cases

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• commit message follows [commit guidelines][]

Affected core subsystem(s)

test

Fixes: #12198

[richardlau]

I don't think these changes are sufficient for the case without intl (--with-intl=none). I noted when reviewing the original PR that added this test (#11255 (review)) that the test runner will fail to execute the test (since the // Flags: makes the runner append the option to the test command line and the option is invalid if node is compiled without intl) and will therefore not get to execute the added check.

[evantorrie]

@richardlau --with-intl=none results in process.binding('config').hasIntl == false, which results in common.hasIntl == false.

Since the skip condition in the PR is

if (!(common.hasIntl && common.hasSmallICU))

having common.hasIntl == false will result in the conditional always being true, i.e. the test will always be skipped when --with-intl=none.

[richardlau]

@evantorrie but that won't get executed because the test runner appends --icu-data-dir=test/fixtures/empty/ to the test command line and node will fail to launch because it is not a valid option.

[evantorrie]

@richardlau Ah yes, you're correct. In my particular case, the requirement was to skip the test when --with-intl=full-intl is specified (since --icu-data-dir=text/fixtures/empty/ does not override any compiled in ICU locales -- which for full-intl, is all of them).

To fix the case for --with-intl=none, I guess we'd have to change the actual test runner python script?

[richardlau]

To fix the case for --with-intl=none, I guess we'd have to change the actual test runner python script?

Either that or the test would have to be rewritten to not use // Flags and instead use child a process.

cc @nodejs/testing

[gibfahn]

I think the "rewrite the test to use a child process" option will work much better than the "rewrite the test runner".

[richardlau]

If you want to go down the modifying the test runner route, #12485 is doing something similar for openssl options.

[watilde]

Hmm... I think Intl.NumberFormat.supportedLocalesOf('en') with empty icu dir should be [] anyway.

[bnoordhuis]

#13053 makes a bad --icu-data-dir= switch a fatal error and removes the test, which should take care of the issue this PR tries to address. I'll go ahead and close it.