Migrate tests to Jest

[scheibo]

• Migrate tests to Jest – Jest (https://jestjs.io/) supports parallel test-running, which would be nice to have over Mocha. Most of its API surface is similar, so this shouldn't take too long.

Originally posted by @Zarel in #2444

Did some investigation here. Have some failing tests still, but I figured I'd dump what I've figured out so far:

1. Jest has a pretty similar API, but since it doesn't use this we could switch function () { ... } to () => { ... } which is the preferred style. I didn't bother with this.

2. A lot of our tests do weird things with timeouts, possibly due to them getting hit by Dex loading? Why do half of these fiddle with timeouts? I can see why team generator might, but basic tests for abilities or moves? Smells like a bug to me.

3. Jest's test runner is much noisier by default, and we'd probably need to write our own/depend on a new one. The biggest difference is that its logging even on success (which I'm OK with), but this means we get tons of:

   PASS  test/simulator/abilities/roughskin.js
   ● Console
   
     console.log server/monitor.js:135
       NEW GLOBAL: global
     console.log server/monitor.js:135
       NEW CHATROOM: lobby
     console.log server/monitor.js:135
       NEW CHATROOM staff
   

   for each test. Maybe we could stub out Monitor.notice etc in the beforeAll?

4. Hot take: Jest will be slower. Is what my computer looks like running Jest, and I'm pretty sure most PS! developers don't have that kind of hardware sitting under their desk. To make matters worse - even on my box, its slower (13-14s instead of 10-12s)! I'm guess this is possibly due to the verbosity of the logging (writing that much to the console is slow), but I'd bet even on my computer its not going to be much faster than Mocha due to Dex loading. Dex loading is super slow, and if we're naively running tests in parallel processes the way Jest does, we're paying the price multiple times. The optimal way of running PS is to spin up a number of processes roughly equal to the number of cores (maybe NUM_CORES-1, to allow for the GC thread), and then run Dex loading once in each one, and then run multiple battles in a loop (incidently, the PS server does this :P)

5. Jest doesn't like Dex loading, it seems to cause failures when a test is skipped:

ReferenceError: You are trying to `import` a file after the Jest environment has been torn down.

      at ModdedDex.includeFormats (.sim-dist/dex.js:1643:17)
      at ModdedDex.get formats [as formats] (.sim-dist/dex.js:246:10)
      at ModdedDex.loadData (.sim-dist/dex.js:1584:80)
      at ModdedDex.includeData (.sim-dist/dex.js:1550:10)
      at Object.<anonymous>.process.nextTick (server/chat-plugins/info.js:2549:7)
.sim-dist/dex.js:1651
      throw new TypeError(`Exported property 'Formats' from "./config/formats.js" must be an array`);
      ^

TypeError: Exported property 'Formats' from "./config/formats.js" must be an array
    at ModdedDex.includeFormats (.sim-dist/dex.js:1651:13)
    at ModdedDex.get formats [as formats] (.sim-dist/dex.js:246:10)
    at ModdedDex.loadData (.sim-dist/dex.js:1584:80)
    at ModdedDex.includeData (.sim-dist/dex.js:1550:10)
    at Object.<anonymous>.process.nextTick (server/chat-plugins/info.js:2549:7)
    at processTicksAndRejections (internal/process/task_queues.js:79:9)

All in all, useful exercise, but not super sure about how much we want to invest in Jest?

[KamilaBorowska]

If I had to guess Jest is slower because it runs multiple processes, which prevents the JIT from optimizing too much, and loading takes a while. 48 cores is kinda a lot. Those aren't threads, they are processes. Ironically, having more cores can slow things down.

[scheibo]

That is because Chrome’s build system is smart enough to spawn more processes if you have more cores, which means that there are more terminating processes fighting over the global lock.

'smart' is used generously here, because clearly its resulting in worse results. Having more cores can definitely slow things down if they're not used properly though.

If I had to guess Jest is slower because it runs multiple processes, which prevents the JIT from optimizing too much, and loading takes a while

Yup. I'm curious though, when I get home I'll try it on my laptop which I think only has maybe 4 cores. I think its still going to be slower because the processes Jest spawns wont reuse Dex loading or benefit from caching etc.

That being said: jestjs/jest#6694

[Slayer95]

In my machine, with a barebones initialization, and running only the simulator/ tests, mocha uses ~23 seconds in my machine (2 cores [1]).

For a similar setup, jest uses ~140 seconds. This duration doesn't change noticeably with --maxWorkers=1 nor --maxWorkers=2, but it does increase a lot with maxWorkers=4 (despite Jest's estimates that each of those would result in halving the run time).

On the other hand, switching to a very simple runner which condenses all the simulator test suites into 4 suites reduces the total time to ~40 seconds. If it's condensed into 2 suites, the total time is ~30 seconds.... It seems to be just slightly faster than the case of 1 suite (~33 seconds), but I don't think that's a conclusive result....

Overall, it's worrying that this results in a performance degradation of anywhere from 50% to 500%, even for the case where we only have 1 process !

---

[1] Initially I thought I had 4 cores, which is why I also tested for the cases of 4 workers / 4 suites.....

EDIT: Fixed links
EDIT2: Also ran --maxWorkers=1.

[Slayer95]

Set env: {jest: true} in test/.eslintrc instead

[scheibo]

TL;DR: "Jest seems slow". Do we want to even bother to try and make it work? I'm happy to close this (once @Zarel weighs in) and remove the idea from #2444.

[Slayer95]

Jest has a pretty similar API, but since it doesn't use this we could switch function () { ... } to () => { ... } which is the preferred style. I didn't bother with this.

ESLint's rule prefer-arrow-callback has an autofixer. Just add it to test/.eslintrc and run it with --fix.

A lot of our tests do weird things with timeouts, possibly due to them getting hit by Dex loading? Why do half of these fiddle with timeouts? I can see why team generator might, but basic tests for abilities or moves? Smells like a bug to me.

Those sometimes to often timed out in my machine. The only explanation I could fathom was that they ran heavy events, but taking several seconds was always odd. That said, since the sim is at least 50% more performant nowadays, those custom timeouts might not be needed anymore.

we get tons of:
PASS test/simulator/abilities/roughskin.js
● Console
console.log server/monitor.js:135
NEW GLOBAL: global
console.log server/monitor.js:135
NEW CHATROOM: lobby
console.log server/monitor.js:135
NEW CHATROOM staff
for each test. Maybe we could stub out Monitor.notice etc in the beforeAll?

We only need to tweak Config.loglevel. Furthermore, we don't actually need to start up the whole server for each test. It's possible to start just the sim itself nowadays. Of course, application tests still need the server, so that's the scope of the fix.

Jest doesn't like Dex loading, it seems to cause failures when a test is skipped:

It doesn't like us loading Dex in process.nextTick() from chat plugins. We can probably wait for a setImmediate() to resolve in the beforeAll() for application/plugins tests.

[scheibo]

ESLint's rule prefer-arrow-callback has an autofixer. Just add it to test/.eslintrc and run it with --fix.

Yeah, jest-codemods does this as well, but I didn't want to cause so much churn.

Those sometimes to often timed out in my machine. The only explanation I could fathom was that they ran heavy events, but taking several seconds was always odd. That said, since the sim is at least 50% more performant nowadays, those custom timeouts might not be needed anymore.

I think this is worth followup. I don't know that I'm the best to try it though, because I think the tests run the fastest on my machines. Maybe @KrisXV can help us here?

We only need to tweak Config.loglevel.

Ahh, TIL.

Furthermore, we don't actually need to start up the whole server for each test. It's possible to start just the sim itself nowadays. Of course, application tests still need the server, so that's the scope of the fix.

I was going to say we should followup on that (not starting the server), but I guess it currently doesnt matter if we're sticking with mocha because the application tests run in the same process as the sim tests anyway.

It doesn't like us loading Dex in process.nextTick() from chat plugins. We can probably wait for a setImmediate() to resolve in the beforeAll() for application/plugins tests.

Good call. Though once again, not necessarily worth changing if we're not going to use Jest.

Thanks @Slayer95 for diving into this! You definitely did a better job than my naive attempt!

[Zarel]

I'd agree with closing and removing the idea. I only ever wanted Jest because it seemed like parallel tests would be a speed boost, but I did give up on using Jest on client after benchmarking. It looks like it doesn't offer anything to be worth it.

[scheibo]

I've been using it for all my personal libraries because I thought it would be better as well, but after now benchmarking on PS im reevaluating. Anyway, will close and remove the suggestion, thanks!