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

Update Jest to more recent release #11409

Merged
merged 18 commits into from
Aug 16, 2023
Merged

Update Jest to more recent release #11409

merged 18 commits into from
Aug 16, 2023

Conversation

richvdh
Copy link
Member

@richvdh richvdh commented Aug 14, 2023

I want to use things like jest.advanceTimersByTimeAsync

based on #11413
Fixes element-hq/element-web#24588


This change is marked as an internal change (Task), so will not be included in the changelog.

I want to use things like `jest.advanceTimersByTimeAsync`
@richvdh richvdh requested a review from a team as a code owner August 14, 2023 17:03
@richvdh richvdh requested a review from t3chguy August 14, 2023 17:03
@richvdh richvdh added the T-Task Refactoring, enabling or disabling functionality, other engineering tasks label Aug 14, 2023
@t3chguy t3chguy mentioned this pull request Aug 14, 2023
1 task
"@types/katex": "^0.16.0",
"@types/lodash": "^4.14.168",
"@types/modernizr": "^3.5.3",
"@types/node": "^16",
"@types/node-fetch": "^2.6.2",
"@types/pako": "^2.0.0",
"@types/prettier": "^2.7.0",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like this was previously being pulled in transitively, but is no more.

@richvdh
Copy link
Member Author

richvdh commented Aug 14, 2023

@t3chguy sorry I needed some more stuff. Could you take another look?

@richvdh
Copy link
Member Author

richvdh commented Aug 15, 2023

This is proving somewhat tricky. The problem is that Jest changed the semantics of restoreAllMocks, and lots of our tests rely on the old behaviour.

`mockImplementation` is (now) reset by `jest.resetAllMocks` and
`jest.restoreAllMocks`, which we don't really want here.

Fixes `theme-test`
This doesn't work well in test suites with multiple tests, because the
`mockReturnValue` is reset for subsequent tests.

In any case, having it mocked out automagically is *magical*. Let's make it
opt-in.
@richvdh richvdh changed the base branch from develop to rav/mediaDevices_mock August 15, 2023 20:57

let onFinished: () => void;

beforeEach(() => {
onFinished = jest.fn();
jest.spyOn(WidgetUtils, "canUserModifyWidgets").mockReturnValue(true);

mockClient = {
getUserId: jest.fn().mockReturnValue(userId),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

restoreAllMocks now clears the mock return value, so move this in here to set it up before each test.

Comment on lines 63 to 58
useMockedCalls();
jest.spyOn(HTMLMediaElement.prototype, "play").mockImplementation(async () => {});
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move these in here from the top of the describe block, so they are set up afresh after the restoreAllMocks in afterEach

Comment on lines 55 to 61
// set up mediaDevices mock
Object.defineProperty(navigator, "mediaDevices", {
value: {
enumerateDevices: jest.fn().mockResolvedValue([]),
getUserMedia: jest.fn(),
},
});
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

move this in here from setupManualMocks, again to ensure that they are set up afresh after restoreAllMocks.

.mockReset()
.mockReturnValueOnce([])
.mockReturnValueOnce([pollStart])
.mockReturnValue(undefined as unknown as MatrixEvent[]);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is necessary due to a change in the way mockReset works.

https://jestjs.io/docs/29.3/mock-function-api#mockfnmockreset says:

Resetting a mock created with jest.spyOn() will result in a function with no return value.

This no longer appears to be the case as of 29.6 and we now get the original implementation instead.

So, to preserve current behaviour, have it return undefined once we run out of mockReturnValueOnces.

@@ -203,7 +207,7 @@ describe("<PollHistory />", () => {
.mockReturnValueOnce("test-pagination-token-3");

const { getByText } = getComponent();
await flushPromises();
await act(flushPromises);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shoved some things in act while I was debugging this test.

Comment on lines +76 to +78
beforeEach(() => {
openLinkModalSpy.mockReturnValue(undefined);
});
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we were previously relying on resetAllMocks to make this return undefined. It doesn't any longer, so we need to do it explicitly.

@@ -28,7 +28,7 @@ describe("<SidebarUserSettingsTab />", () => {
beforeEach(() => {
jest.spyOn(PosthogTrackers, "trackInteraction").mockClear();
jest.spyOn(SettingsStore, "getValue").mockRestore();
jest.spyOn(SettingsStore, "setValue").mockReset();
jest.spyOn(SettingsStore, "setValue").mockResolvedValue(undefined);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

another case of mockReset not working the same way on spyOn

Comment on lines -27 to +32
default: jest.fn().mockImplementation(({ url }) => <div>{url}</div>),
default: ({ url }: { url: string }) => <div>{url}</div>,
}));

jest.mock("../../../../src/components/structures/HomePage", () => ({
__esModule: true,
default: jest.fn().mockImplementation(() => <div>home page</div>),
default: () => <div>home page</div>,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

avoid these being reset by restoreAllMocks by using regular functions instead of mocks.

Comment on lines +59 to +60
// TODO: Extract this to a function and have tests that need it opt into it.
const mockMatchMedia = (query: string) => ({
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

avoid this being reset by restoreAllMocks by using a regular function instead of a mock.

Comment on lines +67 to +74
// spy on various toasts' hide and show functions
// easier than mocking
jest.spyOn(SetupEncryptionToast, "showToast").mockReturnValue(undefined);
jest.spyOn(SetupEncryptionToast, "hideToast").mockReturnValue(undefined);
jest.spyOn(BulkUnverifiedSessionsToast, "showToast").mockReturnValue(undefined);
jest.spyOn(BulkUnverifiedSessionsToast, "hideToast").mockReturnValue(undefined);
jest.spyOn(UnverifiedSessionToast, "showToast").mockResolvedValue(undefined);
jest.spyOn(UnverifiedSessionToast, "hideToast").mockReturnValue(undefined);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we were previously relying on resetAllMocks to make these return undefined. It doesn't any longer, so we need to do it explicitly.

Base automatically changed from rav/mediaDevices_mock to develop August 16, 2023 09:19
@richvdh richvdh marked this pull request as ready for review August 16, 2023 09:52
@richvdh richvdh enabled auto-merge August 16, 2023 09:53
@richvdh richvdh added this pull request to the merge queue Aug 16, 2023
Merged via the queue into develop with commit 7a6d81c Aug 16, 2023
80 checks passed
@richvdh richvdh deleted the rav/update_jest branch August 16, 2023 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-Task Refactoring, enabling or disabling functionality, other engineering tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

We cannot update Jest due to weirdness
2 participants