Node: test-process-env-tz fails

[mihan007]

• Version:
  current master
• Platform:
  Darwin iMac-Mikhail 17.7.0 Darwin Kernel Version 17.7.0: Wed Apr 24 21:17:24 PDT 2019; root:xnu-4570.71.45~1/RELEASE_X86_64 x86_64
• Subsystem:

I'm running standard test suite using make test-only and got

=== release test-process-env-tz ===
Path: parallel/test-process-env-tz
assert.js:89
  throw new AssertionError(obj);
  ^

AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
+ actual - expected

+ 'Sat Apr 14 2018 14:34:56 GMT+0200 (GMT+02:00)'
- 'Sat Apr 14 2018 14:34:56 GMT+0200 (CEST)'
    at Object.<anonymous> (/Users/mihan007/Projects/node/test/parallel/test-process-env-tz.js:29:8)
    at Module._compile (internal/modules/cjs/loader.js:774:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:785:10)
    at Module.load (internal/modules/cjs/loader.js:641:32)
    at Function.Module._load (internal/modules/cjs/loader.js:556:12)
    at Function.Module.runMain (internal/modules/cjs/loader.js:837:10)
    at internal/main/run_main_module.js:17:11 {
  generatedMessage: true,
  code: 'ERR_ASSERTION',
  actual: 'Sat Apr 14 2018 14:34:56 GMT+0200 (GMT+02:00)',
  expected: 'Sat Apr 14 2018 14:34:56 GMT+0200 (CEST)',
  operator: 'strictEqual'
}
Command: out/Release/node /Users/mihan007/Projects/node/test/parallel/test-process-env-tz.js

My best guess here that it happens because I'm using Russian locale at my mac.

[bnoordhuis]

That's no surprise. v11.13.0 doesn't have the necessary changes, they only just landed in master. make test-only isn't for running an arbitrary node.js release against an arbitrary snapshot of the test suite.

[mihan007]

I'm sorry, I just tried to prepare for Node Code & Learn Saint Petersburg session. Manual said keep an eye that all tests passed. It's not. What is correct way to run tests against master branch?

[sam-github]

Correct way is to git clone [email protected]:nodejs/node.git, which will checkout master, cd node; ./configure; make -j6 node; make test

[mihan007]

Thanks. Will do and let you know if smth goes wrong

[Kefir100]

The test is still failing on a fresh master branch on my machine when I do make test-only. Maybe reopen the issue?

[addaleax]

@kefir100 If you’re still around for the Code & Learn, could you reach out to one of the mentors and make sure nothing weird is happening?

[lal12]

Hmm, I got the same exactly same error, after running:

# working dir is cloned git repo on master branch
git pull
./configure
make -j9 
make test

[bnoordhuis]

@lal12 Can you open a new issue? I assume that's with a clean checkout, no local changes?

[BridgeAR]

This has happened for two persons on the C&L event in St. Petersburg. The original description mentioned v11.3.0 which was by mistake. The executed code was actually the current master (I updated the description above).

They both had a clean clone and just ran make test.

[lal12]

I looked a bit more into it. The string is built in deps/v8/src/builtins/builtins-date.cc:184, it retrieves the time zone from the internal time zone cache. Depending on the build options it chooses different Implementations either (if INTL is activated) the ICU one, else an OS dependent one (for Linux this e.g. is the POSIX one via localtime_r which ironically uses a glibc specific extension ^^). I tested the return of my glibc, which is as expected.

So the problem is ICU related (see deps/v8/src/objects/intl-objects.cc:1758). As the documentation of icu::TimeZone::getDisplayName states it will return GMT[+-]HH:mm, which is was apparently happens here.

[bnoordhuis]

I had a look and I think the problem here is that V8 calls the version of icu::TimeZone::getDisplayName() that uses an implicit default locale - i.e., whatever icu::Locale::getDefault() returns, and that's influenced by the LC_ALL and LC_MESSAGES environment variables, it's not stable/predictable:

$ env LC_ALL=nl_NL ./out/Release/node test/parallel/test-process-env-tz.js 
assert.js:89
  throw new AssertionError(obj);
  ^

AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
+ actual - expected

+ 'Sat Apr 14 2018 14:34:56 GMT+0200 (GMT+02:00)'
- 'Sat Apr 14 2018 14:34:56 GMT+0200 (CEST)'
                                      ^
    at Object.<anonymous> (/Users/bnoordhuis/src/v1.x/test/parallel/test-process-env-tz.js:29:8)
    at Module._compile (internal/modules/cjs/loader.js:774:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:785:10)
    at Module.load (internal/modules/cjs/loader.js:641:32)
    at Function.Module._load (internal/modules/cjs/loader.js:556:12)
    at Function.Module.runMain (internal/modules/cjs/loader.js:837:10)
    at internal/main/run_main_module.js:17:11 {
  generatedMessage: true,
  code: 'ERR_ASSERTION',
  actual: 'Sat Apr 14 2018 14:34:56 GMT+0200 (GMT+02:00)',
  expected: 'Sat Apr 14 2018 14:34:56 GMT+0200 (CEST)',
  operator: 'strictEqual'
}

The easy fix is to set process.env.LC_ALL = 'C' in the test (and that's what I'll open a PR for) but it might be worth thinking about setting a default locale at startup. Consistency > configuration, IMO.