nodejs / node-core-utils

CLI tools for Node.js Core collaborators
https://nodejs.github.io/node-core-utils/
MIT License
234 stars 106 forks source link

fail positive on github action fails. #660

Open gengjiawen opened 1 year ago

gengjiawen commented 1 year ago
Commit Queue failed
- Loading data for nodejs/node/pull/45573
✔  Done loading data for nodejs/node/pull/45573
----------------------------------- PR info ------------------------------------
Title      deps: V8: cherry-pick 2ada52cffbff (#45573)
Author     Michaël Zasso  (@targos)
Branch     targos:fix-45171 -> nodejs:main
Labels     build, v8 engine, needs-ci
Commits    1
 - deps: V8: cherry-pick 2ada52cffbff
Committers 1
 - Michaël Zasso 
PR-URL: https://github.com/nodejs/node/pull/45573
Fixes: https://github.com/nodejs/node/issues/45171
Refs: https://github.com/v8/v8/commit/2ada52cffbff11074abfaac18938bf02d85454f5
Reviewed-By: Jiawen Geng 
Reviewed-By: Richard Lau 
Reviewed-By: Colin Ihrig 
Reviewed-By: Yagiz Nizipli 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/45573
Fixes: https://github.com/nodejs/node/issues/45171
Refs: https://github.com/v8/v8/commit/2ada52cffbff11074abfaac18938bf02d85454f5
Reviewed-By: Jiawen Geng 
Reviewed-By: Richard Lau 
Reviewed-By: Colin Ihrig 
Reviewed-By: Yagiz Nizipli 
--------------------------------------------------------------------------------
   ⚠  Commits were pushed since the last review:
   ⚠  - deps: V8: cherry-pick 2ada52cffbff
   ℹ  This PR was created on Tue, 22 Nov 2022 08:21:45 GMT
   ✔  Approvals: 4
   ✔  - Jiawen Geng (@gengjiawen): https://github.com/nodejs/node/pull/45573#pullrequestreview-1189584648
   ✔  - Richard Lau (@richardlau) (TSC): https://github.com/nodejs/node/pull/45573#pullrequestreview-1190006137
   ✔  - Colin Ihrig (@cjihrig) (TSC): https://github.com/nodejs/node/pull/45573#pullrequestreview-1190051320
   ✔  - Yagiz Nizipli (@anonrig): https://github.com/nodejs/node/pull/45573#pullrequestreview-1191762773
   ✖  Last GitHub CI failed
   ℹ  Last Full PR CI on 2022-11-23T14:48:53Z: https://ci.nodejs.org/job/node-test-pull-request/48124/
- Querying data for job/node-test-pull-request/48124/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/3538908052

Originally posted by @nodejs-github-bot in https://github.com/nodejs/node/issues/45573#issuecomment-1326111158

targos commented 1 year ago

It's not a false positive. I rebased and pushed after the last review.

gengjiawen commented 1 year ago

I just retriggered commit-queue, still failed.

targos commented 1 year ago

Yes, but that's expected. Someone needs to submit a review after the push.

gengjiawen commented 1 year ago

Yes, but that's expected. Someone needs to submit a review after the push.

I approved the PR again, new commit-queue still failed.

targos commented 1 year ago

Isn't it because of this? image

gengjiawen commented 1 year ago

Nope, it's a bug in GitHub (not sure how long they can fix it, it's has been really long), there is a issue on it too IIRC.

You can see log here, it shows it's the github action fails.

image
targos commented 1 year ago

I reopened because the error was wrong, but I still think the failure was expected (Jenkins CI had a failure).

aduh95 commented 1 year ago

Yeah, "GitHub CI" is not very accurate because all it does is check the "status of the PR", which includes the Jenkins results (which are sometimes picked up from a run that's testing an older commit, but that's really not something that can be addressed in this repo).

targos commented 1 year ago

But why did ncu say "Last Jenkins CI successful" when https://ci.nodejs.org/job/node-test-pull-request/48124/ is clearly red?

gengjiawen commented 1 year ago

Another one: https://github.com/nodejs/node-core-utils/issues/691

targos commented 1 year ago

I had a look and it's because of this part of the code: https://github.com/nodejs/node-core-utils/blob/4ef0fe2607f3986f702bcd1ada244965cfcd4457/lib/pr_checker.js#L393-L410

It's actually the Jenkins CI check that is red. I wonder if we should remove this whole block. I don't know what kind of "old" checks we expect to be here.

CleanShot 2023-04-03 at 14 09 12