feat: add auto run v8 ci #652

gengjiawen · 2022-10-13T04:41:00Z

Hopefully this will make us test v8 easier :)

local test

node bin/ncu-ci.js run 44986

currently I run into Jenkins credentials invalid. But I have used this credential before, and it worked.

cc @targos @richardlau @nodejs/v8-update

richardlau

Please undo the style changes causing the linter to fail. Also this doesn't work:

$ git rev-parse --short HEAD
8e866ef
$ node bin/ncu-ci.js --owner nodejs --repo node run 44961
✔  Jenkins credentials valid
⠋ Starting PR CI job(node:1644671) [https://github.com/node-fetch/node-fetch/issues/1167] DeprecationWarning: form-data doesn't follow the spec and requires special treatment. Use alternative package
(Use `node --trace-deprecation ...` to show where the warning was created)
✔  PR CI job successfully started
✖  Failed to start CI
$

lib/ci/run_ci.js

richardlau · 2022-10-13T11:27:36Z

$ git rev-parse --short HEAD
8e866ef
$ node bin/ncu-ci.js --owner nodejs --repo node run 44961
✔  Jenkins credentials valid
⠋ Starting PR CI job(node:1644671) [https://github.com/node-fetch/node-fetch/issues/1167] DeprecationWarning: form-data doesn't follow the spec and requires special treatment. Use alternative package
(Use `node --trace-deprecation ...` to show where the warning was created)
✔  PR CI job successfully started
✖  Failed to start CI
$

More info on the failure -- the "Failed to start CI" message is from the try-catch handler

node-core-utils/lib/ci/run_ci.js

Line 102 in 8e866ef

cli.stopSpinner('Failed to start CI', this.cli.SPINNER_STATUS.FAILED);

With additional debug:

diff --git a/lib/ci/run_ci.js b/lib/ci/run_ci.js
index e850a7a..51d6fb4 100644
--- a/lib/ci/run_ci.js
+++ b/lib/ci/run_ci.js
@@ -99,6 +99,7 @@ export class RunPRJob {
       }

     } catch (err) {
+      console.error(err);
       cli.stopSpinner('Failed to start CI', this.cli.SPINNER_STATUS.FAILED);
       return false;
     }

the error is:

$ node bin/ncu-ci.js --owner nodejs --repo node run 44986
✔  Jenkins credentials valid
✔  PR CI job successfully started
TypeError: Cannot read property 'gql' of undefined
    at PRData.getPR (file:///home/rlau/sandbox/github/node-core-utils/lib/pr_data.js:80:34)
    at RunPRJob.start (file:///home/rlau/sandbox/github/node-core-utils/lib/ci/run_ci.js:81:25)
    at processTicksAndRejections (internal/process/task_queues.js:95:5)
✖  Failed to start CI

gengjiawen · 2022-10-13T11:53:36Z

Is the credential put in ~/.ncurc

richardlau · 2022-10-13T12:01:51Z

Is the credential put in ~/.ncurc

Yes. Credential is working -- the pull request job was successfully started and it's the additions in this PR that are not working.

gengjiawen · 2022-10-13T12:03:57Z

Is the credential put in ~/.ncurc

Yes. Credential is working -- the pull request job was successfully started and it's the additions in this PR that are not working.

Yeap. On my local I stuck at credential issue. So I mark it as draft.

gengjiawen · 2022-10-13T12:12:54Z

@legendecas Is this because proxy issue, not supporting socks5 proxy ?

Update: a China only issue, fucking gfw. proxy-agent looks only work in http proxy.

gengjiawen · 2022-10-13T12:59:29Z

Now the issues is it's not auto-commenting pr ? which part should I check ?

lib/ci/run_ci.js

richardlau · 2022-10-13T14:03:00Z

Looks good now (apart from the lint and test failures) and V8 CI is started:

$ node bin/ncu-ci.js --owner nodejs --repo node run 44986
✔  Jenkins credentials valid
⠋ Starting PR CI job(node:1665555) [https://github.com/node-fetch/node-fetch/issues/1167] DeprecationWarning: form-data doesn't follow the spec and requires special treatment. Use alternative package
(Use `node --trace-deprecation ...` to show where the warning was created)
✔  PR CI job successfully started
{ nodes: [ { name: 'v8 engine' } ] }
✔  V8 CI job successfully started
$

Started:

Refs: nodejs/node#44986 (comment)

Now the issues is it's not auto-commenting pr ? which part should I check ?

This is done by the github-bot. I think this is the relevant code: https://github.com/nodejs/github-bot/blob/2f5b61f263ebeda2d9525733bb9dfa055fa24660/lib/push-jenkins-update.js#L15-L27

gengjiawen · 2022-10-13T15:12:57Z

test fixed.

please squash this commit for conventional commit.

codecov · 2022-10-13T15:15:05Z

Codecov Report

Base: 84.03% // Head: 83.60% // Decreases project coverage by -0.42% ⚠️

Coverage data is based on head (232b7d2) compared to base (4e59a64).
Patch coverage: 35.00% of modified lines in pull request are covered.

❗ Current head 232b7d2 differs from pull request most recent head 5fa57b9. Consider uploading reports for the commit 5fa57b9 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #652      +/-   ##
==========================================
- Coverage   84.03%   83.60%   -0.43%     
==========================================
  Files          37       37              
  Lines        4077     4117      +40     
==========================================
+ Hits         3426     3442      +16     
- Misses        651      675      +24

Impacted Files	Coverage Δ
lib/ci/run_ci.js	`78.33% <35.00%> (-21.67%)`	⬇️
lib/verbosity.js	`80.76% <0.00%> (+7.69%)`	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

legendecas · 2022-10-13T16:05:52Z

@gengjiawen:
@legendecas Is this because proxy issue, not supporting socks5 proxy ?

Update: a China only issue, fucking gfw. proxy-agent looks only work in http proxy.

Not sure what I can help here?

gengjiawen · 2022-10-13T16:10:50Z

@gengjiawen:
@legendecas Is this because proxy issue, not supporting socks5 proxy ?
Update: a China only issue, fucking gfw. proxy-agent looks only work in http proxy.

Not sure what I can help here?

fixed, proxy compatibility issue

gengjiawen · 2022-10-15T11:39:40Z

@joyeecheung Can you review this ?

gengjiawen · 2022-10-19T09:51:57Z

@aduh95 @nodejs/node-core-utils Can you take a look at this ? thx

.gitignore

Trott · 2022-10-19T14:36:44Z

This looks good to me. Is there anything we can/should do about the small decrease in test coverage? I'm not worried about it if there isn't much to do.

.npmrc

Trott · 2022-10-19T14:40:12Z

please squash this commit for conventional commit.

Do you want to squash and force push to your branch?

gengjiawen · 2022-10-20T01:32:29Z

please squash this commit for conventional commit.

Do you want to squash and force push to your branch?

I would like merger do this like we do in nodejs/node-gyp and nodejs/gyp-next.

gengjiawen · 2022-10-20T01:34:15Z

This looks good to me. Is there anything we can/should do about the small decrease in test coverage? I'm not worried about it if there isn't much to do.

It's a duplicate logic already tested (different URL and HTTP payload for v8 jenkins task). I think in this case, it's okay.

gengjiawen · 2022-10-21T06:35:20Z

@Trott Can you merge this and nodejs/github-bot#353 (already reviewed by @joyeecheung ) ? thx

.gitignore

Recent changes to node-core-utils attempt to read pull request labels when deciding whether or not to start a V8 CI. This requires a valid GitHub token with appropriate permissions to be set for node-core-utils. Refs: nodejs/node-core-utils#652

Recent changes to node-core-utils attempt to read pull request labels when deciding whether or not to start a V8 CI. This requires a valid GitHub token with appropriate permissions to be set for node-core-utils. Refs: nodejs/node-core-utils#652 PR-URL: #45185 Fixes: #45163 Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com>

richardlau requested changes Oct 13, 2022

View reviewed changes

lib/ci/run_ci.js Outdated Show resolved Hide resolved

This comment was marked as outdated.

Sign in to view

gengjiawen requested a review from richardlau October 13, 2022 12:57

gengjiawen marked this pull request as ready for review October 13, 2022 13:00

richardlau reviewed Oct 13, 2022

View reviewed changes

lib/ci/run_ci.js Outdated Show resolved Hide resolved

gengjiawen requested a review from richardlau October 13, 2022 15:12

gengjiawen mentioned this pull request Oct 13, 2022

feat: add v8 support nodejs/github-bot#353

Merged

Trott reviewed Oct 19, 2022

View reviewed changes

.gitignore Outdated Show resolved Hide resolved

Trott reviewed Oct 19, 2022

View reviewed changes

.npmrc Show resolved Hide resolved

Trott approved these changes Oct 20, 2022

View reviewed changes

Trott force-pushed the feat/v8 branch from fbf26d0 to 4f6eb6a Compare October 21, 2022 22:57

Trott reviewed Oct 21, 2022

View reviewed changes

.gitignore Outdated Show resolved Hide resolved

feat: add auto run v8 ci

5fa57b9

Trott force-pushed the feat/v8 branch from 232b7d2 to 5fa57b9 Compare October 22, 2022 00:08

Trott merged commit 046bc0d into nodejs:main Oct 22, 2022

gengjiawen deleted the feat/v8 branch October 22, 2022 00:38

This was referenced Oct 25, 2022

build: add GitHub token to auto-start-ci workflow nodejs/node#45185

Merged

action lack permission to get pr info nodejs/node#45163

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add auto run v8 ci #652

feat: add auto run v8 ci #652

gengjiawen commented Oct 13, 2022 •

edited

Loading

richardlau left a comment

richardlau commented Oct 13, 2022

gengjiawen commented Oct 13, 2022

richardlau commented Oct 13, 2022

gengjiawen commented Oct 13, 2022

gengjiawen commented Oct 13, 2022 •

edited

Loading

This comment was marked as outdated.

gengjiawen commented Oct 13, 2022

richardlau commented Oct 13, 2022 •

edited

Loading

gengjiawen commented Oct 13, 2022 •

edited

Loading

codecov bot commented Oct 13, 2022 •

edited

Loading

legendecas commented Oct 13, 2022

gengjiawen commented Oct 13, 2022

gengjiawen commented Oct 15, 2022

gengjiawen commented Oct 19, 2022 •

edited

Loading

Trott commented Oct 19, 2022

Trott commented Oct 19, 2022

gengjiawen commented Oct 20, 2022

gengjiawen commented Oct 20, 2022

gengjiawen commented Oct 21, 2022

feat: add auto run v8 ci #652

feat: add auto run v8 ci #652

Conversation

gengjiawen commented Oct 13, 2022 • edited Loading

richardlau left a comment

Choose a reason for hiding this comment

richardlau commented Oct 13, 2022

gengjiawen commented Oct 13, 2022

richardlau commented Oct 13, 2022

gengjiawen commented Oct 13, 2022

gengjiawen commented Oct 13, 2022 • edited Loading

This comment was marked as outdated.

gengjiawen commented Oct 13, 2022

richardlau commented Oct 13, 2022 • edited Loading

gengjiawen commented Oct 13, 2022 • edited Loading

codecov bot commented Oct 13, 2022 • edited Loading

Codecov Report

legendecas commented Oct 13, 2022

gengjiawen commented Oct 13, 2022

gengjiawen commented Oct 15, 2022

gengjiawen commented Oct 19, 2022 • edited Loading

Trott commented Oct 19, 2022

Trott commented Oct 19, 2022

gengjiawen commented Oct 20, 2022

gengjiawen commented Oct 20, 2022

gengjiawen commented Oct 21, 2022

gengjiawen commented Oct 13, 2022 •

edited

Loading

gengjiawen commented Oct 13, 2022 •

edited

Loading

richardlau commented Oct 13, 2022 •

edited

Loading

gengjiawen commented Oct 13, 2022 •

edited

Loading

codecov bot commented Oct 13, 2022 •

edited

Loading

gengjiawen commented Oct 19, 2022 •

edited

Loading