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

test: add automatic wait for vite client connected #5437

Closed
wants to merge 5 commits into from

Conversation

dominikg
Copy link
Member

@dominikg dominikg commented Jul 8, 2022

to page navigation functions

This isn't the nicest way to do it and doesn't recognize cases where goBack leads to a load.
But it can stabilize tests that rely on the vite client and previously may have started work before that was ready.

logic taken from vite-plugin-svelte. cc @bluwy

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. All changesets should be patch until SvelteKit 1.0

@changeset-bot
Copy link

changeset-bot bot commented Jul 8, 2022

⚠️ No Changeset found

Latest commit: 9bf8191

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@dominikg
Copy link
Member Author

dominikg commented Jul 8, 2022

unfortunately it looks like that it now hangs the firefox tests. investigating tomorrow.

@Rich-Harris
Copy link
Member

will mark this as draft until it's ready

@Rich-Harris Rich-Harris marked this pull request as draft July 12, 2022 14:00
@dominikg
Copy link
Member Author

firefox may be hanging due to microsoft/playwright#15550

@benmccann
Copy link
Member

firefox may be hanging due to microsoft/playwright#15550

I've updated to Playwright 1.25.0, so I think that issue should be fixed now

@Rich-Harris
Copy link
Member

What's the status of this — is it still needed? Ready for review?

@dominikg
Copy link
Member Author

oh, didn't realize the firefox issue might be fixed. Not sure if it is still needed, i think separating the write tests into their own app made this mostly obsolete. lets see what happens after bringing this up 2 date.

@dominikg
Copy link
Member Author

now linux+chrome has fails with waiting for that console message. at this point it's safe to say that this does not improve stability

@dominikg dominikg closed this Sep 22, 2022
@benmccann
Copy link
Member

I suspect the reason this was not working is vitejs/vite#7733

@dominikg
Copy link
Member Author

I suspect the reason this was not working is vitejs/vite#7733

i saw that but vite plugin svelte still uses this check, looks like playwrights console api gives you debug too

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.

None yet

3 participants