node-core-utils icon indicating copy to clipboard operation
node-core-utils copied to clipboard

fail positive on github action fails.

Open gengjiawen opened this issue 3 years ago • 11 comments

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

gengjiawen avatar Nov 24 '22 08:11 gengjiawen

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

targos avatar Nov 24 '22 08:11 targos

I just retriggered commit-queue, still failed.

gengjiawen avatar Nov 24 '22 08:11 gengjiawen

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

targos avatar Nov 24 '22 08:11 targos

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

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

gengjiawen avatar Nov 24 '22 08:11 gengjiawen

Isn't it because of this? image

targos avatar Nov 24 '22 08:11 targos

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

gengjiawen avatar Nov 24 '22 09:11 gengjiawen

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

targos avatar Nov 24 '22 10:11 targos

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).

aduh95 avatar Nov 24 '22 11:11 aduh95

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

targos avatar Nov 24 '22 12:11 targos

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

gengjiawen avatar Apr 03 '23 12:04 gengjiawen

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

targos avatar Apr 03 '23 12:04 targos