From 3f2e5e3d7e68b09a139e7f745fe0f2866cbc16c8 Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Thu, 3 Nov 2022 17:50:51 +0530 Subject: [PATCH 1/7] test: add a test to ensure the correctness of timezone upgrades Currently, there's no way to know if a timezone upgrade PR is correct without building and testing the change locally. This change provides a solution for that. Tested in https://github.com/RaisinTen/node/pull/4. Signed-off-by: Darshan Sen --- .github/workflows/timezone-update.yml | 3 +++ test/fixtures/tz-version.txt | 1 + test/parallel/test-tz-version.js | 12 ++++++++++++ 3 files changed, 16 insertions(+) create mode 100644 test/fixtures/tz-version.txt create mode 100644 test/parallel/test-tz-version.js diff --git a/.github/workflows/timezone-update.yml b/.github/workflows/timezone-update.yml index cebf43223e1046..27cbfd2946a1d2 100644 --- a/.github/workflows/timezone-update.yml +++ b/.github/workflows/timezone-update.yml @@ -36,6 +36,9 @@ jobs: - run: ./tools/update-timezone.mjs + - name: Update the expected timezone version in test + run: echo "${{ env.new_version }}" > test/fixtures/tz-version.txt + - name: Open Pull Request uses: gr2m/create-or-update-pull-request-action@dc1726cbf4dd3ce766af4ec29cfb660e0125e8ee # Create a PR or update the Action's existing PR env: diff --git a/test/fixtures/tz-version.txt b/test/fixtures/tz-version.txt new file mode 100644 index 00000000000000..13ad873c89cf2e --- /dev/null +++ b/test/fixtures/tz-version.txt @@ -0,0 +1 @@ +2022e diff --git a/test/parallel/test-tz-version.js b/test/parallel/test-tz-version.js new file mode 100644 index 00000000000000..ff5af21c2f2630 --- /dev/null +++ b/test/parallel/test-tz-version.js @@ -0,0 +1,12 @@ +'use strict'; + +require('../common'); +const { path } = require('../common/fixtures'); + +// This test ensures the correctness of the automated timezone upgrade PRs. + +const { strictEqual } = require('assert'); +const { readFileSync } = require('fs'); + +const expectedVersion = readFileSync(path('tz-version.txt'), 'utf8').trim(); +strictEqual(process.versions.tz, expectedVersion); From f0041404d2119ef1e308af2083bdf638aa46b627 Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Thu, 3 Nov 2022 21:18:42 +0530 Subject: [PATCH 2/7] fixup! test: address review comments and skip if no intl Signed-off-by: Darshan Sen --- test/parallel/test-tz-version.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/test/parallel/test-tz-version.js b/test/parallel/test-tz-version.js index ff5af21c2f2630..fdf964a128d956 100644 --- a/test/parallel/test-tz-version.js +++ b/test/parallel/test-tz-version.js @@ -1,12 +1,17 @@ 'use strict'; -require('../common'); -const { path } = require('../common/fixtures'); +const common = require('../common'); + +if (!common.hasIntl) { + common.skip('missing intl'); +} + +const fixtures = require('../common/fixtures'); // This test ensures the correctness of the automated timezone upgrade PRs. const { strictEqual } = require('assert'); const { readFileSync } = require('fs'); -const expectedVersion = readFileSync(path('tz-version.txt'), 'utf8').trim(); +const expectedVersion = readFileSync(fixtures.path('tz-version.txt'), 'utf8').trim(); strictEqual(process.versions.tz, expectedVersion); From 8780765fb5bd38e0c045c0055c23122129264e76 Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Fri, 4 Nov 2022 12:56:33 +0530 Subject: [PATCH 3/7] fixup! test: intl -> Intl in skip message Signed-off-by: Darshan Sen --- test/parallel/test-tz-version.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-tz-version.js b/test/parallel/test-tz-version.js index fdf964a128d956..8b2fa1f13da6c5 100644 --- a/test/parallel/test-tz-version.js +++ b/test/parallel/test-tz-version.js @@ -3,7 +3,7 @@ const common = require('../common'); if (!common.hasIntl) { - common.skip('missing intl'); + common.skip('missing Intl'); } const fixtures = require('../common/fixtures'); From 693f9bc0b7d4f25711c66e65f326f9226b91d55f Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Fri, 4 Nov 2022 13:37:46 +0530 Subject: [PATCH 4/7] fixup! test: add extra guards Refs: https://github.com/nodejs/node/pull/45299#discussion_r1013135497 Signed-off-by: Darshan Sen --- test/parallel/test-tz-version.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/parallel/test-tz-version.js b/test/parallel/test-tz-version.js index 8b2fa1f13da6c5..4fd0f678898721 100644 --- a/test/parallel/test-tz-version.js +++ b/test/parallel/test-tz-version.js @@ -6,6 +6,10 @@ if (!common.hasIntl) { common.skip('missing Intl'); } +if (process.config.variables.icu_path !== 'deps/icu-small') { + common.skip('not using the icu data file present in deps/icu-small/source/data/in/icudt##l.dat.bz2'); +} + const fixtures = require('../common/fixtures'); // This test ensures the correctness of the automated timezone upgrade PRs. From 69ec58470084813bef0f7ea71bbc45e07aa550fa Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Fri, 4 Nov 2022 14:06:27 +0530 Subject: [PATCH 5/7] fixup! test: add link to canned icu directory path Signed-off-by: Darshan Sen --- test/parallel/test-tz-version.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/parallel/test-tz-version.js b/test/parallel/test-tz-version.js index 4fd0f678898721..b022333680ed8b 100644 --- a/test/parallel/test-tz-version.js +++ b/test/parallel/test-tz-version.js @@ -6,6 +6,7 @@ if (!common.hasIntl) { common.skip('missing Intl'); } +// Refs: https://github.com/nodejs/node/blob/1af63a90ca3a59ca05b3a12ad7dbea04008db7d9/configure.py#L1694-L1711 if (process.config.variables.icu_path !== 'deps/icu-small') { common.skip('not using the icu data file present in deps/icu-small/source/data/in/icudt##l.dat.bz2'); } From c3ee43496199321faf2fe809c42970b9c9a55372 Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Sun, 6 Nov 2022 16:34:51 +0530 Subject: [PATCH 6/7] fixup! test: explain difference between small-icu and deps/icu-small Asked for in https://github.com/nodejs/node/pull/45299#discussion_r1014650144. Signed-off-by: Darshan Sen --- test/parallel/test-tz-version.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/parallel/test-tz-version.js b/test/parallel/test-tz-version.js index b022333680ed8b..ce60a44af29abf 100644 --- a/test/parallel/test-tz-version.js +++ b/test/parallel/test-tz-version.js @@ -8,6 +8,12 @@ if (!common.hasIntl) { // Refs: https://github.com/nodejs/node/blob/1af63a90ca3a59ca05b3a12ad7dbea04008db7d9/configure.py#L1694-L1711 if (process.config.variables.icu_path !== 'deps/icu-small') { + // If Node.js is configured to use its built-in ICU, it uses a strict subset + // of ICU formed using `tools/icu/shrink-icu-src.py`, which is present in + // `deps/icu-small`. It is not the same as configuring the build with + // `./configured --with-intl=small-icu`. The latter only uses a subset of the + // locales, i.e., it uses the English locale, `root,en`, by default and other + // locales can also be specified using the `--with-icu-locales` option. common.skip('not using the icu data file present in deps/icu-small/source/data/in/icudt##l.dat.bz2'); } From dfa9a346e90eea3a925c77e9e989e66b16cadf13 Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Sun, 6 Nov 2022 16:37:46 +0530 Subject: [PATCH 7/7] fixup! test: fix typo configured -> configure Signed-off-by: Darshan Sen --- test/parallel/test-tz-version.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/parallel/test-tz-version.js b/test/parallel/test-tz-version.js index ce60a44af29abf..6e4b14e1ac1880 100644 --- a/test/parallel/test-tz-version.js +++ b/test/parallel/test-tz-version.js @@ -11,7 +11,7 @@ if (process.config.variables.icu_path !== 'deps/icu-small') { // If Node.js is configured to use its built-in ICU, it uses a strict subset // of ICU formed using `tools/icu/shrink-icu-src.py`, which is present in // `deps/icu-small`. It is not the same as configuring the build with - // `./configured --with-intl=small-icu`. The latter only uses a subset of the + // `./configure --with-intl=small-icu`. The latter only uses a subset of the // locales, i.e., it uses the English locale, `root,en`, by default and other // locales can also be specified using the `--with-icu-locales` option. common.skip('not using the icu data file present in deps/icu-small/source/data/in/icudt##l.dat.bz2');