Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tools: update doctool dependencies, migrate to ESM #38966

Closed
wants to merge 4 commits into from

Conversation

targos
Copy link
Member

@targos targos commented Jun 8, 2021

  • Migrated to ESM because some dependencies now require it.
  • Did not update highlight.js to v11 because it has many breaking
    changes.
  • Used non-deprecated highlight.js API.

Refs: highlightjs/highlight.js#2277

@github-actions github-actions bot added build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory. labels Jun 8, 2021
@targos targos mentioned this pull request Jun 8, 2021
@targos
Copy link
Member Author

targos commented Jun 8, 2021

Note to myself: need to verify that the output didn't change in a bad way.

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

tools/doc/allhtml.mjs Outdated Show resolved Hide resolved
tools/doc/generate.mjs Outdated Show resolved Hide resolved
Copy link
Member

@Trott Trott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM if the resulting diff between docs before/after this PR isn't significant.

@Trott
Copy link
Member

Trott commented Jun 9, 2021

Maybe add Fixes: https://github.com/nodejs/node/issues/38938 or Closes: https://github.com/nodejs/node/issues/38938 to the commit message?

@targos
Copy link
Member Author

targos commented Jun 9, 2021

Somehow, this seems to break coverage, but I don't know how.

@targos
Copy link
Member Author

targos commented Jun 9, 2021

@targos
Copy link
Member Author

targos commented Jun 9, 2021

I see no visual difference in addons.html, embedding.html, or n-api.html.

@Trott
Copy link
Member

Trott commented Jun 9, 2021

Somehow, this seems to break coverage, but I don't know how.

@bcoe Any ideas? I don't see anything (other than exit with status code 1) in the GitHub Action output that indicates what the problem might be, but maybe I'm looking in the wrong place or something.

@richardlau
Copy link
Member

Somehow, this seems to break coverage, but I don't know how.

@bcoe Any ideas? I don't see anything (other than exit with status code 1) in the GitHub Action output that indicates what the problem might be, but maybe I'm looking in the wrong place or something.

Mixed in with the report table are some error lines about coverage not meeting global thresholds:

ERROR: Coverage for lines (85.78%) does not meet global threshold (95%)
  buffer.js                      |   77.95 |     93.6 |   52.32 |   77.95 | ...-913,927-929,939-941 
ERROR: Coverage for branches (92.32%) does not meet global threshold (93%)
  child_process.js               |   96.55 |    90.03 |   97.87 |   96.55 | ...,805-806,851,910-914 
ERROR: Coverage for statements (85.78%) does not meet global threshold (95%)

@Trott
Copy link
Member

Trott commented Jun 9, 2021

Somehow, this seems to break coverage, but I don't know how.

@bcoe Any ideas? I don't see anything (other than exit with status code 1) in the GitHub Action output that indicates what the problem might be, but maybe I'm looking in the wrong place or something.

Mixed in with the report table are some error lines about coverage not meeting global thresholds:

ERROR: Coverage for lines (85.78%) does not meet global threshold (95%)
  buffer.js                      |   77.95 |     93.6 |   52.32 |   77.95 | ...-913,927-929,939-941 
ERROR: Coverage for branches (92.32%) does not meet global threshold (93%)
  child_process.js               |   96.55 |    90.03 |   97.87 |   96.55 | ...,805-806,851,910-914 
ERROR: Coverage for statements (85.78%) does not meet global threshold (95%)

So these changes reduce coverage (or at least that's what the GitHub Action thinks) and therefore the coverage job is failing? I didn't know (or maybe I've just forgotten?) that we did that.

@targos
Copy link
Member Author

targos commented Jun 9, 2021

Yeah, the reason of the failure seems to be a reduction of coverage, but I still don't know how this change can impact coverage at all!

@richardlau
Copy link
Member

richardlau commented Jun 9, 2021

FWIW I copy/pasted the reports from

and ran them through diff. The drop in coverage is significant in some cases but I can't easily tell if that's due to the updates to the tool itself made by this PR (which is possible as the doc generation will be run with coverage enabled) or an issue with coverage and ESM.

--- master.txt  2021-06-09 14:21:04.050000000 +0100
+++ esm.txt     2021-06-09 14:22:06.490000000 +0100
@@ -1,14 +1,14 @@
 ---------------------------------|---------|----------|---------|---------|-------------------------
 File                             | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s
 ---------------------------------|---------|----------|---------|---------|-------------------------
-All files                        |   96.86 |    94.46 |    96.7 |   96.86 |
- lib                             |    97.9 |    95.77 |   96.96 |    97.9 |
+All files                        |   85.77 |    92.31 |   77.58 |   85.77 |
+ lib                             |   88.34 |    94.47 |   79.74 |   88.34 |
   _http_agent.js                 |   98.88 |     96.4 |     100 |   98.88 | 154-155,210-211,359-360
   _http_client.js                |   98.77 |    97.16 |     100 |   98.77 | ...-345,837-838,844-845
   _http_common.js                |     100 |      100 |     100 |     100 |
   _http_incoming.js              |   97.68 |    99.12 |   88.24 |   97.68 | ...,141-142,213-216,300
   _http_outgoing.js              |    96.7 |    97.21 |   97.92 |    96.7 | ...,856,941-943,949-951
-  _http_server.js                |   98.19 |    95.85 |     100 |   98.19 | ...-727,729-730,912-914
+  _http_server.js                |   98.39 |    95.41 |     100 |   98.39 | ...-727,729-730,912-914
   _stream_duplex.js              |     100 |      100 |     100 |     100 |
   _stream_passthrough.js         |     100 |      100 |     100 |     100 |
   _stream_readable.js            |     100 |      100 |     100 |     100 |
@@ -18,8 +18,8 @@
   _tls_common.js                 |      98 |    95.35 |     100 |      98 | 73-75
   _tls_wrap.js                   |   96.17 |    85.88 |     100 |   96.17 | ...,1444-1445,1463-1464
   assert.js                      |   98.33 |    96.67 |     100 |   98.33 | ...-280,361-362,380-381
-  async_hooks.js                 |     100 |      100 |     100 |     100 |
-  buffer.js                      |   99.37 |    98.06 |   97.65 |   99.37 | 611,656-660,1041-1042
+  async_hooks.js                 |   60.34 |    89.77 |      55 |   60.34 | ...-316,319-322,329-334
+  buffer.js                      |   67.19 |    92.37 |   61.08 |   67.19 | ...,1205-1213,1221-1227
   child_process.js               |   98.86 |    98.08 |   96.15 |   98.86 | ...-596,616-617,624-626
   cluster.js                     |     100 |       50 |     100 |     100 | 24
   console.js                     |     100 |      100 |     100 |     100 |
@@ -29,8 +29,8 @@
   diagnostics_channel.js         |     100 |      100 |      90 |     100 |
   dns.js                         |   98.83 |    92.86 |     100 |   98.83 | 79-80,119-120
   domain.js                      |   98.32 |    96.97 |     100 |   98.32 | 111-112,290-292,348-351
-  events.js                      |   99.41 |    97.89 |     100 |   99.41 | 822-823,864-865,880-881
-  fs.js                          |   96.16 |    94.33 |   93.08 |   96.16 | ...,2942,2951,2962,2971
+  events.js                      |   62.64 |    93.23 |    61.8 |   62.64 | ...,867,880-881,883-890
+  fs.js                          |   64.16 |    89.98 |   57.14 |   64.16 | ...,2942,2951,2962,2971
   http.js                        |     100 |      100 |     100 |     100 |
   http2.js                       |     100 |      100 |     100 |     100 |
   https.js                       |     100 |      100 |     100 |     100 |
@@ -38,24 +38,24 @@
   module.js                      |     100 |      100 |     100 |     100 |
   net.js                         |   96.43 |    95.62 |   96.77 |   96.43 | ...-1577,1660,1708-1730
   os.js                          |   97.46 |    84.62 |     100 |   97.46 | 176-181,233-234,237-238
-  path.js                        |   96.82 |    97.14 |     100 |   96.82 | ...00,964-965,1072-1079
+  path.js                        |   64.03 |    93.94 |      66 |   64.03 | ...,1274,1304,1340-1385
   perf_hooks.js                  |   89.13 |      100 |      50 |   89.13 | 45-47,50-61
   process.js                     |     100 |      100 |     100 |     100 |
   punycode.js                    |   96.67 |    92.59 |     100 |   96.67 | ...-277,345-346,353-354
   querystring.js                 |     100 |      100 |     100 |     100 |
   readline.js                    |   97.81 |    97.74 |   98.21 |   97.81 | ...25-926,988,1128-1149
   repl.js                        |    96.7 |    93.07 |    87.5 |    96.7 | ...,1745-1747,1750-1752
-  stream.js                      |     100 |      100 |     100 |     100 |
+  stream.js                      |   89.87 |      100 |      80 |   89.87 | 47-52,55-56
   string_decoder.js              |   98.46 |    90.48 |     100 |   98.46 | 59-60
   sys.js                         |     100 |      100 |     100 |     100 |
-  timers.js                      |     100 |      100 |     100 |     100 |
+  timers.js                      |   89.97 |      100 |      50 |   89.97 | ...,267,272-274,276-294
   tls.js                         |     100 |    85.54 |     100 |     100 | ...,180,188,193,212,224
   trace_events.js                |     100 |    94.12 |     100 |     100 | 25
   tty.js                         |   98.18 |    89.74 |     100 |   98.18 | 78-80
-  url.js                         |   99.61 |     98.5 |     100 |   99.61 | 876-878,896
+  url.js                         |   82.99 |     98.5 |      50 |   82.99 | ...,876-878,896,946-951
   util.js                        |     100 |      100 |     100 |     100 |
   v8.js                          |     100 |      100 |     100 |     100 |
-  vm.js                          |   98.59 |    96.97 |     100 |   98.59 | 365-366,369-370,407-408
+  vm.js                          |   57.51 |    96.97 |   51.52 |   57.51 | ...-377,391-397,407-408
   wasi.js                        |     100 |      100 |     100 |     100 |
   worker_threads.js              |     100 |      100 |     100 |     100 |
   zlib.js                        |   99.27 |    95.58 |   95.24 |   99.27 | ...-393,395-396,726-727
@@ -65,21 +65,21 @@
   promises.js                    |     100 |      100 |     100 |     100 |
  lib/fs                          |     100 |      100 |     100 |     100 |
   promises.js                    |     100 |      100 |     100 |     100 |
- lib/internal                    |   96.35 |    95.58 |   97.36 |   96.35 |
+ lib/internal                    |   82.25 |    92.61 |   73.89 |   82.25 |
   abort_controller.js            |     100 |    96.15 |     100 |     100 | 54
   assert.js                      |     100 |      100 |     100 |     100 |
-  async_hooks.js                 |   99.16 |    98.43 |     100 |   99.16 | 174-175,527-529
+  async_hooks.js                 |   51.77 |    98.45 |      55 |   51.77 | ...-539,541-543,545-548
   blob.js                        |   99.58 |       90 |     100 |   99.58 | 184
   blocklist.js                   |     100 |     93.1 |     100 |     100 | 52,156
-  buffer.js                      |     100 |    96.39 |     100 |     100 | ...17,137,155,1021-1028
-  child_process.js               |      96 |    90.03 |   97.87 |      96 | ...-806,849-855,910-914
+  buffer.js                      |   77.95 |     93.6 |   52.32 |   77.95 | ...-913,927-929,939-941
+  child_process.js               |      96 |    90.06 |   97.87 |      96 | ...-806,849-855,910-914
   cli_table.js                   |     100 |    93.33 |     100 |     100 | 68
   constants.js                   |     100 |       50 |     100 |     100 | 55
   dgram.js                       |     100 |      100 |     100 |     100 |
   dtrace.js                      |     100 |     87.5 |     100 |     100 | 12
-  encoding.js                    |   85.29 |    93.24 |   94.44 |   85.29 | 433-516
+  encoding.js                    |   78.28 |    82.42 |   63.89 |   78.28 | ...-527,530-532,536-538
   error_serdes.js                |   98.59 |    91.67 |     100 |   98.59 | 109,139
-  errors.js                      |   98.21 |    94.52 |    97.5 |   98.21 | ...,1235-1236,1515-1517
+  errors.js                      |    75.3 |    88.04 |   78.57 |    75.3 | ...,1459-1465,1515-1517
   event_target.js                |     100 |    98.39 |     100 |     100 | 122,160,485
   fixed_queue.js                 |     100 |      100 |     100 |     100 |
   freelist.js                    |     100 |      100 |     100 |     100 |
@@ -87,40 +87,40 @@
   heap_utils.js                  |     100 |      100 |     100 |     100 |
   histogram.js                   |   99.02 |    94.29 |     100 |   99.02 | 115-116
   http.js                        |     100 |      100 |     100 |     100 |
-  idna.js                        |   66.67 |       50 |     100 |   66.67 | 7-9
+  idna.js                        |   77.78 |       50 |     100 |   77.78 | 8-9
   inspector_async_hook.js        |   86.84 |     87.5 |     100 |   86.84 | 53-62
   js_stream_socket.js            |   98.76 |      100 |   88.89 |   98.76 | 25,111-112
   linkedlist.js                  |     100 |      100 |     100 |     100 |
   net.js                         |   75.36 |      100 |      75 |   75.36 | 45-61
-  options.js                     |     100 |      100 |     100 |     100 |
-  priority_queue.js              |     100 |     97.3 |     100 |     100 | 40
-  querystring.js                 |     100 |      100 |     100 |     100 |
+  options.js                     |      50 |    85.71 |      75 |      50 | 14-29
+  priority_queue.js              |   99.15 |     97.3 |   88.89 |   99.15 | 117
+  querystring.js                 |   92.44 |      100 |      50 |   92.44 | 36-44
   repl.js                        |     100 |      100 |     100 |     100 |
   socket_list.js                 |     100 |      100 |     100 |     100 |
   socketaddress.js               |     100 |    92.31 |     100 |     100 | 103-107
   stream_base_commons.js         |   96.18 |    93.33 |     100 |   96.18 | ...-206,217-218,230-234
-  timers.js                      |    99.7 |    98.68 |     100 |    99.7 | 344-345
+  timers.js                      |    59.7 |    98.68 |   53.33 |    59.7 | ...-624,626-633,635-638
   tls.js                         |   95.93 |    93.52 |     100 |   95.93 | ...-217,275-278,311-312
   trace_events_async_hooks.js    |   93.75 |    94.74 |   85.71 |   93.75 | 87-92
   tty.js                         |   97.47 |    98.75 |     100 |   97.47 | 150-155
-  url.js                         |    93.9 |    96.72 |   97.18 |    93.9 | ...,1411-1430,1455-1475
-  util.js                        |   99.58 |    98.48 |     100 |   99.58 | 286-287
+  url.js                         |   62.27 |    88.85 |   71.13 |   62.27 | ...-1444,1448,1455-1475
+  util.js                        |    74.9 |    94.81 |   61.54 |    74.9 | ...-378,394-398,401-404
   v8_prof_polyfill.js            |   31.79 |    81.82 |   16.67 |   31.79 | 39-91,93-122,139-173
   v8_prof_processor.js           |   94.34 |    66.67 |   66.67 |   94.34 | 26,28-29
-  validators.js                  |     100 |      100 |     100 |     100 |
+  validators.js                  |   70.24 |    92.04 |   72.22 |   70.24 | ...,159-167,169-179,189
   watchdog.js                    |   76.27 |     87.5 |      80 |   76.27 | 26-27,42-53
   worker.js                      |   98.47 |    95.24 |   95.45 |   98.47 | 421-424,484-487
  lib/internal/assert             |     100 |      100 |      90 |     100 |
   assertion_error.js             |     100 |      100 |     100 |     100 |
   calltracker.js                 |     100 |      100 |      75 |     100 |
- lib/internal/bootstrap          |   97.37 |    94.44 |   92.31 |   97.37 |
+ lib/internal/bootstrap          |   88.95 |    87.72 |   86.44 |   88.95 |
   environment.js                 |     100 |      100 |     100 |     100 |
-  loaders.js                     |   96.89 |    94.34 |   93.33 |   96.89 | 148-152,248-251,348-349
-  node.js                        |   98.25 |      100 |   83.33 |   98.25 | ...94-95,99-100,118-120
-  pre_execution.js               |   96.71 |     91.8 |     100 |   96.71 | 162-166,284-290,370-373
- lib/internal/bootstrap/switches |   97.52 |    96.77 |   84.62 |   97.52 |
+  loaders.js                     |   89.55 |     90.7 |      80 |   89.55 | ...,248,320-326,348-349
+  node.js                        |    96.3 |      100 |      80 |    96.3 | ...-120,196-198,202-205
+  pre_execution.js               |   80.49 |    80.58 |   94.34 |   80.49 | ...,363-406,442-445,458
+ lib/internal/bootstrap/switches |   86.94 |    96.94 |   73.53 |   86.94 |
   does_not_own_process_state.js  |   94.44 |    66.67 |     100 |   94.44 | 34-35
-  does_own_process_state.js      |   99.22 |    96.15 |     100 |   99.22 | 57
+  does_own_process_state.js      |    62.5 |    96.67 |   68.75 |    62.5 | ...1-99,113-116,120-123
   is_main_thread.js              |    96.6 |    98.25 |   66.67 |    96.6 | 65-70,88-89
   is_not_main_thread.js          |     100 |      100 |     100 |     100 |
  lib/internal/child_process      |   96.21 |    80.95 |     100 |   96.21 |
@@ -132,8 +132,8 @@
   shared_handle.js               |     100 |      100 |     100 |     100 |
   utils.js                       |     100 |      100 |     100 |     100 |
   worker.js                      |     100 |      100 |     100 |     100 |
- lib/internal/console            |     100 |      100 |     100 |     100 |
-  constructor.js                 |     100 |      100 |     100 |     100 |
+ lib/internal/console            |   80.52 |      100 |   56.52 |   80.52 |
+  constructor.js                 |   78.97 |      100 |   56.52 |   78.97 | ...,609-619,623,648-649
   global.js                      |     100 |      100 |     100 |     100 |
  lib/internal/crypto             |    94.6 |    85.94 |   98.49 |    94.6 |
   aes.js                         |   90.78 |    69.86 |     100 |   90.78 | ...-298,305-306,325-327
@@ -159,23 +159,23 @@
  lib/internal/dns                |   99.58 |     99.1 |     100 |   99.58 |
   promises.js                    |   99.23 |    98.53 |     100 |   99.23 | 116-117
   utils.js                       |     100 |      100 |     100 |     100 |
- lib/internal/fs                 |   93.85 |    92.01 |   94.44 |   93.85 |
-  dir.js                         |     100 |      100 |     100 |     100 |
-  promises.js                    |   99.12 |    96.52 |     100 |   99.12 | 185-186,342-343,654-656
+ lib/internal/fs                 |   71.39 |    86.16 |   61.09 |   71.39 |
+  dir.js                         |   82.39 |      100 |      55 |   82.39 | ...-213,229-234,257-261
+  promises.js                    |   66.29 |    87.23 |   57.72 |   66.29 | ...,680,683-688,693-694
   read_file_context.js           |   98.44 |    94.44 |     100 |   98.44 | 65-66
-  rimraf.js                      |   67.74 |    61.02 |   81.82 |   67.74 | ...,262-274,276,283-307
-  streams.js                     |   95.13 |    93.29 |      84 |   95.13 | ...,431-434,437-438,456
+  rimraf.js                      |   51.94 |    61.02 |   42.86 |   51.94 | ...-232,240-241,262-307
+  streams.js                     |   61.66 |    93.29 |   53.85 |   61.66 | ...,431-434,437-438,456
   sync_write_stream.js           |     100 |     87.5 |     100 |     100 | 32
-  utils.js                       |   96.39 |    96.17 |     100 |   96.39 | ...-395,489-490,519-520
+  utils.js                       |   71.66 |    84.71 |   66.23 |   71.66 | ...-732,739-761,766-776
   watchers.js                    |   89.42 |    78.79 |   81.82 |   89.42 | ...-341,348-358,362-364
  lib/internal/http2              |   97.88 |    93.85 |   97.98 |   97.88 |
   compat.js                      |    99.2 |    95.31 |     100 |    99.2 | 229,247,721-722,803-805
   core.js                        |   97.11 |    92.77 |   96.71 |   97.11 | ...,3189-3191,3244-3245
   util.js                        |     100 |    99.15 |     100 |     100 | 520
- lib/internal/inspector          |   91.43 |     86.3 |   88.62 |   91.43 |
+ lib/internal/inspector          |   91.43 |    86.34 |   88.62 |   91.43 |
   _inspect.js                    |   95.01 |    91.78 |   95.83 |   95.01 | ...-300,337-338,368-371
   inspect_client.js              |   87.64 |    76.27 |   93.33 |   87.64 | ...,285-287,330,333-338
-  inspect_repl.js                |   91.45 |    87.06 |   85.71 |   91.45 | ...80-981,985,1016-1018
+  inspect_repl.js                |   91.45 |    87.11 |   85.71 |   91.45 | ...80-981,985,1016-1018
  lib/internal/legacy             |     100 |      100 |     100 |     100 |
   processbinding.js              |     100 |      100 |     100 |     100 |
  lib/internal/main               |   95.97 |    85.85 |     100 |   95.97 |
@@ -188,26 +188,26 @@
   repl.js                        |   81.54 |       70 |     100 |   81.54 | 30-34,43-44,47-51
   run_main_module.js             |     100 |      100 |     100 |     100 |
   worker_thread.js               |   97.22 |    83.87 |     100 |   97.22 | ...,176,221-222,237-238
- lib/internal/modules            |     100 |    97.62 |     100 |     100 |
-  package_json_reader.js         |     100 |      100 |     100 |     100 |
-  run_main.js                    |     100 |    96.67 |     100 |     100 | 48
- lib/internal/modules/cjs        |   94.82 |    94.42 |   93.55 |   94.82 |
-  helpers.js                     |   99.48 |    94.74 |     100 |   99.48 | 74
-  loader.js                      |   94.11 |    94.39 |      92 |   94.11 | ...,1147-1149,1256-1258
- lib/internal/modules/esm        |   96.05 |    90.81 |   96.15 |   96.05 |
+ lib/internal/modules            |   97.64 |    81.82 |     100 |   97.64 |
+  package_json_reader.js         |   97.56 |       90 |     100 |   97.56 | 33
+  run_main.js                    |   97.67 |    78.26 |     100 |   97.67 | 59-60
+ lib/internal/modules/cjs        |   77.38 |    87.12 |   74.17 |   77.38 |
+  helpers.js                     |   88.66 |    91.84 |   80.95 |   88.66 | ...91,94-97,124-125,132
+  loader.js                      |   75.67 |    86.71 |   72.73 |   75.67 | ...,1215-1216,1256-1258
+ lib/internal/modules/esm        |   84.29 |    83.23 |   80.58 |   84.29 |
   create_dynamic_module.js       |   95.83 |     87.5 |   71.43 |   95.83 | 59-61
-  get_format.js                  |   93.98 |    76.92 |     100 |   93.98 | 66-69,76
-  get_source.js                  |   91.11 |       80 |     100 |   91.11 | 33-34,38-39
-  loader.js                      |   89.14 |    85.33 |     100 |   89.14 | ...,220-221,224-226,237
-  module_job.js                  |     100 |      100 |     100 |     100 |
-  module_map.js                  |     100 |      100 |     100 |     100 |
-  resolve.js                     |   97.79 |    93.22 |     100 |   97.79 | ...-803,805-810,832-834
+  get_format.js                  |   92.77 |    73.68 |     100 |   92.77 | 66-69,76,78
+  get_source.js                  |   91.11 |    72.22 |     100 |   91.11 | 33-34,38-39
+  loader.js                      |    83.9 |    78.22 |   83.33 |    83.9 | ...-214,220-221,224-226
+  module_job.js                  |     100 |    95.08 |     100 |     100 | 103,110,157
+  module_map.js                  |   97.06 |    84.62 |   85.71 |   97.06 | 27
+  resolve.js                     |   76.97 |    85.99 |   74.58 |   76.97 | ...-803,805-810,832-834
   transform_source.js            |     100 |      100 |     100 |     100 |
-  translators.js                 |   95.31 |    86.14 |   93.75 |   95.31 | ...-311,325-329,368-370
- lib/internal/per_context        |   98.62 |    95.24 |   96.67 |   98.62 |
+  translators.js                 |   87.24 |    75.74 |      80 |   87.24 | ...-329,350-355,368-370
+ lib/internal/per_context        |   98.62 |    95.35 |   96.77 |   98.62 |
   domexception.js                |   96.72 |    82.35 |    87.5 |   96.72 | 63-64,72-73
   messageport.js                 |     100 |      100 |     100 |     100 |
-  primordials.js                 |   99.05 |    98.25 |     100 |   99.05 | 107-110
+  primordials.js                 |   99.05 |    98.31 |     100 |   99.05 | 107-110
  lib/internal/perf               |   92.54 |    90.36 |   94.12 |   92.54 |
   event_loop_delay.js            |   94.55 |       90 |     100 |   94.55 | 26-28
   event_loop_utilization.js      |     100 |      100 |     100 |     100 |
@@ -219,16 +219,16 @@
  lib/internal/policy             |   92.12 |    87.68 |   94.74 |   92.12 |
   manifest.js                    |   91.07 |     87.2 |   94.44 |   91.07 | ...,392-395,399-402,465
   sri.js                         |     100 |    92.31 |     100 |     100 | 57
- lib/internal/process            |   97.77 |     94.7 |   96.88 |   97.77 |
+ lib/internal/process            |   72.29 |    92.09 |   63.19 |   72.29 |
   esm_loader.js                  |   93.67 |    89.47 |     100 |   93.67 | 49-50,71-73
-  execution.js                   |   98.21 |    94.12 |     100 |   98.21 | 51-52,180-181
-  per_thread.js                  |   99.25 |    96.43 |     100 |   99.25 | 338,388-389
+  execution.js                   |   91.52 |    83.87 |   70.59 |   91.52 | ...-103,180-181,197-202
+  per_thread.js                  |   71.11 |    94.19 |   77.14 |   71.11 | ...-347,362-367,388-389
   policy.js                      |   75.41 |    71.43 |      40 |   75.41 | ...40,45-48,52-55,59-60
-  promises.js                    |     100 |    93.65 |     100 |     100 | 171-172,195,229
-  report.js                      |     100 |      100 |     100 |     100 |
-  signal.js                      |   94.44 |    90.91 |     100 |   94.44 | 35-37
-  task_queues.js                 |     100 |      100 |     100 |     100 |
-  warning.js                     |   96.67 |    91.78 |     100 |   96.67 | 32-33,59-60,69-70
+  promises.js                    |   68.21 |    93.75 |   53.85 |   68.21 | ...,264,268-280,283-293
+  report.js                      |    57.5 |    97.06 |   55.56 |    57.5 | ...,101-105,112,118-120
+  signal.js                      |   85.19 |       80 |   83.33 |   85.19 | 35-42
+  task_queues.js                 |    43.1 |      100 |   56.25 |    43.1 | ...-136,140-147,152-163
+  warning.js                     |   78.33 |    91.78 |      50 |   78.33 | ...,148,150,152-155,159
   worker_thread_only.js          |     100 |      100 |     100 |     100 |
  lib/internal/readline           |   99.19 |    99.12 |     100 |   99.19 |
   callbacks.js                   |     100 |      100 |     100 |     100 |
@@ -238,42 +238,42 @@
   await.js                       |     100 |    96.88 |     100 |     100 | 111
   history.js                     |      92 |    83.33 |     100 |      92 | ...-135,144-145,156-158
   utils.js                       |    97.5 |    93.72 |     100 |    97.5 | ...-398,414-416,613-614
- lib/internal/source_map         |   94.33 |    90.74 |   93.55 |   94.33 |
+ lib/internal/source_map         |   91.18 |    88.24 |   73.81 |   91.18 |
   prepare_stack_trace.js         |   97.93 |    88.46 |     100 |   97.93 | 35-38
   source_map.js                  |   91.01 |    96.49 |   86.67 |   91.01 | ...,162,168-177,204-205
-  source_map_cache.js            |   96.33 |    86.79 |     100 |   96.33 | 194-195,202,207-212
- lib/internal/streams            |    98.7 |    95.99 |    98.1 |    98.7 |
+  source_map_cache.js            |   86.12 |    80.33 |   60.87 |   86.12 | ...-195,202,204,207-214
+ lib/internal/streams            |   62.17 |    95.99 |   57.22 |   62.17 |
   add-abort-signal.js            |     100 |      100 |     100 |     100 |
-  buffer_list.js                 |     100 |      100 |     100 |     100 |
-  destroy.js                     |   97.37 |    94.66 |     100 |   97.37 | ...-153,332,346,353-354
+  buffer_list.js                 |   70.17 |      100 |      50 |   70.17 | ...,107,134-138,161-170
+  destroy.js                     |   82.11 |    94.66 |    61.9 |   82.11 | ...-177,332,346,353-354
   duplex.js                      |     100 |      100 |     100 |     100 |
   duplexpair.js                  |   98.04 |     87.5 |     100 |   98.04 | 29
-  end-of-stream.js               |   99.57 |    95.87 |   93.75 |   99.57 | 146
+  end-of-stream.js               |   87.98 |    95.87 |   65.22 |   87.98 | ...9,41-45,49-54,61,146
   from.js                        |     100 |    97.83 |     100 |     100 | 61
   lazy_transform.js              |     100 |      100 |     100 |     100 |
-  legacy.js                      |   96.49 |       80 |     100 |   96.49 | 108-111
+  legacy.js                      |   95.61 |       80 |   81.82 |   95.61 | 9,108-111
   passthrough.js                 |     100 |      100 |     100 |     100 |
-  pipeline.js                    |   95.15 |    93.02 |     100 |   95.15 | ...,117,186-189,256-259
-  readable.js                    |   99.49 |    98.13 |   95.77 |   99.49 | ...-1266,1272,1275-1276
-  state.js                       |     100 |      100 |     100 |     100 |
-  transform.js                   |   95.93 |    85.19 |     100 |   95.93 | ...-156,227-228,235-236
+  pipeline.js                    |   37.31 |    93.02 |   41.18 |   37.31 | ...,154-218,221,223-268
+  readable.js                    |   38.21 |    98.13 |   53.54 |   38.21 | ...-1350,1358,1360-1361
+  state.js                       |   69.44 |      100 |      75 |   69.44 | 19-29
+  transform.js                   |    68.7 |    85.19 |      40 |    68.7 | ...-230,232-240,243-246
   utils.js                       |     100 |    94.44 |     100 |     100 | 25
-  writable.js                    |   99.08 |    95.89 |     100 |   99.08 | ...-669,698-699,704-705
+  writable.js                    |   63.42 |    95.89 |      50 |   63.42 | ...,756-760,768-769,772
  lib/internal/test               |   94.83 |       75 |     100 |   94.83 |
   binding.js                     |   81.25 |       50 |     100 |   81.25 | 12-14
   transfer.js                    |     100 |    83.33 |     100 |     100 | 28
- lib/internal/util               |   97.45 |    99.39 |   96.61 |   97.45 |
+ lib/internal/util               |   68.98 |    93.21 |   68.42 |   68.98 |
   comparisons.js                 |     100 |      100 |     100 |     100 |
-  debuglog.js                    |     100 |     93.1 |   91.67 |     100 | 84,98
-  inspect.js                     |   96.47 |    99.87 |   95.16 |   96.47 | 2036-2110
-  inspector.js                   |     100 |    83.33 |     100 |     100 | 13,26
+  debuglog.js                    |   95.73 |    89.74 |   78.26 |   95.73 | 37,53-54,82-83
+  inspect.js                     |   56.35 |    91.04 |    61.6 |   56.35 | ...,2014-2018,2028-2110
+  inspector.js                   |   93.33 |    88.24 |      70 |   93.33 | 7-10,68
   iterable_weak_map.js           |     100 |      100 |     100 |     100 |
-  types.js                       |    96.3 |    89.47 |     100 |    96.3 | 83-84,98-99
- lib/internal/vm                 |   98.91 |    97.73 |     100 |   98.91 |
-  module.js                      |   98.91 |    97.73 |     100 |   98.91 | 373-374,419,445-446
- lib/internal/worker             |   96.54 |    98.78 |   97.06 |   96.54 |
+  types.js                       |   76.85 |    90.48 |   61.54 |   76.85 | ...52,55-57,83-84,98-99
+ lib/internal/vm                 |   52.62 |    97.73 |   55.26 |   52.62 |
+  module.js                      |   52.62 |    97.73 |   55.26 |   52.62 | ...,433-434,442-456,458
+ lib/internal/worker             |   96.14 |     98.8 |   97.14 |   96.14 |
   io.js                          |    96.1 |    98.65 |   96.88 |    96.1 | 367-379,404-407
-  js_transferable.js             |     100 |      100 |     100 |     100 |
+  js_transferable.js             |   96.43 |      100 |     100 |   96.43 | 25-26
  lib/path                        |     100 |      100 |     100 |     100 |
   posix.js                       |     100 |      100 |     100 |     100 |
   win32.js                       |     100 |      100 |     100 |     100 |

@richardlau
Copy link
Member

This is interesting -- there's a discrepancy in the number of tests being run:
https://github.com/nodejs/node/runs/2779985623?check_suite_focus=true#step:7:1320 (master)

Running 3374 tests

https://github.com/nodejs/node/runs/2780915355?check_suite_focus=true#step:7:1253 (this PR)

Running 3366 tests

@targos
Copy link
Member Author

targos commented Jun 9, 2021

I would consider that a bug in our setup if the code run to generate the documentation counts as coverage.

@targos
Copy link
Member Author

targos commented Jun 9, 2021

btw, there are 8 missing tests between these CI runs, but the doctool directory only has 6 tests.

@richardlau
Copy link
Member

Could the "missing" tests be the ones we generate from the addon docs?

@targos
Copy link
Member Author

targos commented Jun 9, 2021

Could the "missing" tests be the ones we generate from the addon docs?

Yes!

/Applications/Xcode.app/Contents/Developer/usr/bin/make -s build-addons
ninja: Entering directory `out/Release'
ninja: no work to do.
node:internal/process/esm_loader:74
    internalBinding('errors').triggerUncaughtException(
                              ^

TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received undefined
    at new NodeError (node:internal/errors:363:5)
    at validateString (node:internal/validators:119:11)
    at Object.resolve (node:path:1098:7)
    at Function.readSync (file:///Users/targos/git/nodejs/node/tools/doc/node_modules/to-vfile/lib/index.js:55:37)
    at file:///Users/targos/git/nodejs/node/tools/doc/addon-verify.mjs:18:22
    at ModuleJob.run (node:internal/modules/esm/module_job:175:25)
    at async Loader.import (node:internal/modules/esm/loader:178:24)
    at async Object.loadESM (node:internal/process/esm_loader:68:5)
    at async handleMainPromise (node:internal/modules/run_main:63:12) {
  code: 'ERR_INVALID_ARG_TYPE'
}
node:internal/process/esm_loader:74
    internalBinding('errors').triggerUncaughtException(
                              ^

TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received undefined
    at new NodeError (node:internal/errors:363:5)
    at validateString (node:internal/validators:119:11)
    at Object.resolve (node:path:1098:7)
    at Function.readSync (file:///Users/targos/git/nodejs/node/tools/doc/node_modules/to-vfile/lib/index.js:55:37)
    at file:///Users/targos/git/nodejs/node/tools/doc/addon-verify.mjs:18:22
    at ModuleJob.run (node:internal/modules/esm/module_job:175:25)
    at async Loader.import (node:internal/modules/esm/loader:178:24)
    at async Object.loadESM (node:internal/process/esm_loader:68:5) {
  code: 'ERR_INVALID_ARG_TYPE'
}

@targos
Copy link
Member Author

targos commented Jun 9, 2021

^ so, it means that make build-addons doesn't fail when there's an error.

@richardlau
Copy link
Member

^ so, it means that make build-addons doesn't fail when there's an error.

#38983 should fix that.

- Migrated to ESM because some dependencies now require it.
- Did not update `highlight.js` to v11 because it has many breaking
  changes.
- Used non-deprecated `highlight.js` API.

Refs: highlightjs/highlight.js#2277
Fixes: nodejs#38938
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
tools/doc/package.json Outdated Show resolved Hide resolved
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
@nodejs-github-bot
Copy link
Collaborator

@aduh95 aduh95 added dont-land-on-v12.x author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels Jun 11, 2021
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@targos
Copy link
Member Author

targos commented Jun 13, 2021

I pushed a fix for Windows

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@jasnell
Copy link
Member

jasnell commented Jun 14, 2021

Landed in de01f47

jasnell pushed a commit that referenced this pull request Jun 14, 2021
- Migrated to ESM because some dependencies now require it.
- Did not update `highlight.js` to v11 because it has many breaking
  changes.
- Used non-deprecated `highlight.js` API.

Refs: highlightjs/highlight.js#2277
Fixes: #38938
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>

PR-URL: #38966
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
@jasnell jasnell closed this Jun 14, 2021
@targos targos deleted the update-doctool branch June 14, 2021 15:47
danielleadams pushed a commit that referenced this pull request Jun 15, 2021
- Migrated to ESM because some dependencies now require it.
- Did not update `highlight.js` to v11 because it has many breaking
  changes.
- Used non-deprecated `highlight.js` API.

Refs: highlightjs/highlight.js#2277
Fixes: #38938
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>

PR-URL: #38966
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
@danielleadams danielleadams mentioned this pull request Jun 15, 2021
danielleadams pushed a commit that referenced this pull request Jun 17, 2021
- Migrated to ESM because some dependencies now require it.
- Did not update `highlight.js` to v11 because it has many breaking
  changes.
- Used non-deprecated `highlight.js` API.

Refs: highlightjs/highlight.js#2277
Fixes: #38938
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>

PR-URL: #38966
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Copy link

@haquemofijul haquemofijul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#targos:update-doctool

richardlau pushed a commit that referenced this pull request Jul 19, 2021
- Migrated to ESM because some dependencies now require it.
- Did not update `highlight.js` to v11 because it has many breaking
  changes.
- Used non-deprecated `highlight.js` API.

Refs: highlightjs/highlight.js#2277
Fixes: #38938
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>

PR-URL: #38966
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
richardlau pushed a commit that referenced this pull request Jul 20, 2021
- Migrated to ESM because some dependencies now require it.
- Did not update `highlight.js` to v11 because it has many breaking
  changes.
- Used non-deprecated `highlight.js` API.

Refs: highlightjs/highlight.js#2277
Fixes: #38938
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>

PR-URL: #38966
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
@richardlau richardlau mentioned this pull request Jul 20, 2021
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
- Migrated to ESM because some dependencies now require it.
- Did not update `highlight.js` to v11 because it has many breaking
  changes.
- Used non-deprecated `highlight.js` API.

Refs: highlightjs/highlight.js#2277
Fixes: nodejs#38938
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>

PR-URL: nodejs#38966
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Richard Lau <rlau@redhat.com>
Reviewed-By: Anto Aravinth <anto.aravinth.cse@gmail.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants