test: add API surface test

[LiviaMedeiros]

Closes: #58676

Since the issue got a few thumbs up and no objections, here's the implementation.

---

This adds:

• Fixtures in test/fixtures/apiSurface/ directory, one per module. Each fixture is a text file with all properties found in the module using recursive Reflect.ownKeys() calls.
• Test test/parallel/test-api-surface.mjs that builds same lists of properties and compares it with the fixtures.
• Tool tools/update-api-surface-fixture.mjs that builds same lists and updates the fixtures.

The problem is that sometimes internal-only properties are unintentionally exposed to the userland. Inevitably, userland discovers them, uses them, depends on them. Keeping them makes hard or impossible to change the internal logic, removing them is a long and painful process.
This happened many times in the past, this keeps happening, nothing prevents it from happening again.

This test should help with this problem.
If there are unintentional changes to public API surface, the test will fail.
If the changes are intentional (usually it's only semver-minor PRs that contain new features and should be released in the next minor version. and semver-major PRs that contain breaking changes and should be released in the next major version. PRs), the author can regenerate the fixture, and the list of changes will clearly indicate what is added or removed.

Additionally, the test performs some sanity checks: properties shouldn't be nested deeper than 16 levels, the only acceptable error thrown by accessor is TypeError caused by wrong this value on prototypes.

The test is marked as flaky. It should fail locally in case of such changes, but it should not break CI on main and staging branches after landing PRs based on earlier commits, and it should not make Jenkins CI red. After it's being adopted and proven to be robust, we can consider unmarking it.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.11%. Comparing base (faada65) to head (d1f4a43).
Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #58793      +/-   ##
==========================================
+ Coverage   90.09%   90.11%   +0.01%     
==========================================
  Files         640      640              
  Lines      188271   188271              
  Branches    36923    36951      +28     
==========================================
+ Hits       169625   169655      +30     
+ Misses      11386    11329      -57     
- Partials     7260     7287      +27     

see 36 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Renegade334]

Is this fixture going to make it challenging to land PRs across different branches?

As an aside, Set.prototype.symmetricDifference() would exclude this from landing on v20.x.

[Ethan-Arrowood]

this is a cool idea. I think the scripts and test needs some work though - tests are failing

[LiviaMedeiros]

Is this fixture going to make it challenging to land PRs across different branches?

Assuming it gets backported on all release lines, it should be fine. Do you have a scenario in mind where this fixture would cause troubles that can't solved by rerunning the regeneration script?

The test will definitely fail on main after landing currently opened PRs that change API surface though, unless they all will be rebased on top of this. This is why currently the test doesn't actually throw and just logs the differences. We can also mark it as flaky as soon as it starts throwing, so it doesn't break main and staging branches.

As an aside, Set.prototype.symmetricDifference() would exclude this from landing on v20.x.

Good catch! I'll rewrite it.

I think the scripts and test needs some work though - tests are failing

Currently, tests are failing because some accessors are throwing incorrect error. #58765 should fix it.

[jasnell]

@LiviaMedeiros ... overall I think it's a good idea but the purpose here is rather vague. I know you linked to the original issue but could I ask if you can expand the PR description a bit more with some detail on how this works, what it's for, etc? That would be helpful for folks coming into the PR to review. As is, it's a bit obscure.

[jasnell]

My fear here is that apiSurface, even as a generated file, is likely to become quite massive and difficult to manage. As is we cannot even use the github UI to review it since it says, "29,926 additions... not shown because the diff is too large...". Is there any way to break it up further? Maybe generate a set of files rather than one single massive one?

[LiviaMedeiros]

Agreed. I'd expect the subsequent changes to be small enough for GitHub UI to handle them, but splitting them per-module looks nicer.

$ wc -l test/fixtures/apiSurface/* | sed s#test/fixtures/apiSurface/##
    684 assert
    684 assert-strict
    272 async_hooks
   2110 buffer
    162 child_process
    136 cluster
    386 console
    472 constants
   1428 crypto
    278 dgram
    106 diagnostics_channel
    726 dns
    356 dns-promises
    120 domain
    346 events
   1792 fs
    318 fs-promises
   1156 http
    822 http2
    158 _http_agent
    112 _http_client
    210 _http_common
    113 _http_incoming
    352 _http_outgoing
    224 https
    344 _http_server
    248 inspector
    236 inspector-promises
    506 module
    970 net
     38 node-sea
    226 node-sqlite
    288 node-test
     46 node-test-reporters
    550 os
    303 path
    303 path-posix
    303 path-win32
    454 perf_hooks
    703 process
     62 punycode
     72 querystring
    460 readline
     74 readline-promises
    168 repl
   1821 stream
     32 stream-consumers
     82 _stream_duplex
     11 _stream_passthrough
     22 stream-promises
    314 _stream_readable
     26 _stream_transform
    672 stream-web
     32 _stream_wrap
    197 _stream_writable
     48 string_decoder
   1040 sys
    116 timers
     34 timers-promises
    478 tls
     32 _tls_common
    422 _tls_wrap
     18 trace_events
    124 tty
    390 url
   1040 util
    312 util-types
    456 v8
    116 vm
     30 wasi
    342 worker_threads
    939 zlib
  29023 total

[Ethan-Arrowood]

This is a great call out. I definitely think from a security standpoint we need to be able to human-review all files in the repo efficiently.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/67637/