fail positive on github action fails.

[gengjiawen]

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 nodejs/node#45573 (comment)

[targos]

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

[gengjiawen]

I just retriggered commit-queue, still failed.

[targos]

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

[gengjiawen]

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]

Isn't it because of this?

[gengjiawen]

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.

[targos]

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

[aduh95]

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]

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

[gengjiawen]

Another one: #691

[targos]

I had a look and it's because of this part of the code:

node-core-utils/lib/pr_checker.js

Lines 393 to 410 in 4ef0fe2

	// GitHub old commit status API
	if (commit.status) {
	const { state } = commit.status;
	if (state === 'PENDING') {
	cli.error('GitHub CI is still running');
	return false;
	}
	if (!['SUCCESS', 'EXPECTED'].includes(state)) {
	cli.error('Last GitHub CI failed');
	return false;
	}
	}
	cli.ok('Last GitHub CI successful');
	this.CIStatus = true;
	return true;
	}

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.