Zora + pta doesn't work with top level dynamic imports

[wokalski]

Right now PTA reporter depends on all tests to be registered before the register function executes. It causes a race condition. We are using a framework which ends up importing tests like this:

import("test.mjs").catch(e => {
  if (e.code !== "ERR_MODULE_NOT_FOUND") {
    return Promise.reject(e);
  }
});

In that case, pta only works if synchronous module resolution for other tests (which are not using that framework) takes long enough for the promise above to be resolved. It's a horrible race condition to have. In my opinion, the reporting step should only be concluded on beforeExit. If that was the case, dynamic imports would work as intended in this case.

https://nodejs.org/api/process.html#event-beforeexit

[lorenzofox3]

That is a good idea. The only caveat I see is that there would not be any feedback at all before all the tests have finished. We could add some loading placeholder though.

In your case, as you load the test files by yourself (through your framework) and as pta does not do much more, I believe the easier would be to build your "own test-runner". That's pretty easy: pta itself is only few lines of code.

You could get something like:

import {
    createDiffReporter,
} from 'zora-reporters';

(async () => {
    // loading zora to hold the singleton:
    const { hold, report } = await import('path/to/zora/file/used/in/your/suites'); // 'zora/es' would likely be enough in you case
    hold();
    
    // Here using your framework
    await loadFiles()
    
    // now reporting
    await report({
        reporter: createDiffReporter(),
    });
})();

Let me know how it goes

[wokalski]

I think this is not the optimal solution to this. Adds another tight coupling where it's not really needed.

It's been some time since I used those features but if you used an async iterator for test reporting instead of simply pushing to an array. You would register tests dynamically through the execution. It would be fully compatible with all frameworks and all approaches. There are basically two conditions to resolve async next():

1. A test is discovered
2. beforeExit is called.

https://github.com/repeaterjs/repeater could be used to create the iterator.

[lorenzofox3]

🤔 I am not sure to understand, would mind providing a PR ? Zora uses already async iterables.

Probably easier: would you mind sharing a reproduction of your setup which causes the problem ?

[wokalski]

I created a mock PR

[lorenzofox3]

Thanks, I have left a comment (I am not convinced this would solve the problem). It would be great if you could share an example of a test suite which reproduces the problem so I can eventually provide a solution