Add to Node.js CITGM #7239

GeoffreyBooth · 2023-10-23T21:21:50Z

GeoffreyBooth
Oct 23, 2023

Hi, in nodejs/citgm#1020 I’m adding support for pnpm to Node’s Canary in the Gold Mine project, which aims to test new builds of Node.js against many popular community projects to inform Node maintainers of potential breakages before new versions of Node are released. I’m adding support both for pnpm to be used as one of the package managers (like how CITGM already supports npm and Yarn) and also as one of the libraries that get tested. Having pnpm here will help ensure that new versions of Node don’t break pnpm, and having support for pnpm as one of the package managers in CITGM means that we can add other projects to CITGM that use pnpm as their package manager.

Can someone please help me determine the best way to build and test pnpm in this environment? In nodejs/citgm@4678175 I added the commands pnpm run compile and pnpm run test-main to build and test pnpm, but it fails in CI: https://github.com/nodejs/citgm/actions/runs/6612820965/job/17959419464. The pnpm: true part tells CITGM to use pnpm to install packages and to run these build and test commands. I’m not familiar with the pnpm codebase itself; this was based on looking in https://github.com/pnpm/pnpm/blob/main/CONTRIBUTING.md and https://github.com/pnpm/pnpm/blob/main/.github/workflows/ci.yml. Any advice is greatly appreciated!

zkochan · 2023-10-24T13:19:34Z

zkochan
Oct 24, 2023
Maintainer

At first glance everything looks correct. Although, it is weird that tests of only 3 workspace projects were started.

12 replies

GeoffreyBooth Oct 28, 2023
Author

On my local Mac with latest main, I see consistent failures with pnpm recursive exec works with PnP.

How would you feel if there was some way to run the pnpm test suite with flaky tests excluded? Like maybe a certain environment variable, when set, would exclude particular tests that aren’t guaranteed to pass. Then we could add pnpm to CITGM and run its tests with this flag, and we’d know that if the tests failed then Node somehow broke pnpm.

It’s a pretty hard rule of CITGM that we can’t add modules to test that don’t have rock solid test suites that are without flakes, because we need to avoid false positives where modules’ test suites fail due to factors other than the new Node code.

zkochan Oct 28, 2023
Maintainer

There are only 2-3 tests that are "flaky" (which I guess we can either fix or skip on CITGM).

There are a few other tests that fail with timeout, when they try to download the node.js tarball. These we need to probably address somehow by mocking the node.js tarball request.

And there are some tests that sometimes fail but always pass on second run. Hence, we tell jest setup to retry a test 1 time if it fails.

Testing the "@pnpm/core" library might be enough. It should cover most of the things.

GeoffreyBooth Oct 28, 2023
Author

Can you tell me a command to run that skips the flaky tests, and also skips any tests that depend on external resources like downloads?

Testing the “@pnpm/core” library might be enough.

What would be the command to do this?

zkochan Oct 28, 2023
Maintainer

Can you tell me a command to run that skips the flaky tests, and also skips any tests that depend on external resources like downloads?

There's no such command. Almost all tests make network requests to a locally running verdaccio. But those are stable. The unstable ones are only those that download and install Node.js.

Testing the “@pnpm/core” library might be enough.

What would be the command to do this?

pnpm -F core test

GeoffreyBooth Oct 29, 2023
Author

Almost all tests make network requests to a locally running verdaccio. But those are stable. The unstable ones are only those that download and install Node.js.

Network requests to localhost don’t concern me; that should never fail in CITGM’s CI. It’s the tests that rely on external resources like the Node.js binary download server that I’d like to disable. How would you feel about adding an environment variable or other method that would allow me to skip such tests?

GeoffreyBooth · 2023-11-03T22:51:16Z

GeoffreyBooth
Nov 3, 2023
Author

If you look at https://github.com/nodejs/citgm/actions/runs/6748209270, I have the pnpm tests passing in CITGM for a few of the test platforms and Node versions. Do you mind helping me understand the failures, and decide what you think we should do about them?

The three Windows runs all failed on Error: fuse-shared-library is not currently supported on: win32. Do you expect your test suite to pass on Windows? If not, perhaps certain tests could be marked to be skipped on Windows?
The macOS tests both fail on ERR_PNPM_RECURSIVE_RUN_FIRST_FAIL @pnpm/[email protected] test: `pnpm run compile && pnpm run _test` . What does this mean? And for some reason the macOS Node 20 run doesn’t print the summary line with the number of tests passed and failed.

6 replies

GeoffreyBooth Nov 4, 2023
Author

Is it optional for building pnpm running its tests? Should I install pnpm without its optional dependencies?

GeoffreyBooth Nov 4, 2023
Author

It seems like pnpm can’t function without its optional dependencies:

.../[email protected]/node_modules/esbuild postinstall$ node install.js                                                                                                                         
.../[email protected]/node_modules/esbuild postinstall: [esbuild] Failed to find package "@esbuild/darwin-x64" on the file system                                                               
.../[email protected]/node_modules/esbuild postinstall: This can happen if you use the "--no-optional" flag. The "optionalDependencies"                                                         
.../[email protected]/node_modules/esbuild postinstall: package.json feature is used by esbuild to install the correct binary executable                                                        
.../[email protected]/node_modules/esbuild postinstall: for your current platform. This install script will now attempt to work around                                                          
.../[email protected]/node_modules/esbuild postinstall: this. If that fails, you need to remove the "--no-optional" flag to use esbuild.                                                        
.../[email protected]/node_modules/esbuild postinstall: [esbuild] Trying to install package "@esbuild/darwin-x64" using npm                                                                     
.../[email protected]/node_modules/esbuild postinstall: Done

So does this just mean that the test suite can’t run in Windows? Do you not test pnpm in Windows?

And can you explain the error I’m seeing in the Mac runs?

zkochan Nov 4, 2023
Maintainer

Failing optional dependencies don't make installation fail with a non-0 exit code. Installation works on Windows. Fuse is mocked in tests.

zkochan Nov 4, 2023
Maintainer

On macOS this two tests failed:

error:                     | FAIL test/install/misc.ts (414.172 s)                                                                                                               
error:                     | ● building native addons                                                                                                                            
error:                     |                                                                                                                                                     
error:                     | [email protected] install: `node-gyp rebuild`                                                                                                         
error:                     | Exit status 1                                                                                                                                       
error:                     |                                                                                                                                                     
error:                     | at EventEmitter.<anonymous> (../../node_modules/.pnpm/@[email protected][email protected]/node_modules/@pnpm/npm-lifecycle/index.js:286:13)   
error:                     | at ChildProcess.<anonymous> (../../node_modules/.pnpm/@[email protected][email protected]/node_modules/@pnpm/npm-lifecycle/lib/spawn.js:55:14)
error:                     |                                                                                                                                                     
error:                     | FAIL test/install/lifecycleScripts.ts (30.123 s)                                                                                                    
error:                     | ● lifecycle scripts have access to node-gyp                                                                                                         
error:                     |                                                                                                                                                     
error:                     | [email protected] install: `node-gyp rebuild`                                                                                                         
error:                     | Exit status 1                                                                                                                                       
error:                     |                                                                                                                                                     
error:                     | at EventEmitter.<anonymous> (../../node_modules/.pnpm/@[email protected][email protected]/node_modules/@pnpm/npm-lifecycle/index.js:286:13)   
error:                     | at ChildProcess.<anonymous> (../../node_modules/.pnpm/@[email protected][email protected]/node_modules/@pnpm/npm-lifecycle/lib/spawn.js:55:14)

We can check if we can rewrite them or skip them.

GeoffreyBooth Nov 4, 2023
Author

I looked at the Windows runs again and you’re right, it’s not failing on the install step; it runs the test suite and for Node 20 and 21, there are some failing tests:

error:                     | Summary of all failing tests                                                                                                                                    
error:                     | FAIL test/install/peerDependencies.ts (28.499 s)                                                                                                                
error:                     | ● run pre/postinstall scripts of each variations of packages with peer dependencies                                                                             
error:                     |                                                                                                                                                                 
error:                     | @pnpm.e2e/[email protected] preinstall: `hello-world-js-bin && node create generated-by-preinstall`                                               
error:                     | spawn C:\Windows\system32\cmd.exe ENOENT                                                                                                                        
error:                     |                                                                                                                                                                 
error:                     |                                                                                                                                                                 
error:                     |                                                                                                                                                                 
error:                     | FAIL test/install/lifecycleScripts.ts (17.934 s)                                                                                                                
error:                     | ● scripts have access to unlisted bins when hoisting is used                                                                                                    
error:                     |                                                                                                                                                                 
error:                     | @pnpm.e2e/[email protected] postinstall: `node -e "process.stdout.write('Hello world!')" | json-append output.json`                    
error:                     | spawn C:\Windows\system32\cmd.exe ENOENT

For Node 18, the test suite times out.

GeoffreyBooth · 2023-11-12T22:04:54Z

GeoffreyBooth
Nov 12, 2023
Author

I’ve opened a PR at nodejs/citgm#1027 specifically for adding pnpm to the Node CITGM test suite. Once you think pnpm‘s test suite is stable enough that it should pass consistently in the CITGM GitHub Actions runs, please comment on that PR and we can test it.

I would like to include pnpm in Node CITGM but I don’t have any more time to spend on this; if you would like it in CITGM, so that future Node updates hopefully don’t break pnpm, then I’ll need someone from the pnpm project to take it from here and get a test suite that is stable and passes reliably and remains so.

1 reply

zkochan Nov 15, 2023
Maintainer

Thanks for the effort. We'll look into it but we'll need some time to get back to it.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

pnpm

Add to Node.js CITGM #7239

{{title}}

Replies: 3 comments 19 replies

{{title}}

{{title}}

{{title}}

{{editor}}'s edit

{{editor}}'s edit

{{title}}

{{editor}}'s edit

{{editor}}'s edit

{{title}}

{{editor}}'s edit

{{editor}}'s edit

{{title}}

{{title}}

{{title}}

{{title}}

{{editor}}'s edit

{{editor}}'s edit

{{title}}

{{title}}

{{title}}

{{title}}

{{title}}

{{editor}}'s edit

{{editor}}'s edit

Select a reply

pnpm

Add to Node.js CITGM #7239

GeoffreyBooth Oct 23, 2023

Replies: 3 comments · 19 replies

zkochan Oct 24, 2023 Maintainer

GeoffreyBooth Oct 28, 2023 Author

zkochan Oct 28, 2023 Maintainer

GeoffreyBooth Oct 28, 2023 Author

zkochan Oct 28, 2023 Maintainer

GeoffreyBooth Oct 29, 2023 Author

GeoffreyBooth Nov 3, 2023 Author

GeoffreyBooth Nov 4, 2023 Author

GeoffreyBooth Nov 4, 2023 Author

zkochan Nov 4, 2023 Maintainer

zkochan Nov 4, 2023 Maintainer

GeoffreyBooth Nov 4, 2023 Author

GeoffreyBooth Nov 12, 2023 Author

zkochan Nov 15, 2023 Maintainer

GeoffreyBooth
Oct 23, 2023

Replies: 3 comments 19 replies

zkochan
Oct 24, 2023
Maintainer

GeoffreyBooth Oct 28, 2023
Author

zkochan Oct 28, 2023
Maintainer

GeoffreyBooth Oct 28, 2023
Author

zkochan Oct 28, 2023
Maintainer

GeoffreyBooth Oct 29, 2023
Author

GeoffreyBooth
Nov 3, 2023
Author

GeoffreyBooth Nov 4, 2023
Author

GeoffreyBooth Nov 4, 2023
Author

zkochan Nov 4, 2023
Maintainer

zkochan Nov 4, 2023
Maintainer

GeoffreyBooth Nov 4, 2023
Author

GeoffreyBooth
Nov 12, 2023
Author

zkochan Nov 15, 2023
Maintainer