You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
It was noted (by @bnoordhuis) that test/parallel/test-icu-data-dir.js only runs with small-icu builds and we don't seem to have those in CI anymore. It used to be tested in CI on the Raspberry Pi devices, but they don't run on the main branch anymore. (They seem to run against v12.x staging daily. Since we don't support v12.x anymore, I suppose we could/should remove that nightly job?)
Should we add something to the linux-containered group that compiles with small ICU so we can continue to exercise that test?
(They seem to run against v12.x staging daily. Since we don't support v12.x anymore, I suppose we could/should remove that nightly job?)
Just on this point -- I've kept the job because we don't have a v14.x or v16.x nightly job and the existing v12.x job gives me some confidence that the CI machines still work (for example we no longer run builds on the CentOS 7/RHEL 7 machines for Node.js 18 and later).
Built locally to reproduce but all tests passed, likely because my system used Python3. I think that points in the direction of it being a Python2 problem as well.
@richardlau , if we need to test before this gets back into 14.x. I could make the test job for small icu apply the patch before buildting/testing. That would allow us to enable testing for 14.x earlier.
Built locally to reproduce but all tests passed, likely because my system used Python3. I think that points in the direction of it being a Python2 problem as well.
@richardlau , if we need to test before this gets back into 14.x. I could make the test job for small icu apply the patch before buildting/testing. That would allow us to enable testing for 14.x earlier.
@mhdawson Another option would be to always build with Python 3 (e.g. running with PYTHON=python3) for the small icu job?
Activity
richardlau commentedon Jul 14, 2022
Just on this point -- I've kept the job because we don't have a v14.x or v16.x nightly job and the existing v12.x job gives me some confidence that the CI machines still work (for example we no longer run builds on the CentOS 7/RHEL 7 machines for Node.js 18 and later).
mhdawson commentedon Jan 17, 2023
Working on adding small-icu to the contained tests.
Once tests pass will add to regular containered job.
mhdawson commentedon Jan 17, 2023
Passed in https://ci.nodejs.org/view/All/job/node-test-commit-linux-small-icu/6/nodes=ubuntu1804_sharedlibs_smallicu_x64/console
Will add to regular job first thing tomorrow.
richardlau commentedon Jan 18, 2023
@mhdawson the job is broken with Node.js v14.x-staging: https://ci.nodejs.org/job/node-test-commit-linux-small-icu/7/nodes=ubuntu1804_sharedlibs_smallicu_x64/
Started a run with v16.x-staging: https://ci.nodejs.org/job/node-test-commit-linux-small-icu/8/
mhdawson commentedon Jan 18, 2023
Looks like it passes for 16, will add so that it only runs on 16 and later for now.
mhdawson commentedon Jan 18, 2023
@richardlau I assume that's because small-icu is broken on 14 and not the job itself ?
We not only have a different version of ICU in 14 but also looks like some of the tools are slightly different versions?
The script that is failing though https://github.com/nodejs/node/tree/main/tools/icu)/icutrim.py seems to be the same version though as what we have in main
failure reported
richardlau commentedon Jan 18, 2023
@mhdawson I would guess it's a Python 2 vs Python 3 issue. Node.js 14 builds with Python 2 by default.
mhdawson commentedon Jan 18, 2023
@richardlau thanks for the suggestion of Python 2 versus Python 3, I was wondering if that might be the case as well.
mhdawson commentedon Jan 18, 2023
Built locally to reproduce but all tests passed, likely because my system used Python3. I think that points in the direction of it being a Python2 problem as well.
mhdawson commentedon Jan 18, 2023
Yup switching to ubuntu with python 2 and I get the same error as in the job.
mhdawson commentedon Jan 18, 2023
Working on a fix
tools: allow icutrim.py to run on python2
mhdawson commentedon Jan 18, 2023
PR with fix - nodejs/node#46263.
@richardlau , if we need to test before this gets back into 14.x. I could make the test job for small icu apply the patch before buildting/testing. That would allow us to enable testing for 14.x earlier.
richardlau commentedon Jan 19, 2023
@mhdawson Another option would be to always build with Python 3 (e.g. running with
PYTHON=python3) for the small icu job?mhdawson commentedon Jan 19, 2023
I've done that and enabled it for 14, job passed here https://ci.nodejs.org/job/node-test-commit-linux-containered/nodes=ubuntu1804_sharedlibs_smallicu_x64/35542/
I think we can probably now close this.
tools: allow icutrim.py to run on python2
tools: allow icutrim.py to run on python2
tools: allow icutrim.py to run on python2
tools: allow icutrim.py to run on python2