testing-library / pptr-testing-library

puppeteer + dom-testing-library = 💖
MIT License
287 stars 29 forks source link

waitFor does not return the callback's returned value. #73

Open Izhaki opened 2 years ago

Izhaki commented 2 years ago

The waitFor exported by this library does not seem to be complaint with the testing-library API, where it should return the callback's return value.

In the following code, x will have a value, but y wouldn't:

  const y = await waitFor(async () => {
    const $container = await getContainer();
    const x = await queries.findByRole($container, ...parameters);
    console.log(x);
    return x;
  });
  console.log(y);

We have a case where a user action opens a new tab, so we need to retry getting the page itself, not just the dom query.

Izhaki commented 2 years ago

We have ended up modifying wait-for-expect code to support both return values and timeout adherence. This exclude the handling of Jest fake timers (although can easily be added).

/*
A modification of https://github.com/TheBrainFamily/wait-for-expect/blob/6be6e2ed8e47fd5bc62ab2fc4bd39289c58f2f66/src/index.ts
Which is exported by pptr-testing-library, but has two issues:
- Timeout not adhered to with long promises. https://github.com/TheBrainFamily/wait-for-expect/issues/35
- No return value. https://github.com/testing-library/pptr-testing-library/issues/73
*/

const { now } = Date;

interface Options {
  timeout?: number;
  interval?: number;
}

/**
 * Waits for the expectation to pass and returns a Promise
 *
 * @param  expectation  Function  Expectation that has to complete without throwing
 * @param  timeout  Number  Maximum wait interval, 4500ms by default
 * @param  interval  Number  Wait-between-retries interval, 50ms by default
 * @return  Promise  Promise to return a callback result
 */
export default async function waitFor(
  expectation: () => void | Promise<any>,
  options: Options = {}
): Promise<any> {
  const { timeout = 4500, interval = 50 } = options;
  if (interval < 1) {
    throw new Error("Interval set to a number smaller than 1 ms.");
  }
  const startTime = now();
  return new Promise((resolve, reject) => {
    const rejectOrRerun = (error: Error) => {
      const timeElapsed = now() - startTime;
      if (timeElapsed >= timeout) {
        reject(error);
        return;
      }
      // eslint-disable-next-line no-use-before-define
      setTimeout(runExpectation, interval);
    };
    async function runExpectation() {
      try {
        const result = await expectation();
        resolve(result);
      } catch (error) {
        rejectOrRerun(error as Error);
      }
    }
    setTimeout(runExpectation, 0);
  });
}
patrickhulce commented 2 years ago

Ah, thanks for filing @Izhaki great find! We're happy to update to latest waitFor version if there's a fix available there.

Manvel commented 1 year ago

There seem to be fix for it https://github.com/TheBrainFamily/wait-for-expect/pull/33, but considering the fact that the lib has last been updated 2 years ago, not sure if it's actively being maintained anymore.