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

Avoid using deprecated method waitForTimeout #3948

Merged
merged 1 commit into from
Oct 29, 2024
Merged

Avoid using deprecated method waitForTimeout #3948

merged 1 commit into from
Oct 29, 2024

Conversation

jcoyne
Copy link
Collaborator

@jcoyne jcoyne commented Oct 29, 2024

Fixes #3947

@jcoyne jcoyne force-pushed the without-timeout branch 3 times, most recently from ab886e5 to 518a975 Compare October 29, 2024 03:58
@jcoyne jcoyne marked this pull request as ready for review October 29, 2024 04:06
@marlo-longley
Copy link
Member

marlo-longley commented Oct 29, 2024

Thanks @jcoyne -- locally I got 4 failing tests with the suggestion to add testname-fn-timeout instead. For example:

● Window Sidebars › renders canvas navigation and updates canvas after clicking a navigation item

    thrown: "Exceeded timeout of 10000 ms for a hook.
    Add a timeout value to this test to increase the timeout, if this is a long-running test. See https://jestjs.io/docs/api#testname-fn-timeout."

      2 |
      3 | describe('Window Sidebars', () => {
    > 4 |   beforeAll(async () => {
        |   ^
      5 |     await page.goto('http://127.0.0.1:4488/__tests__/integration/mirador/blank.html');
      6 |
      7 |     await expect(page).toClick('#addBtn');

      at beforeAll (__tests__/integration/mirador/window_sidebar.test.js:4:3)
      at Object.describe (__tests__/integration/mirador/window_sidebar.test.js:3:1)

Is there a reason to remove the wait time rather than replace it with the newer methods/syntax?

@jcoyne
Copy link
Collaborator Author

jcoyne commented Oct 29, 2024

@marlo-longley I don't understand what you mean by the newer syntax. The method waitForTimeout is removed puppeteer/puppeteer#11780

@cbeer
Copy link
Collaborator

cbeer commented Oct 29, 2024

I'm not seeing any local failures from this change (although there's something funny about one of the plugin tests, unrelated to this.). I don't think any of the waitForTimeout calls are anything but trying to address flappy tests, but maybe the ecosystem has improved...

@cbeer cbeer merged commit 3b37950 into master Oct 29, 2024
7 checks passed
@cbeer cbeer deleted the without-timeout branch October 29, 2024 16:36
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

Successfully merging this pull request may close these issues.

waitForTimeout was removed from puppeteer
3 participants