Skip to content

test: improve test stability#12159

Merged
jcfranco merged 52 commits intodevfrom
jcfranco/disable-waitForChanges-timeout
Jun 3, 2025
Merged

test: improve test stability#12159
jcfranco merged 52 commits intodevfrom
jcfranco/disable-waitForChanges-timeout

Conversation

@jcfranco
Copy link
Copy Markdown
Member

@jcfranco jcfranco commented May 15, 2025

Related Issue: #10433

Summary

This PR improves tests by:

  • replacing event awaiting pattern (see note on ESLint rule below)
  • remove redundant tests (e.g., covered by themed test util or screenshot tests)
  • remove redundant waitForChanges calls (newE2EPage, setContent and E2EElement interaction APIs call this method internally)
  • correct test nesting (e.g., mouse interaction group nested in keyboard interaction instead of being a sibling)
  • skip animations for some event-related tests
  • add missing awaits

Notes

  • adds waitForAnimationFrame for Puppeteer/E2E contexts

  • adds custom ESLint rule to avoid using the following pattern:

    const pendingEvent = e2ePageOrEl.waitForEvent("calciteFoo");
    // do stuff that triggers event
    await pendingEvent;

    which can lead to timing issues once the waitForChanges delay is removed, in favor of:

    const spy = await e2ePageOrEl.spyOnEvent("calciteFoo");
    // do stuff that triggers event
    await spy.next();

@github-actions github-actions Bot added the chore Issues with changes that don't modify src or test files. label May 15, 2025
@jcfranco jcfranco requested a review from benelan as a code owner May 28, 2025 23:42
@jcfranco jcfranco changed the title chore: drop waitForChanges delay test: update tests for stability Jun 3, 2025
@jcfranco jcfranco changed the title test: update tests for stability test: improve test stability Jun 3, 2025
@jcfranco jcfranco added the skip visual snapshots Pull requests that do not need visual regression testing. label Jun 3, 2025
Copy link
Copy Markdown
Member

@driskull driskull left a comment

Choose a reason for hiding this comment

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

LGTM!

Copy link
Copy Markdown
Contributor

@aPreciado88 aPreciado88 left a comment

Choose a reason for hiding this comment

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

This LGTM 🚀

@jcfranco jcfranco merged commit 34856ad into dev Jun 3, 2025
21 checks passed
@jcfranco jcfranco deleted the jcfranco/disable-waitForChanges-timeout branch June 3, 2025 20:21
@github-actions github-actions Bot added this to the 2025-06-24 - Jun Milestone milestone Jun 3, 2025
benelan pushed a commit that referenced this pull request Sep 16, 2025
**Related Issue:** #10433

## Summary

This PR improves tests by:

* replacing event awaiting pattern (see note on ESLint rule below)
* remove redundant tests (e.g., covered by `themed` test util or
screenshot tests)
* remove redundant `waitForChanges` calls (`newE2EPage`, `setContent`
and `E2EElement` interaction APIs call this method internally)
* correct test nesting (e.g., mouse interaction group nested in keyboard
interaction instead of being a sibling)
* skip animations for some event-related tests
* add missing `await`s

### Notes

* adds `waitForAnimationFrame` for Puppeteer/E2E contexts
* adds custom ESLint rule to avoid using the following pattern:

    ```ts
    const pendingEvent = e2ePageOrEl.waitForEvent("calciteFoo");
    // do stuff that triggers event
    await pendingEvent;
    ```

which can lead to timing issues once the waitForChanges delay is
removed, in favor of:

    ```ts
    const spy = await e2ePageOrEl.spyOnEvent("calciteFoo");
    // do stuff that triggers event
    await spy.next();
    ```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Issues with changes that don't modify src or test files. skip visual snapshots Pull requests that do not need visual regression testing.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants