Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: Upgrading from 29.6.2 to 29.6.4 breaks the tests #14498

Closed
HarelM opened this issue Sep 7, 2023 · 24 comments
Closed

[Bug]: Upgrading from 29.6.2 to 29.6.4 breaks the tests #14498

HarelM opened this issue Sep 7, 2023 · 24 comments

Comments

@HarelM
Copy link

HarelM commented Sep 7, 2023

Version

29.6.4

Steps to reproduce

Clone the following repo:
https://github.com/maplibre/maplibre-gl-js
run npm i
run npx jest ./src/ui/map.test.ts
Unit test fail

Expected behavior

The tests should not fail as this wasn't a major version release and there should not be breaking changes.

Actual behavior

The tests started failing, this can also be observed in our PR which uses dependabot: maplibre/maplibre-gl-js#3042

Additional context

Not sure, I tired to debug the tests and understand why they started failing, but I'm not sure what's the root cause.
Sorry that I can't do a minimal reproduction, but I wouldn't know where to start...

Environment

Reproduces on mac, windows, linux.
@SimenB
Copy link
Member

SimenB commented Sep 7, 2023

We'll need a reduced reproduction in order to debug this I'm afraid - that project is huge.

@HarelM
Copy link
Author

HarelM commented Sep 7, 2023

I don't know where to start unfortunately in order to minimize it...
I can point you to a single test that started failing if that helps...
I can easily reproduce by simply installing the latest version.
I can maybe try with npm override to narrow down the relevant dependency if that helps...

@leoShabalkin
Copy link

leoShabalkin commented Sep 8, 2023

Hello, we also encountered this problem on two of our projects. Out of 1500 tests, one describe fell, where call one static method.

it('name', () => {
    expect(ClassA.someMethod()).toEqual(5);
)

When I added logs, I can understand from them that the function someMethod was not executed. someMethod is a public static method. We have tests for other static method, they work out. The function itself, I facilitated, and made

static someMethod() {
    return 5;
}

@SimenB
Copy link
Member

SimenB commented Sep 8, 2023

Without a minimal reproduction I won't be able to debug this.

Most accurate way to pin down the regression would probably be to run Jest from this repository in your project while using git bisect between the two releases. Might be a good idea to check if 29.6.3 is affected as well (can possible use npm's before option to force it https://docs.npmjs.com/cli/v8/using-npm/config#before)

@mrazauskas
Copy link
Contributor

Mysteriously failing tests after Jest upgrade makes me think of #14133 and #14375.

In the CI of maplibre-gl-js tests are timing out. The issues mentioned above were introduced when support for symbol key support was added to the toMatchObject() matcher. See #14375 (comment)

The problem occurs in the jsdom environment. Jest freezes, because checks for symbol keys introduce infinite loops. jsdom is using symbol keys for information hiding. It is adding a lot of props to DOM APIs through symbol keys and many of them are circular references. This is not handle well by Jest.

Here I talk about older releases. Looking through the change log of Jest 29.6.3, #14414 looks similar to what I described above. Can it be that expect.objectContaining() is used by Puppeteer and is causing problems?

@fizz3rworkday
Copy link

Also seeing test failures when upgrading from 29.6.2 -> 29.6.4.

Not sure what is causing the issue.

@rousah
Copy link

rousah commented Sep 13, 2023

Same here!

@SimenB
Copy link
Member

SimenB commented Sep 13, 2023

If anybody has a minimal reproduction, I'd love to see one 🙂

@Donald47
Copy link

Donald47 commented Sep 22, 2023

Unsure if this'll help anyone but I had an issue updating from 29.6.2 to 29.7.0 tracked it back to 29.6.3 and this change: #14366

The error was

TypeError: (0 , _jestUtil.invariant) is not a function

Forcing the resolution of jest-util to 29.7.0 and updating babel-jest to the same solved it.

@SimenB
Copy link
Member

SimenB commented Sep 22, 2023

Interesting... Do you have a full trace? Sounds like a missing dependency

@Donald47
Copy link

  ● Test suite failed to run

    TypeError: (0 , _jestUtil.invariant) is not a function

      at eventHandler (../node_modules/jest-circus/build/eventHandler.js:248:31)

Seemingly something in jest-circus, sorry I should have included that in my original message.

@HarelM
Copy link
Author

HarelM commented Sep 24, 2023

I did not notice any issue when upgrading jest and jest-environment-jsdom with the jest-util dependency - all are updated to 29.7.0.
But upgrading to latest version (29.7.0) still doesn't solve my test failures, I have no clue how to start looking into creating a minimal reproduction unfortunately, although I would like to do that very much, as I don't know how these tests are different from other tests... :-(

This is one of the error I get which is not a timeout of test error as far as I understand:

console.error
      Error: Uncaught [TypeError: Cannot read properties of undefined (reading 'get')]
          at reportException (/Users/harelmazor/dev/maplibre-gl-js/node_modules/jest-environment-jsdom/node_modules/jsdom/lib/jsdom/living/helpers/runtime-script-errors.js:66:24)
          at runAnimationFrameCallbacks (/Users/harelmazor/dev/maplibre-gl-js/node_modules/jest-environment-jsdom/node_modules/jsdom/lib/jsdom/browser/Window.js:605:13)
          at Timeout._onTimeout (/Users/harelmazor/dev/maplibre-gl-js/node_modules/jest-environment-jsdom/node_modules/jsdom/lib/jsdom/browser/Window.js:581:11)
          at listOnTimeout (node:internal/timers:569:17)
          at processTimers (node:internal/timers:512:7) {
        detail: TypeError: Cannot read properties of undefined (reading 'get')
            at new LayerPlacement (/Users/harelmazor/dev/maplibre-gl-js/src/style/pauseable_placement.ts:22:51)
            at PauseablePlacement.continuePlacement (/Users/harelmazor/dev/maplibre-gl-js/src/style/pauseable_placement.ts:112:45)
            at Style._updatePlacement (/Users/harelmazor/dev/maplibre-gl-js/src/style/style.ts:1536:37)
            at Map._render (/Users/harelmazor/dev/maplibre-gl-js/src/ui/map.ts:3178:57)
            at /Users/harelmazor/dev/maplibre-gl-js/src/ui/map.ts:3314:22
            at invokeTheCallbackFunction (/Users/harelmazor/dev/maplibre-gl-js/node_modules/jest-environment-jsdom/node_modules/jsdom/lib/jsdom/living/generated/Function.js:19:26)
            at runAnimationFrameCallbacks (/Users/harelmazor/dev/maplibre-gl-js/node_modules/jest-environment-jsdom/node_modules/jsdom/lib/jsdom/browser/Window.js:603:13)
            at Timeout._onTimeout (/Users/harelmazor/dev/maplibre-gl-js/node_modules/jest-environment-jsdom/node_modules/jsdom/lib/jsdom/browser/Window.js:581:11)
            at listOnTimeout (node:internal/timers:569:17)
            at processTimers (node:internal/timers:512:7),
        type: 'unhandled exception'
      }

@lucianojsjr
Copy link

Hi guys, I got the same problem after update from 29.6.1 to 29.6.3.

In my case it was related to the resetAllMocks. I have the following script defined on setupFilesAfterEnv which works well in version 29.6.1

beforeEach(() => { jest.resetAllMocks(); });

After updating to 29.6.3 the tests stopped working. First, I took a look at my code and discovered that the tests were broken because the mock functions were not being reset, returning the same value as the previously run tests.
So I took a look at the release notes and found that some changes related to mocks were reverted in the version related to this PR #14429

In trial and error I changed the script to the following code and it worked again.

beforeEach(() => { jest.restoreAllMocks(); });

@SimenB I'm studying jest under the table because I can't explain exactly why this happened, but it solved my problem. Maybe this could be a tip on how to reproduce this problem.

@tweaktastic
Copy link

tweaktastic commented Oct 4, 2023

Hello everyone, I was running into the same issue after upgrading jest to 29.7.0. I spent some time going through the docs around what restoreAllMocks, clearAllMocks and resetAllMocks was. Earlier I was using resetAllMocks but it turns out that if we want to revert the mocked method to its original implementation, we should be using restoreAllMocks.

Eg:
testUtils.ts

export function testingMethod(): boolean {
  return true;
}

testUtils.test.ts

import * as utils from './testUtils';

describe('Reproduce jest mock bug', () => {
  afterEach(() => {
    jest.restoreAllMocks(); //  rests the mock to revert to original implementation, tests pass
//    jest.resetAllMocks();  // resets the mocked methods to return undefined. tests fail
  });

  test('mocks the utils method and returns false', () => {
    const utilSpy = jest.spyOn(utils, 'testingMethod');
    utilSpy.mockImplementation(() => false);

    expect(utils.testingMethod()).toBe(false);
  });

  test('should return true as the method is no longer mocked', () => {
    expect(utils.testingMethod()).toBe(true);
  });
});

I hope this helps.
https://jestjs.io/docs/mock-function-api#mockfnmockreset
https://jestjs.io/docs/mock-function-api#mockfnmockrestore

@rousah
Copy link

rousah commented Oct 5, 2023

Changing resetAllMocks to restoreAllMocks fixed it for me too! Thanks for the tips.

@HarelM
Copy link
Author

HarelM commented Oct 6, 2023

Unfortunately, maplibre-gl-js test code doesn't have resetAllMocks calls so this doesn't help in my case :-(
Thanks for the tips though.

@lucasvaladares-hotmart
Copy link

  ● Test suite failed to run

    TypeError: (0 , _jestUtil.invariant) is not a function

      at eventHandler (../node_modules/jest-circus/build/eventHandler.js:248:31)

Seemingly something in jest-circus, sorry I should have included that in my original message.

I have the same issue

@dani1793
Copy link

I encountered some errors while attempting to upgrade the package. Initially, I tried switching from resetAllMocks to resetAllMocks, which helped resolve the issue partially.

I shuffled the tests around and made adjustments to the spy functions. Ran into some 'undefined' cases which I guess could be related to this

@SimenB
Copy link
Member

SimenB commented Oct 29, 2023

Without reproductions, it's hard to really do anything here, so I'll close this. If anybody is able to provide reproductions, please open a new issue.

@SimenB SimenB closed this as completed Oct 29, 2023
@HarelM
Copy link
Author

HarelM commented Oct 29, 2023

I have provided a way to reproduce this issue.
Yes, the library is large and so are the tests, but for someone that is familiar with the jest code it can be useful I hoped - using an old version, adding logs and being able to debug and understand what's the difference between the versions and what might have caused this regression.
Closing this issue means I'll be stuck with version 29.6.2 and the only way to upgrade would be to disable some tests - that's not a great solution for me.
I'll be happy to assist in any way to solve this, but I'm not familiar with the jest codebase... :-(

@SimenB
Copy link
Member

SimenB commented Oct 29, 2023

If you can fork that and chip away as much as possible while the issue still reproduces, that'd might make it minimal?

@HarelM
Copy link
Author

HarelM commented Oct 31, 2023

I'll see what I can do.
I've having a real hard time with tests that have done - the error message is cryptic in a lot of places, so I'm looking into migrating most of the tests to use async code, maybe this will be solved this way.
I'll update once I have something.

@HarelM
Copy link
Author

HarelM commented Nov 13, 2023

Seems like my issue was the same as everyone else here with the spy.mockReset() vs spy.mockRestore().
The reason why my test failed is that the mockReset in one test cause the other test to fail, which uses done and causes a timeout.
The more done I remove the better I see the error message of the test etc, so it ended up improving the code.
Thanks for everyone in this thread for the helpful suggestions!! :-)

Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests