search

nodejs / import-in-the-middle

Like `require-in-the-middle`, but for ESM import
https://www.npmjs.com/package/import-in-the-middle
Apache License 2.0
67 stars 24 forks source link

Clean up the test harness a bit #55

Closed bengl closed 8 months ago

bengl commented 8 months ago

Primarily, this gets rid of the test/runtest executable, preventing the need for spawning an additional subprocess for each test. The current approach handles all the things that test/runtest would in a --require and in a loader that switches whether or not to use the IITM loader via the same filename check that was originally done in test/runtest.

One interesting thing that happened while making this change is that we need some way of passing the entrypoint filename to the loader. Since newer versions of Node.js run the loader in a separate thread, process.argv[1] no longer works. The approved alternative is to use inter-thread messages via the register() method, but that's not available on some versions of Node.js that also use loader threads. To get around this, an environment variable is set in the --require that's then propagated to the loader thread. This is a bit hacky, but probably the only thing that will work across the supported version range.

An attempt was made to use the more popular tap test package, rather than imhotap, but there's no version of tap that supports both the entire Node.js version range that IITM supports and also supports ESM. For now, this means sticking with imhotap, but were it not for the incompatibility, tap would be just as suitable here, and is a much more popular test package.

bengl commented 8 months ago

@jsumners-nr

I am concerned this will make debugging more difficult since this introduces a loader on top of the loader being tested.

The wrapper loader used for testing is just a pass-through. It's not chaining loaders or anything like that. It should be completely transparent. What's the concern?

jsumners-nr commented 8 months ago

@jsumners-nr

I am concerned this will make debugging more difficult since this introduces a loader on top of the loader being tested.

The wrapper loader used for testing is just a pass-through. It's not chaining loaders or anything like that. It should be completely transparent. What's the concern?

It's not a blocker. I'm just unclear what a debugging session will be like.

bengl commented 8 months ago

@jsumners-nr You make a good point here that the test README should be updated. I'll do that now.