callstack / react-native-testing-library

🦉 Simple and complete React Native testing utilities that encourage good testing practices.
https://callstack.github.io/react-native-testing-library/
MIT License
3.01k stars 264 forks source link

How to test userEvent.press() in combination with precise timers advancing? #1584

Closed ardentum-c closed 2 months ago

ardentum-c commented 2 months ago

I am trying to test a component that shows some loading state while the passed async onPress callback is executing.

As I understand, the mysterious 130ms is from DEFAULT_MIN_PRESS_DURATION constant, which is used to add delay to press events, mimicking the React Native Pressability behavior.

Here is a complete test case to reproduce:

import "@testing-library/react-native/extend-expect";
import { act, render, screen, userEvent } from "@testing-library/react-native";
import React from "react";
import { Pressable, Text } from "react-native";
import { DEFAULT_MIN_PRESS_DURATION } from "@testing-library/react-native/build/user-event/press/constants";

// Let's imagine there is a component that accepts async onPress callback
// and shows a loader while the callback is in progress.
function TestComponent(props: { onPress: () => Promise<void> }) {
  const [isHandlingOnPress, setHandlingOnPress] = React.useState(false);

  const handlePress = async () => {
    setHandlingOnPress(true);
    await props.onPress();
    setHandlingOnPress(false);
  };

  return (
    <>
      <Pressable onPress={handlePress}>
        <Text>press me</Text>
      </Pressable>

      {isHandlingOnPress && <Text>loading...</Text>}
    </>
  );
}

// some delay function to imitate async action
async function delay(time: number) {
  await new Promise((resolve): void => void setTimeout(resolve, time));
}

describe("TestComponent", () => {
  it("should show loading label during the onPress callback execution", async () => {
    jest.useFakeTimers();

    const ASYNC_CALLBACK_DURATION = 300;
    const user = userEvent.setup();
    render(<TestComponent onPress={() => delay(ASYNC_CALLBACK_DURATION)} />);

    // User presses the button
    await act(async () => user.press(screen.getByText("press me")));

    // Local component's state updates immediately, showing the "loading..." label
    expect(screen.getByText("loading...")).toBeVisible();

    // The "loading..." label should be visible,
    // as the provided async callback is in progress yet
    // But it's not. I have to subtract additional 130ms from ASYNC_CALLBACK_DURATION,
    // as timers were already advanced by 130ms.

    // this doesn't work
    await act(() => jest.advanceTimersByTimeAsync(ASYNC_CALLBACK_DURATION - 1));
    // and here is a workaround option:
    // await act(() => jest.advanceTimersByTimeAsync(ASYNC_CALLBACK_DURATION - DEFAULT_MIN_PRESS_DURATION - 1));

    expect(screen.getByText("loading...")).toBeVisible();

    // Now the loading label should finally be hidden.
    // So the overall time it was visible equals ASYNC_CALLBACK_DURATION.
    await act(() => jest.advanceTimersByTimeAsync(1));
    expect(screen.queryByText("loading...")).toBeNull();
  });
});

So, the question:

Is there a better, more obvious way to test such a behavior and not depend on the internal DEFAULT_MIN_PRESS_DURATION constant? Is this the expected and correct behavior of user.press()?

Initially I was planning to report this as a bug, but the more I'm thinking of it the more I am not sure.

Versions

@testing-library/react-native: ^12.4.4 => 12.4.5
react: 18.2.0 => 18.2.0
react-native: 0.73.6 => 0.73.6
react-test-renderer: 18.2.0 => 18.2.0
mdjastrzebski commented 2 months ago

@pierrezimmermannbam could you take a look, as you know press() the best

pierrezimmermannbam commented 2 months ago

Hmm yeah this is not an easy one, I'm not too sure how I would go about this. The reason we wait for 130ms is because onPressOut will never be called before that time and we wanted onPressOut to be called because it felt like what users would expect. Also I think it makes sense that the simulation of a press event involves some delay because it accounts for the user performing an action. The DEFAULT_MIN_PRESS_DURATION is implementation detail so you don't want it to leak in your tests for sure. I see a couple of approaches that could work here:

I hope this helps, let me know if anything is unclear or if you have suggestions that would help make this clearer for other users

mdjastrzebski commented 2 months ago

I am not sure that trying to pinpoint timer + renders to a millisecond is a good approach, it's getting into area of testing RN/RNTL internal implementation and is prone to break if RN constant changes, etc.

Instead of this:

// this doesn't work
await act(() => jest.advanceTimersByTimeAsync(ASYNC_CALLBACK_DURATION - 1));
// and here is a workaround option:
// await act(() => jest.advanceTimersByTimeAsync(ASYNC_CALLBACK_DURATION - DEFAULT_MIN_PRESS_DURATION - 1));

expect(screen.getByText("loading...")).toBeVisible();

// Now the loading label should finally be hidden.
// So the overall time it was visible equals ASYNC_CALLBACK_DURATION.
await act(() => jest.advanceTimersByTimeAsync(1));
expect(screen.queryByText("loading...")).toBeNull();

Just do this:

await waitForElementToBeRemoved(() => screen.getByText("loading..."));

https://callstack.github.io/react-native-testing-library/docs/api#waitforelementtoberemoved

This does not rely on knowing min press duration implementation details, yet both conditions: that the loading element was there, and I stopped being there after some time (async call). Which would be correct from the users perspective.

Alternatively you can do:

// Loading just after render
expect(screen.getByText("loading...")).toBeOnTheScreen();

// Wait for something other to appear:
await screen.findByText("Loaded")
expect(screen.queryByText("loading...")).not.toBeOnTheScreen();
ardentum-c commented 2 months ago

Thanks for the answers! Indeed, the example I provided is a bit far-fetched. There could be more realistic scenarios when the issue is relevant. I can think of an example of instagram-like stories component that requires precise slides durations to be tested along with press events.

I guess the fireEvent.press is preferable in such cases.

I learned something new. Thank you again!

mdjastrzebski commented 2 months ago

Closing as resolved.