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: Timeout not adhered to due to long async callbacks #35

Open
Izhaki opened this issue May 25, 2022 · 0 comments
Open

Bug: Timeout not adhered to due to long async callbacks #35

Izhaki opened this issue May 25, 2022 · 0 comments

Comments

@Izhaki
Copy link

Izhaki commented May 25, 2022

Currently, the library ignores the fact that async callbacks may take time. This means waitForExpect does not timeout when it should.

Context

This library is used by pptr-testing-library, where callbacks need to interface with puppeteer, which can take time.

We are setting the waitForExpect timeout to 2000ms, yet it is the test itself that timeouts after 5000ms (rather than the waitForExpect timeout). We are asserting on non-existence (problematic alright, but we must at the moment), so need waitFor to timeout correctly.

Log

Below the setting given to waitForExpect and a millisecond log on each retry. You'll see that we are past 2000 at retry 17, although 40 are queued.

{ timeout: 2000, interval: 50, maxTries: 40 }
Try 1 ms: 1
Try 2 ms: 116
Try 3 ms: 218
Try 4 ms: 318
Try 5 ms: 416
Try 6 ms: 590
Try 7 ms: 719
Try 8 ms: 849
Try 9 ms: 974
Try 10 ms: 1101
Try 11 ms: 1226
Try 12 ms: 1358
Try 13 ms: 1484
Try 14 ms: 1614
Try 15 ms: 1754
Try 16 ms: 1886
Try 17 ms: 2016
Try 18 ms: 2149
Try 19 ms: 2285
Try 20 ms: 2413
Try 21 ms: 2543
Try 22 ms: 2671
Try 23 ms: 2799
Try 24 ms: 2932
Try 25 ms: 3062
Try 26 ms: 3197
Try 27 ms: 3327
Try 28 ms: 3460
Try 29 ms: 3589
Try 30 ms: 3712
Try 31 ms: 3839
Try 32 ms: 3971
Try 33 ms: 4102
Try 34 ms: 4230
Try 35 ms: 4360
Try 36 ms: 4485
Try 37 ms: 4615
Try 38 ms: 4738
Try 39 ms: 4863
Try 40 ms: 4985
    ✖ failed
      Error: function timed out, ensure the promise resolves within 5000 milliseconds
          at Timeout._onTimeout (/Users/roey.izhaki/Development/frontend/bdd/node_modules/@cucumber/cucumber/src/user_code_runner.ts:80:18)
          at listOnTimeout (internal/timers.js:557:17)
          at processTimers (internal/timers.js:500:7)
Try 41 ms: 5121

Propose solution

The library should assert on time elapsed, rather than retries. We have modified the code to do just that here. Note, however:

  • Removed Jest fake timers support.
  • Adherence to the testing-library API:
    • Change to the options API.
    • The waitFor function returns a value.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant