Skip to content
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

Re-enable readable-stream #1039

Merged
merged 4 commits into from Dec 29, 2023
Merged

Re-enable readable-stream #1039

merged 4 commits into from Dec 29, 2023

Conversation

mcollina
Copy link
Member

@mcollina mcollina commented Dec 15, 2023

#1037

Checklist
  • npm test passes
  • tests are included
  • documentation is changed or added
  • contribution guidelines followed
    here

Signed-off-by: Matteo Collina <hello@matteocollina.com>
@mcollina
Copy link
Member Author

@mcollina
Copy link
Member Author

@codecov-commenter
Copy link

codecov-commenter commented Dec 15, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (e2a0731) 95.23% compared to head (67aa775) 95.23%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1039   +/-   ##
=======================================
  Coverage   95.23%   95.23%           
=======================================
  Files          28       28           
  Lines        2181     2181           
=======================================
  Hits         2077     2077           
  Misses        104      104           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@targos
Copy link
Member

targos commented Dec 15, 2023

Looks broken on v21.

@mcollina
Copy link
Member Author

working on it in nodejs/readable-stream#528, hopefully landing soon.

@richardlau
Copy link
Member

Installation fails on AIX as esbuild doesn't support it.

https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/3360/nodes=aix72-ppc64/console

09:34:22 warn: readable-stream npm-install:| npm ERR! code 1                                                                                                                                                        
09:34:22 warn:                             | npm ERR! path /home/iojs/tmp/citgm_tmp/cd2c8a76-4c62-439c-840f-095946746320/readable-stream/node_modules/esbuild                                                       
09:34:22 warn:                             | npm ERR! command failed                                                                                                                                                
09:34:22 warn:                             | npm ERR! command sh -c node install.js                                                                                                                                 
09:34:22 warn:                             | npm ERR! /home/iojs/tmp/citgm_tmp/cd2c8a76-4c62-439c-840f-095946746320/readable-stream/node_modules/esbuild/install.js:69                                              
09:34:22 warn:                             | npm ERR!     throw new Error(`Unsupported platform: ${platformKey}`);                                                                                                  
09:34:22 warn:                             | npm ERR!           ^                                                                                                                                                   
09:34:22 warn:                             | npm ERR!                                                                                                                                                               
09:34:22 warn:                             | npm ERR! Error: Unsupported platform: aix ppc64 BE                                                                                                                     
09:34:22 warn:                             | npm ERR!     at pkgAndSubpathForCurrentPlatform (/home/iojs/tmp/citgm_tmp/cd2c8a76-4c62-439c-840f-095946746320/readable-stream/node_modules/esbuild/install.js:69:11)  
09:34:22 warn:                             | npm ERR!     at checkAndPreparePackage (/home/iojs/tmp/citgm_tmp/cd2c8a76-4c62-439c-840f-095946746320/readable-stream/node_modules/esbuild/install.js:211:28)          
09:34:22 warn:                             | npm ERR!     at Object.<anonymous> (/home/iojs/tmp/citgm_tmp/cd2c8a76-4c62-439c-840f-095946746320/readable-stream/node_modules/esbuild/install.js:238:1)               
09:34:22 warn:                             | npm ERR!     at Module._compile (node:internal/modules/cjs/loader:1375:14)                                                                                             
09:34:22 warn:                             | npm ERR!     at Module._extensions..js (node:internal/modules/cjs/loader:1434:10)                                                                                      
09:34:22 warn:                             | npm ERR!     at Module.load (node:internal/modules/cjs/loader:1206:32)                                                                                                 
09:34:22 warn:                             | npm ERR!     at Module._load (node:internal/modules/cjs/loader:1022:12)                                                                                                
09:34:22 warn:                             | npm ERR!     at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:142:12)                                                                    
09:34:22 warn:                             | npm ERR!     at node:internal/main/run_main_module:28:49                                                                                                               
09:34:22 warn:                             | npm ERR!                                                                                                                                                               
09:34:22 warn:                             | npm ERR! Node.js v22.0.0-pre                                                                                                                                           
09:34:22 warn:                             |                                                                                                                                                                        
09:34:22 warn:                             | npm ERR! A complete log of this run can be found in: /home/iojs/tmp/citgm_tmp/cd2c8a76-4c62-439c-840f-095946746320/home/.npm/_logs/2023-12-15T09_44_41_045Z-debug-0.log

@merceyz
Copy link
Member

merceyz commented Dec 17, 2023

@richardlau I opened an issue in the esbuild repo about adding aix ppc64 BE support, do you think it would be possible to test if it works on that machine?

Ref evanw/esbuild#3549 (comment)

Signed-off-by: Matteo Collina <hello@matteocollina.com>
@mcollina
Copy link
Member Author

@mcollina
Copy link
Member Author

@mcollina
Copy link
Member Author

It seems it's passing on Linux. Why is it run on aix and windows even if it's skipped in the definition? https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/3363/nodes=aix72-ppc64/testReport/junit/(root)/citgm/readable_stream_v4_5_0/

@targos
Copy link
Member

targos commented Dec 18, 2023

Skips only apply to the citgm-all command.

lib/lookup.json Outdated Show resolved Hide resolved
Co-authored-by: Richard Lau <rlau@redhat.com>
@mcollina
Copy link
Member Author

@RafaelGSS @richardlau what's the next step here?

@RafaelGSS
Copy link
Member

@RafaelGSS @richardlau what's the next step here?

If that's passing in the expected environments we can merge it and release a new version.

@richardlau
Copy link
Member

@richardlau I opened an issue in the esbuild repo about adding aix ppc64 BE support, do you think it would be possible to test if it works on that machine?

Ref evanw/esbuild#3549 (comment)

@merceyz I've asked @abmusse from IBM to take a look.

@mcollina
Copy link
Member Author

Note that the esbuild package is not strictly needed to run those tests, but I'm not sure how I can skip its install at this stage.

@abmusse
Copy link

abmusse commented Dec 20, 2023

@richardlau I opened an issue in the esbuild repo about adding aix ppc64 BE support, do you think it would be possible to test if it works on that machine?
Ref evanw/esbuild#3549 (comment)

@merceyz I've asked @abmusse from IBM to take a look.

UPDATE

I've tested installing the current latest version of esbuild@0.19.10 on AIX 7.2 and the install was successful.

Ref: evanw/esbuild#3549 (comment)

Signed-off-by: Matteo Collina <hello@matteocollina.com>
@mcollina
Copy link
Member Author

@mcollina
Copy link
Member Author

Should be good to go if the run pass. What are the next steps?

@targos targos merged commit 419202a into main Dec 29, 2023
8 of 11 checks passed
@targos targos deleted the re-add-readable-stream branch December 29, 2023 10:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants