-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
make tests deterministic #559
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so much better!
Do you intend the events to be part of a public API that are present in production apps? Is this (for now?) what's happening instead of the |
Was thinking public API, yeah. Docs will need updating. Not so much an alternative to |
The test that failed in https://github.com/sveltejs/kit/pull/559/checks?check_run_id=2127374998 looks like it was probably a not-waiting-long-enough thing for a click that didn't result in a navigation. We could try to deal with that in some way (via retries?) in this PR or elsewhere or bump up the delay or just ignore it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'm happy with this now. The extra code for the new events is really minimal, so we can do without separate for-test and for-prod builds as well, which might also be undesirable for other reasons anyway.
No description provided.