[ESM] Tests no longer work on windows or WSL

[gadicc]

@gadicc Tests no longer work on windows or WSL. I am trying some hacks and reverting some things to make it work. I think that some parts of this PR will have to be reverted, like the tests. After reverting the .js addition and the changes to the jest config, I still failed to get tests running. node_modules/.bin/jest doesn't work on windows, see jestjs/jest#3750. Next, after fixing that, the tests fail saying that "the test environment has already been torn down", pointing at await import('./lib/fetchDevel.js').

Jest ESM support is currently poor, I think that this PR should be held off until ESM support works properly in jest, or at least until tests work on all platforms.
Also see #167.

Originally posted by @PythonCreator27 in #144 (comment)

[gadicc]

Hey @PythonCreator27, let's address this issue (esm testing on windows) separately and see if we can solve it. We can do the same with any other issues that crop up and then make a call on whether to revert or not.

Tests are all passing on my system and on the CI, so I think Jest's ESM support is working ok, but obviously we want it to work on Windows too.

Two things that came to mind:

• Can I just double check that you ran yarn install again after pulling in the latest devel changes, that would have downloaded the latest "next" releases for jest and ts-jest that we specify in package.json.

• Can you try deleting dist, api, tests-modules (and tests/modules if it exists) directories and see if that makes any difference? I think I recall this error when getting everything set up on overlapping tests of differing types / modules. Maybe an ignore pattern using / when windows needs \. But first let's see if this helps.

And if still not, we'll see what we can figure out.

[gadicc]

Oh wait, I just re-read your message... you said you reverted your jest.config too... that will definitely fail. Let's see if any of the above helps on a clean devel clone (I fixed the .bin/jest issue you mentioned in devel too).

[advaiyalad]

I did run yarn again, and deleting the dist, api, and tests-modules folders seems to have made little to no difference.
A lot of tests are failing since they check for a specific type of error, and the error that it is replaced with is the jest "env has been torn down" error. I think that the ignore paths do not work on windows, as adding "\\tests-modules\\" to the ignore pattern sends the failing test count down from 87 (number varies) to 56 (number varies, and has gone down to 33 and up to 97), at least usually. Deleting tests-modules does the same thing. I think that this is a very fluctuating thing, so I might be wrong.

[advaiyalad]

@gadicc After a little bit more of trying things out, I tried to add jest.useFakeTimers() to setupTests.js as suggested somewhere. I also added jest.setTimeout(50000000), but it seems to make the tests hang on the concurrency or some other yahooFinanceFetch tests. Overall, no progress.

[gadicc]

Hey @PythonCreator27, thanks (as usual!) for all your efforts here.

The problem seems to be with the dynamic import, but only when running a lot of tests. As you point out, the number of failures vary with each run. I've been trying to simplify it down to a minimal reproduction to submit as an upstream issue, but no luck yet.

In any event, just FYI, you can get the tests running by replacing the existing fetchDevel() function in env-node.js with something like:

import fetchDevel_ from "./lib/fetchDevel.js";
function fetchDevel() {
  return fetchDevel_;
}

With that, everything passes except for just this 1 failure:

● yahooFinanceFetch › concurrency › waits if exceeding concurrency max

which we can look at separately.

I'd like to try figure this out and submit upstream, but if we're impatient, we could pick a different way to conditionally include fetchDevel (probably with an environment variable). Might not be a bad idea to go that route anyway.

Unfortunately I have an overseas trip coming up next week so my time is a bit limited, but let's see how we go.

[gadicc]

Related to Jest issue #10944 and probably caused by Node issue #36351. Easier temporary fix, set # of workers to >= # of test suits, e.g.

# node-yahoo-finance2 has 16 test suites
$ yarn test -w 16

[advaiyalad]

@gadicc Tests running (after I add --experimental-json-modules to package.json)! I am not sure how CI runs, since the current test scripts do not use --experimental-json-modules. Any ideas?

[gadicc]

Ok great news!

The main test script is using ts-jest which is converting from the source typescript (not the dist output) to whatever they use internally, presumably "es modules implemented in commonjs" so it's probably still requiring'ing the json under the hood. I think! Haven't actually checked their code 😅

Even test:esm presumably goes through a similar process... without the typescript but processed through babel still. Presumably 😁