nodejs / citgm

Canary in the Gold Mine
https://www.npmjs.com/package/citgm
Other
560 stars 146 forks source link

Latest CITGM results #997

Closed RafaelGSS closed 9 months ago

RafaelGSS commented 11 months ago

Let's use this issue to list failing modules and take action such as removing offending modules, skip specific arches and so on

@nodejs/citgm

ljharb commented 11 months ago

tape failed on rhel8-ppc64le due to a timeout; i suspect with a longer timeout it'd pass.

alfonsograziano commented 11 months ago

I just had a nice chat with @RafaelGSS. Will look into it :)

alfonsograziano commented 11 months ago

Regarding this test fail. On my machine works fine with these specifications:

Command run: citgm underscore@1.13.6 The tests are not flacky on this package.

alfonsograziano commented 11 months ago

All the issues regarding macOS are not reproducible for me locally. I'm using the correct Node version and the correct package version, the tests are not failing on my machine. @nodejs/build

RafaelGSS commented 11 months ago

Let's wait v18.x and main to see if the same happens for these versions. But, I guess that's something related to our machines.

Trott commented 11 months ago

The failures on three platforms for nan already has an issue (https://github.com/nodejs/citgm/issues/996). It would be great if someone could figure that one out. (I haven't looked and don't have the right expertise probably anyway.)

EDIT: Oh, I see @RafaelGSS already linked to it in the description. I'd love to see someone get to the bottom of that one.

Trott commented 11 months ago

The macOS failures have a PR that might be worth landing just to bypass things for straightforward JavaScript modules.

https://github.com/nodejs/citgm/pull/971

Trott commented 11 months ago

tape failed on rhel8-ppc64le due to a timeout; i suspect with a longer timeout it'd pass.

@ljharb Is it possible to pass a longer timeout value via the command line? Something along the lines of npm run tests-only -- --timeout=99999? If so, it would just be a matter of updating line 483 of lookup.json. https://github.com/nodejs/citgm/blob/cd1b28855259254bb860fb71ed95f9b7636e65b4/lib/lookup.json#L483

alfonsograziano commented 11 months ago

Is really interesting to notice that in the last run on main there are no issues over macOS even without the skip applied, but on v18.18 there are 6 errors.

I managed to reproduce the issue over pino@8.15.3 and also the multer issue. The multer one is flaky on my machine, here is the PR to fix it

alfonsograziano commented 11 months ago

@ljharb probably also semver failed due to a timeout. Shall we try to increase it or set it as flaky due to tests timeout?

ruyadorno commented 11 months ago

Please keep the max timeout to a reasonable amount. We don't want to end up with a citgm run that takes forever to timeout when something got stuck in the test.

ruyadorno commented 11 months ago

I managed to reproduce the issue over pino@8.15.3 and also the multer issue. The multer one is flaky on my machine, here is the PR to fix it

While that's a good short-term solution to the problem, an actual fix should hopefully land in either multer or node so that it stop failing on macos. In these cases please open an issue pinging the maintainer - you can find it by browsing lookup.json e.g:

https://github.com/nodejs/citgm/blob/cd1b28855259254bb860fb71ed95f9b7636e65b4/lib/lookup.json#L337-L341

Here's an example of an issue pinging the maintainer of a package for help: https://github.com/nodejs/citgm/issues/988. Keep in mind you might be able to post more info, since you're able to reproduce in your machine you might be able to add some helpful logs or maybe test which combinations of package version + node version it fails vs succeeds, etc.

Thanks for the help @alfonsograziano it's really appreciated! 🙏

alfonsograziano commented 10 months ago

Here are a few issues/PRs open now to fix these issues:

RafaelGSS commented 10 months ago

After merging both PRs:

alfonsograziano commented 10 months ago

After the latest run on v18.18, we have 14 issues.

alfonsograziano commented 10 months ago

After the latest run on v21.0.0, we have 14 issues.

RafaelGSS commented 10 months ago

For the install failing, I think we should just drop these packages if we confirm these failures aren't machine-related.

jest: ENOSPC: no space left on device. Do we know how to fix it? @RafaelGSS

Usually, we open an issue on nodejs/build.

alfonsograziano commented 10 months ago

For the install failing:

They are always failing on v18.18 but I cannot reproduce the failure on my local machine.

1006 will skip them for now on OSX and at the same time will create an issue to find a proper solution :)

Usually, we open an issue on nodejs/build.

Done here: https://github.com/nodejs/build/issues/3512 Outcome: The space has been increased by deleting some files, we now have more than 5gb free. Hopefully shouldn't happen in the next run

RafaelGSS commented 10 months ago

Next run: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/3316/ - long queue - we should wait a bit

alfonsograziano commented 10 months ago

@RafaelGSS I'm not sure if this run is correct, there are still errors from torrent-stream that has been removed entirely from the lookup

richardlau commented 10 months ago

@RafaelGSS I'm not sure if this run is correct, there are still errors from torrent-stream that has been removed entirely from the lookup

The CI job runs by default on published versions of CITGM -- there hasn't been a release with the changes to the lookup.

RafaelGSS commented 10 months ago

@RafaelGSS I'm not sure if this run is correct, there are still errors from torrent-stream that has been removed entirely from the lookup

The CI job runs by default on published versions of CITGM -- there hasn't been a release with the changes to the lookup.

Should I use nodejs/citgm#main on

image

?

UPDATE: Running https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/3318

alfonsograziano commented 10 months ago

The current status after this run is: 15 failures on these packages:

panva commented 10 months ago

I've traced the issue down to a recent zlib upgrade that landed on main which changed the outputs. Opened https://github.com/nodejs/node/issues/50138 and requested a revert and further investigation.

RafaelGSS commented 9 months ago

It seems we got a green CI for LTS release lines. I'm opening https://github.com/nodejs/citgm/issues/1033 for v21 failures.