Unflagging PR blockers

[guybedford]

I wanted to create a tracking issue for the unflagging PR which is located at https://github.com/nodejs/ecmascript-modules/tree/unflag-test-sadness.

We initially had 118 failing tests which is down to 66 down to 35 but we still have a way to go to get there.

Here's a summary of the failing tests that are remaining:

• Async Hooks: Async hooks tests are the major cause of failures with 53 18 failures from test/async-hooks and 7 11 failures from test/parallel/test-async-* and 1 from test/parallel/test-heapdump-async-hooks-init-promise.js. The reason for a lot of these seems to be along the lines that introducing promises into the Node.js bootstrap means that we now have promise events emitted from the bootstrap promises, which break previous assumptions. There are two major aspects to this (1) updating fixtures to deal with core promises being emitted, possibly even implementing a core flag on the async hook event itself (2) handling the fact that initialization promises aren't being emitted to async hooks as the bootstrap is happening before user code can listen to the initialization of those promises, but it will still get the completion of those promises resulting in issues where only the completion is seen by hooks but not the initialization.

• Domain: 3 failing tests from test/parallel/test-domain-* that seems to do with process exit handling.

• Process exception: 2 failing tests for test/parallel/test-process-exception-* to do with process exit handling on exceptions.

• Coverage: 1 failing test for test/parallel/test-v8-coverage.js due to the fact that when executing CommonJS through create-dynamic-module.js, we use the wrapper looking like export default commonjs resulting in coverage being reported for the ESM wrapper instead of the CommonjS module itself. One fix might be to change the internal ids. A better fix might be to update create-dynamic-module.js to use synthetic module records which might avoid the coverage reporting from "overwriting" the CJS coverage. Perhaps there is another way to tackle this too.

And that really is everything before we can look at getting the PR going. Help working on this branch would be amazing, as we are currently blocked on just working through these cases.

[guybedford]

PR for synthetic module records implemented at nodejs/node#29846.

I can confirm that this fixes the v8 coverage issue! The remaining failure in the test-v8-coverage.js file is now actually an async hooks bug, so moving that case under async hooks.

[guybedford]

The PR for async hooks internal bootstrap event buffering has been put together at nodejs/node#29848.

[guybedford]

Builtin changes and synthetic module records have been merged into master.

This PR has been rebased to the async hooks PR at nodejs/node#29848.

With the above we are now down to 35 failing tests.

[guybedford]

I finally managed to fix the remaining tests. The unflagging PR is available at nodejs/node#29866.

[ljharb]

That seems premature; we don’t have consensus for unflagging yet in the modules group.

[guybedford]

@ljharb it's necessary to get the PR up early for feedback from other areas of the project. If you view the PR there, it very clearly states that the PR has a number of tasks needed before it can land, most importantly including modules consensus.