-
Notifications
You must be signed in to change notification settings - Fork 156
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
Remove timer faking from functionWatcher tests #5151
base: main
Are you sure you want to change the base?
Conversation
We detected some changes at packages/*/src and there are no updates in the .changeset. |
Coverage report
Test suite run success2035 tests passing in 909 suites. Report generated by 🧪jest coverage report action from 583ec94 |
Vitest has its own |
fd3bf4a
to
967e9a0
Compare
Differences in type declarationsWe detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:
New type declarationsWe found no new type declarations in this PR Existing type declarationspackages/cli-kit/dist/public/node/testing/ui.d.ts@@ -1 +1 @@
-export { getLastFrameAfterUnmount, render, Stdin, waitForInputsToBeReady, waitForContent, sendInputAndWait, sendInputAndWaitForChange, sendInputAndWaitForContent, } from '../../../private/node/testing/ui.js';
\ No newline at end of file
+export { getLastFrameAfterUnmount, render, Stdin, waitFor, waitForInputsToBeReady, waitForContent, sendInputAndWait, sendInputAndWaitForChange, sendInputAndWaitForContent, } from '../../../private/node/testing/ui.js';
\ No newline at end of file
|
c348720
to
583ec94
Compare
This PR seems inactive. If it's still relevant, please add a comment saying so. Otherwise, take no action. |
Still relevant, waiting on https://github.com/Shopify/develop-app-inner-loop/issues/2552 |
WHY are these changes introduced?
Part of https://github.com/Shopify/shopify-functions/issues/481
This test was reported as flakey, so this attempts to fix it by removing the unnecessary timer faking.
WHAT is this pull request doing?
Swaps out our timer faking with a more straightforward
waitFor
that just checks the last returned result from our hook function until it matches what we expect.I used
waitFor
fromcli-kit
'stesting/ui
, and had to add it to the public export.I figured this was fine, but if it's not I could also copy the code from it into our tests more directly.
I also looked at usePollAppLogsTest where we used a similar approach, but I chose to keep it as is over there since that test has a lot to do with timing, retries, and enqueued timers. It makes sense over there.
How to test your changes?
Post-release steps
N/A
Measuring impact
How do we know this change was effective? Please choose one:
Checklist