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

try to get some tests passing #5333

Merged
merged 11 commits into from Jul 1, 2022
Merged

try to get some tests passing #5333

merged 11 commits into from Jul 1, 2022

Conversation

Rich-Harris
Copy link
Member

@Rich-Harris Rich-Harris commented Jun 30, 2022

I'm tearing my hair out over some of these. Running pnpm test locally consistently fails — some of the tests are flaky, others (like handles external api) fail reliably, and I'm struggling to understand why. It feels like a fairly recent phenomenon.

The handles external api case is particularly weird. The server that serves /server-fetch-request-modified.json is created twice (once for javaScriptEnabled: true and once for false), and listens on the same port in both cases. Adding logging shows that the first server is closed before the second server is started, so it's not weird that the original port is still available, but what is weird is that the first server handles the request intended for the second server even though it's already been closed. Because of that, the request isn't intercepted in the right place.

I'm sure someone with a better understanding of all this could fix it in no time.

Sometimes tests fail with ERR_NETWORK_CHANGED; I have no idea what's causing those.

Flaky tests that fail with more specific errors (may add to this list as I find more):

  • navigation is cancelled upon subsequent navigation
  • links to unmatched routes result in a full page navigation, not a 404
  • rest parameters do not swallow characters
  • $page.url.hash is correctly set on navigation

@changeset-bot
Copy link

changeset-bot bot commented Jun 30, 2022

⚠️ No Changeset found

Latest commit: eb9cd4b

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

@benmccann
Copy link
Member

benmccann commented Jul 1, 2022

I've been testing various commits. I'm getting hanging behavior with a6781ae, but not the commit before and its pretty consistent

The only explanation I can come up with as to why that commit changed what we're seeing is that it made things more efficient by not watching the generated directory and so we're now more easily hitting whatever race condition is causing the hang

@Rich-Harris
Copy link
Member Author

Merging this so that it's easier to diagnose the browser-specific failures exposed by #5058

@Rich-Harris Rich-Harris merged commit aae5c65 into master Jul 1, 2022
@Rich-Harris Rich-Harris deleted the flaky-tests branch July 1, 2022 16:35
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

2 participants