Tests fails on Windows

[asvishnyakov]

trafficstars

Issue Description

I'm just trying to follow contribution guidelines.

I already fixed the error on build, but when I'm trying to run npm run test, I receive the following errors:

Log.pdf

Do you have suggestions what may cause this?

Link to Reproduction

N/A

Reproduction Steps

npm install npm run build npm run test

@apollo/client version

3.9.2

[phryneas]

That pdf doesn't show anything, could you share a text file please?

[asvishnyakov]

@phryneas Yeah sure, here it is: Log.txt It's just "a little bit" long, so I tried to preserve colors 😁

[alessbell]

Hi @asvishnyakov 👋 I see the failing tests in your Log.txt - out of curiosity, if you run one of the test files from the list of failing tests by itself (e.g. npm run test src/react/hooks/__tests__/useSuspenseQuery.test.tsx), does it fail or pass?

When running all 3000+ tests locally they can be flaky, so I can't say whether this is Windows-specific yet :) @phryneas has offered to try on his Windows machine, but I'm without one at the moment.

If none of the tests fail when run on their own, I can add a disclaimer to call local test flakiness out in our CONTRIBUTING.md and will otherwise close this out. Thanks!

[phryneas]

Okay, I'm quite flabbergasted here.

Some notes:

Running the tests in WSL without --runInBand is very flaky and causes a lot of errors.

Now, if you have a super new version of node (this is not limited to Windows!), you also have a new version of npm, and that doesn't seem to pass argument along anymore.

So npm run test --runInBand won't work anymore.

Instead, you have to call

node --expose-gc ./node_modules/jest/bin/jest.js --config ./config/jest.config.js --runInBand

With that, the tests pass just fine in WSL2.

But then I tested the tests on plain Windows, and it's the weirdest thing: React is rendering differently in that environment.

Like, even executing individual tests, I have a completely different rendering behaviour.

So this is an example of a test that would need these changes:

https://github.com/apollographql/apollo-client/commit/ecce16945fa3783d10894a3b0871cad02851b868

I honestly don't know what is going on there. @jerelmiller you have worked a lot with those tests - you have any idea here?

=> I think so far we can recommend WSL with --runInBand (but with the full command, not just npm run test) as a solution for Windows, but not running the test suite natively with a Windows build of node.

[jerelmiller]

@phryneas I'm not really sure to be honest, though some of those tests that failed are tests that have flaked in the past in CI. Would be nice to update those useSuspenseQuery tests to use the new profiler, but I have no idea if this would change things in Windows or not. Unfortunately I don't have a windows machine around so I'm pretty useless helping here 😞

[asvishnyakov]

@alessbell Yep, individually they fails too.

@phryneas So, what's the conclusion if I want to do some contributions? Use Linux or run only related tests in WSL on Windows? :)

[phryneas]

@asvishnyakov To be honest, I'd highly recommend you to use WSL2 (or any other unix of your choice) for contributing to AC at this point.

I understand that Windows is a perfectly valid lifestyle choice (as I personally would much prefer using Linux than the Macbooks we have to use here at Apollo 😉), but none of us core developers uses Windows for AC development, and it's probably a herculean effort to get the tests play nicely on there.

On the positive side, Visual Studio Code integrates very nicely with WSL2 nowadays, and developing JS is even faster and more consistent there than on a normal NTFS file system.

[asvishnyakov]

Well, I tried to look up at some render results and it interesting. Just in case it somehow someone helps: https://github.com/apollographql/apollo-client/blob/65ab695470741e8dcaef1ebd7742c3c397526354/src/react/hooks/tests/useSuspenseQuery.test.tsx#L4339 with act() - failed:

 <div><div style=""><button>Refetch</button><div data-testid="todo">Clean room</div></div></div>

without act()`:

    Warning: An update to Todo inside a test was not wrapped in act(...).

    When testing, code that causes React state updates should be wrapped into act(...):
    
    act(() => {
      /* fire events that update state */
    });
    /* assert on the output */

but success:

<div><div style="display: none;"><button>Refetch</button><div data-testid="todo">Clean room</div></div><p>Loading</p></div>

[phryneas]

In an unrelated take of events, I just tried to swap jsdom to happy-dom, and I see very different timing in some of the tests. So my guess right now is that the "simulated browser" of jsdom is behaving differently and mocking some of the timers differently.

I know I can't give you any satisfying answer right now, but at least though I'd share that finding.

[asvishnyakov]

@phryneas I'll try to run tests in WSL and if it will work, I think it should be enough just to mention about that in your contribution guidelines. Thank you for your help!

[alessbell]

Hi @asvishnyakov - did you have better luck with WSL? If so I can update CONTRIBUTING.md, thanks!

[asvishnyakov]

@alessbell Hi! No, I moved to Linux (for another reason) so this issue is no longer actual for me 😀

[alessbell]

Thanks for letting us know! In that case I'm going to go ahead and close this for now - I've added the discussion label so it doesn't get locked and we can reopen it if anyone runs into it in the future. :)

[github-actions[bot]]

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo Client usage and allow us to serve you better.