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

feat: allow citgm job comparison #455

Closed codebytere closed 3 years ago

codebytere commented 3 years ago

Follow-up to https://github.com/nodejs/node-core-utils/pull/454. Closes https://github.com/nodejs/node-core-utils/issues/352.

Extends ncu-ci cigtm <jobid> to ncu-cu cigtm <jobid> [jobid2], allowing for direct comparisons between two CITGM runs to see if new failures were introduced.

I also did a bit of a refactor here, since there were a million classes in the result parser file and it was getting wildly unwieldy. I tried to do the minimum necessary to make this work but I plan to follow it up with the remainder of the the class split-out work.

Display:

Developer ❯ ncu-ci citgm 2392 2390                                             11:06PM
--------------------------------------------------------------------------------
[1/1] Running CITGM: 2392
--------------------------------------------------------------------------------
✔  Summary data downloaded
✔  Results data downloaded
✔  Summary data downloaded
✔  Results data downloaded
----------------------------------- Summary ------------------------------------
Result     FAILURE
URL        https://ci.nodejs.org/job/citgm-smoker/2392/
Source     https://api.github.com/repos/nodejs/node/git/refs/heads/v12.x
Commit     [feed95cd4c2c] Working on v12.18.1
Date       2020-06-02 20:27:47 +0200
Author     Michaël Zasso <targos@protonmail.com>
----------------------------------- Summary ------------------------------------
Result     FAILURE
URL        https://ci.nodejs.org/job/citgm-smoker/2390/
Source     https://github.com/nodejs/node/pull/33811/
Commit     [9a60117875dd] 2020-06-16, Version 12.18.1 'Erbium' (LTS)
Date       2020-06-09 20:23:09 -0700
Author     Shelley Vohr <shelley.vohr@gmail.com>
----------------------------------- Results ------------------------------------
┌────────────────────────┬───────────────────────────┬────────────────────────────┬─────────────────────────┐
│        (index)         │             0             │             1              │            2            │
├────────────────────────┼───────────────────────────┼────────────────────────────┼─────────────────────────┤
│       debian9-64       │ 'express-session-v1.17.1' │ 'yeoman-generator-v4.10.1' │                         │
│     centos7-ppcle      │    'ember-cli-v3.18.0'    │      'multer-v1.4.2'       │ 'torrent-stream-v1.2.0' │
│      rhel7-s390x       │  'torrent-stream-v1.2.0'  │                            │                         │
│   fedora-latest-x64    │    'spawn-wrap-v2.0.0'    │                            │                         │
│     ubuntu1804-64      │                           │                            │                         │
│ fedora-last-latest-x64 │                           │                            │                         │
│     ubuntu1604-64      │                           │                            │                         │
└────────────────────────┴───────────────────────────┴────────────────────────────┴─────────────────────────┘
Sample JSON Output ```json [ { "baseBuild": { "source": "https://api.github.com/repos/nodejs/node/git/refs/heads/v12.x", "upstream": "https://ci.nodejs.org/job/citgm-smoker/2390/" }, "comparisonBuild": { "source": "https://github.com/nodejs/node/pull/33811/", "upstream": "https://ci.nodejs.org/job/citgm-smoker/2392/" }, "debian9-64": [ { "name": "express-session-v1.17.1", "status": "FAILED" }, { "name": "yeoman-generator-v4.10.1", "status": "FAILED" } ], "centos7-ppcle": [ { "name": "ember-cli-v3.18.0", "status": "FAILED" }, { "name": "multer-v1.4.2", "status": "FAILED" }, { "name": "torrent-stream-v1.2.0", "status": "FAILED" } ], "rhel7-s390x": [ { "name": "torrent-stream-v1.2.0", "status": "FAILED" } ], "fedora-latest-x64": [ { "name": "spawn-wrap-v2.0.0", "status": "FAILED" } ], "ubuntu1804-64": [], "fedora-last-latest-x64": [], "ubuntu1604-64": [] } ] ```
Sample Markdown Output ```md # CITGM Data for [#2392](https://ci.nodejs.org/job/citgm-smoker/2392/) - [#2390](https://ci.nodejs.org/job/citgm-smoker/2390/) ## New Failures in job [#2390](https://ci.nodejs.org/job/citgm-smoker/2390/) ### debian9-64 * express-session-v1.17.1 * yeoman-generator-v4.10.1 ### centos7-ppcle * ember-cli-v3.18.0 * multer-v1.4.2 * torrent-stream-v1.2.0 ### rhel7-s390x * torrent-stream-v1.2.0 ### fedora-latest-x64 * spawn-wrap-v2.0.0 ### ubuntu1804-64 None. ### fedora-last-latest-x64 None. ### ubuntu1604-64 None. ```

cc @richardlau (since i can't request your review)

codecov[bot] commented 3 years ago

Codecov Report

Merging #455 into master will decrease coverage by 0.25%. The diff coverage is 69.56%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #455      +/-   ##
==========================================
- Coverage   76.66%   76.41%   -0.26%     
==========================================
  Files          21       27       +6     
  Lines        1603     1717     +114     
==========================================
+ Hits         1229     1312      +83     
- Misses        374      405      +31     
Impacted Files Coverage Δ
lib/ci/build-types/test_build.js 49.52% <49.52%> (ø)
lib/ci/build-types/citgm_comparison_build.js 64.93% <64.93%> (ø)
lib/ci/build-types/citgm_build.js 79.26% <79.26%> (ø)
lib/ci/ci_result_parser.js 42.54% <79.31%> (-9.14%) :arrow_down:
lib/ci/build-types/job.js 81.63% <81.63%> (ø)
lib/ci/ci_utils.js 100.00% <100.00%> (ø)
lib/ci/jenkins_constants.js 100.00% <100.00%> (ø)
... and 3 more

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 d4b189b...3f4fcb1. Read the comment docs.

MylesBorins commented 3 years ago

Just realized that we might want to compare both citgm + citgm-nobuild jobs. Is that possible with this implementation ?

codebytere commented 3 years ago

@MylesBorins it does not at present - this pulls citgm-smoker jobs at the API level only so i'd need to extend this by adding the ability to fetch jobs at:

I'll make an issue and add that as a followup TODO, if that sounds good to you!

codebytere commented 3 years ago

Also - for more effective reviewing, just https://github.com/nodejs/node-core-utils/pull/455/commits/d22d5e13ea425fd22dc828c43c09eaeff343e047 is the actual new change introduced in this PR.

richardlau commented 3 years ago

Trying to make some sense of this:

-bash-4.2$ npx -p codebytere/node-core-utils#allow-citgm-comparison ncu-ci citgm 2421 2417
npx: installed 349 in 18.908s
--------------------------------------------------------------------------------
[1/1] Running CITGM: 2421
--------------------------------------------------------------------------------
✔  Summary data downloaded
✔  Results data downloaded
✔  Summary data downloaded
✔  Results data downloaded
----------------------------------- Summary ------------------------------------
Result     FAILURE
URL        https://ci.nodejs.org/job/citgm-smoker/2421/
Source     https://api.github.com/repos/nodejs/node/git/refs/heads/v10.x
Commit     [0bd44b4a88b7] Working on v10.21.1
Date       2020-06-02 18:57:33 +0100
Author     Bethany.Griggs <Bethany.Griggs@uk.ibm.com>
----------------------------------- Summary ------------------------------------
Result     FAILURE
URL        https://ci.nodejs.org/job/citgm-smoker/2417/
Source     https://api.github.com/repos/nodejs/node/git/refs/heads/v10.22.0-proposal
Commit     [c5215d0b3da0] 2020-07-21, Version 10.22.0 'Dubnium' (LTS)
Date       2020-07-15 11:27:18 -0400
Author     Richard Lau <riclau@uk.ibm.com>
----------------------------------- Results ------------------------------------
┌────────────────────────┬─────────────────────────┬─────────────────────┬───────────────────────────────┬─────────────────────┬─────────────────────┬──────────────────┬──────────────────────┬─────────────────┬──────────────────────┐
│        (index)         │            0            │          1          │               2               │          3          │          4          │        5         │          6           │        7        │          8           │
├────────────────────────┼─────────────────────────┼─────────────────────┼───────────────────────────────┼─────────────────────┼─────────────────────┼──────────────────┼──────────────────────┼─────────────────┼──────────────────────┤
│      aix71-ppc64       │ 'path-to-regexp-v6.1.0' │    'pug-v3.0.0'     │         'uuid-v8.2.0'         │                     │                     │                  │                      │                 │                      │
│        osx1015         │     'acorn-v7.3.1'      │    'ava-v3.10.1'    │        'clinic-v6.0.2'        │ 'ember-cli-v3.19.0' │  'react-v16.13.1'   │ 'semver-v7.3.2'  │ 'underscore-v1.10.2' │                 │                      │
│   fedora-latest-x64    │    'react-v16.13.1'     │                     │                               │                     │                     │                  │                      │                 │                      │
│        osx1014         │     'acorn-v7.3.1'      │    'ava-v3.10.1'    │ 'eslint-plugin-jest-v23.18.0' │   'clinic-v6.0.2'   │ 'ember-cli-v3.19.0' │ 'fastify-v3.0.3' │   'react-v16.13.1'   │ 'semver-v7.3.2' │ 'underscore-v1.10.2' │
│ fedora-last-latest-x64 │    'react-v16.13.1'     │                     │                               │                     │                     │                  │                      │                 │                      │
│     ubuntu1804-64      │    'react-v16.13.1'     │                     │                               │                     │                     │                  │                      │                 │                      │
│     centos7-ppcle      │     'clinic-v6.0.2'     │ 'ember-cli-v3.19.0' │                               │                     │                     │                  │                      │                 │                      │
│     ubuntu1604-64      │                         │                     │                               │                     │                     │                  │                      │                 │                      │
│     ubuntu1404-64      │                         │                     │                               │                     │                     │                  │                      │                 │                      │
│       debian9-64       │    'react-v16.13.1'     │                     │                               │                     │                     │                  │                      │                 │                      │
│      rhel7-s390x       │ 'torrent-stream-v1.2.0' │                     │                               │                     │                     │                  │                      │                 │                      │
└────────────────────────┴─────────────────────────┴─────────────────────┴───────────────────────────────┴─────────────────────┴─────────────────────┴──────────────────┴──────────────────────┴─────────────────┴──────────────────────┘
-bash-4.2$

This shows, for example, 'torrent-stream-v1.2.0' for rhel-s390x but that module appears as failing in both citgm runs. image image

codebytere commented 3 years ago

@richardlau spotted the error - fix incoming!

codebytere commented 3 years ago

@richardlau you should see:

node-core-utils on git:allow-citgm-comparison ❯ ncu-ci citgm 2421 2417
--------------------------------------------------------------------------------
[1/1] Running CITGM: 2421
--------------------------------------------------------------------------------
✔  Summary data downloaded
✔  Results data downloaded
✔  Summary data downloaded
✔  Results data downloaded
----------------------------------- Summary ------------------------------------
Result     FAILURE
URL        https://ci.nodejs.org/job/citgm-smoker/2421/
Source     https://api.github.com/repos/nodejs/node/git/refs/heads/v10.x
Commit     [0bd44b4a88b7] Working on v10.21.1
Date       2020-06-02 18:57:33 +0100
Author     Bethany.Griggs <Bethany.Griggs@uk.ibm.com>
----------------------------------- Summary ------------------------------------
Result     FAILURE
URL        https://ci.nodejs.org/job/citgm-smoker/2417/
Source     https://api.github.com/repos/nodejs/node/git/refs/heads/v10.22.0-proposal
Commit     [c5215d0b3da0] 2020-07-21, Version 10.22.0 'Dubnium' (LTS)
Date       2020-07-15 11:27:18 -0400
Author     Richard Lau <riclau@uk.ibm.com>
----------------------------------- Results ------------------------------------

FAILURE: 9 failures in 2417 not present in 2421

┌────────────────────────┬─────────────────────┬──────────────────┐
│        (index)         │          0          │        1         │
├────────────────────────┼─────────────────────┼──────────────────┤
│      aix71-ppc64       │ 'ember-cli-v3.19.0' │                  │
│     ubuntu1804-64      │                     │                  │
│ fedora-last-latest-x64 │ 'spawn-wrap-v2.0.0' │ 'winston-v3.3.3' │
│        osx1014         │  'fastify-v3.0.3'   │                  │
│     ubuntu1404-64      │                     │                  │
│      rhel7-s390x       │ 'uglify-js-v3.10.0' │                  │
│        osx1015         │                     │                  │
│     centos7-ppcle      │ 'ember-cli-v3.19.0' │ 'multer-v1.4.2'  │
│     ubuntu1604-64      │   'clinic-v6.0.2'   │ 'yargs-v15.4.1'  │
│       debian9-64       │                     │                  │
│   fedora-latest-x64    │                     │                  │
└────────────────────────┴─────────────────────┴──────────────────┘

now which makes far more sense!

MylesBorins commented 3 years ago

At a cursory glance it seems like the --no-build flag is a binary flag for the entire tool, not for independent jobs... so there would not be a way to compare a regular CITGM job to a no-build job, is that accurate?

Could we potentially also allow a small grammar here, rather than a flag... perhaps ${jobType}:${jobNumber}

e.g.

1337 ~ citgm job 1337 citgm:1337 ~ citgm job 1337 nobuild:1337 ~ nobuild job 1337

thoughts?

codebytere commented 3 years ago

@MylesBorins the way i've written it at present is that for comparison jobs, if --nobuild is passed that would apply to the base job:

https://github.com/nodejs/node-core-utils/pull/455/files#diff-5bdf2b1736f8191ae8dc6098a8f25651R6-R21

so if one passed nci-ci citgm 880 2433 --nobuild that would compare 880, a nobuild job, to 2433, a regular job. However, i do agree that that might be a little confusing 🤔

MylesBorins commented 3 years ago

@codebytere as long as it is documented and possible that seems like a good way to move forward... and we could revisit later if we want to improve the dx