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

Add `ncu-ci resume <prid>` command

Open MoLow opened this issue 3 years ago • 6 comments

refs: https://github.com/nodejs/node/issues/40817 refs: https://github.com/nodejs/node/issues/42125

I was able to test most of the flow except for the actual resume part - since my user dose not have resume permissions , but this resume url worked on a local jenkins I use - if any reviewer can check the entire flow for me it will be great

MoLow avatar Jul 19 '22 13:07 MoLow

CC @richardlau @nodejs/node-core-utils

MoLow avatar Jul 19 '22 14:07 MoLow

Codecov Report

Merging #642 (acfb36b) into main (bbad9a0) will increase coverage by 0.14%. The diff coverage is 94.11%.

:exclamation: Current head acfb36b differs from pull request most recent head ed06933. Consider uploading reports for the commit ed06933 to get more accurate results

@@            Coverage Diff             @@
##             main     #642      +/-   ##
==========================================
+ Coverage   84.09%   84.24%   +0.14%     
==========================================
  Files          37       37              
  Lines        4074     4138      +64     
==========================================
+ Hits         3426     3486      +60     
- Misses        648      652       +4     
Impacted Files Coverage Δ
lib/ci/ci_type_parser.js 90.47% <42.85%> (-1.65%) :arrow_down:
lib/ci/jenkins_constants.js 100.00% <100.00%> (ø)
lib/ci/run_ci.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update bbad9a0...ed06933. Read the comment docs.

codecov[bot] avatar Jul 19 '22 14:07 codecov[bot]

So I tried this branch and got the following errors:

$ ./bin/ncu-ci.js resume https://github.com/nodejs/node/pull/43919
   ✖  GitHub repository is missing, please set it via ncu-config or pass it via the --repo option
   ✖  GitHub owner is missing, please set it via ncu-config or pass it via the --owner option
$ ./bin/ncu-ci.js resume --repo node --owner nodejs https://github.com/nodejs/node/pull/43919
Error: [undefined] GraphQL request Error: Variable $prid of type Int! was provided invalid value
    at Request.query (file://…/lib/request.js:112:19)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async Request.gql (file://…/lib/request.js:69:22)
    at async PRData.getPR (file://…/lib/pr_data.js:80:20)
    at async Promise.all (index 0)
    at async JobParser.fromPRId (file://…/lib/ci/ci_type_parser.js:194:3)
    at async ResumePRJobCommand.start (file://…/bin/ncu-ci.js:319:20) {
  data: { variables: { prid: NaN, owner: 'nodejs', repo: 'node' } }
}

The following works though, and produces the expected result (https://github.com/nodejs/node/pull/43919#issuecomment-1191179362):

$ ./bin/ncu-ci.js resume --repo node --owner nodejs 43919
✔  Jenkins credentials valid
✔  Build data downloaded
✔  PR CI job successfully resumed

aduh95 avatar Jul 21 '22 08:07 aduh95

@aduh95 that is expected the ncu-ci resume command expects a pull-request id , not a pull-request URL, just like ncu-ci run

MoLow avatar Jul 21 '22 08:07 MoLow

This should not say Fixes: https://github.com/nodejs/node/issues/40817 unless the referenced issue (https://github.com/nodejs/node/issues/40817) should actually be closed once this PR is merged.

tniessen avatar Jul 21 '22 12:07 tniessen

There are a few conflicts to address here.

aduh95 avatar Feb 23 '23 09:02 aduh95