Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

svelte: Fix failing/skipped playwright tests#62866

Merged
fkling merged 2 commits intomainfrom
fkling/srch-119-chore-fix-playwright-tests-in-web-sveltekit
May 22, 2024
Merged

svelte: Fix failing/skipped playwright tests#62866
fkling merged 2 commits intomainfrom
fkling/srch-119-chore-fix-playwright-tests-in-web-sveltekit

Conversation

@fkling
Copy link
Contributor

@fkling fkling commented May 22, 2024

Follow up for #62820 to fix failing/skipped tests that are easy to fix (which is all but one).

Re-enabling the syntax highlighting error test should be done in a separate PR.

Test plan

pnpm test

Follow up for #62820 to fix failing/skipped tests that are easy to fix
(which is all but one).
@fkling fkling requested a review from a team May 22, 2024 19:02
@fkling fkling self-assigned this May 22, 2024
@cla-bot cla-bot bot added the cla-signed label May 22, 2024
Copy link
Member

@camdencheek camdencheek left a comment

Choose a reason for hiding this comment

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

Any takeaways other than "we should run playwright in CI"?

@fkling
Copy link
Contributor Author

fkling commented May 22, 2024

@camdencheek

Any takeaways other than "we should run playwright in CI"?

It's easy to break GraphQL operation overwrites by renaming the query. TypeScript won't help atm because the overwrite API also accepts arbitrary strings, so that we can overwrite requests made by shared code (i.e. graphql queries that are not defined inside web-sveltekit). Running the tests in CI would have caught that of course, but the error still wouldn't have been obvious.

OTOH, changing the name of queries might not happen so often that it warrants any other solution then finally running the tests in CI.

@fkling fkling merged commit a376a02 into main May 22, 2024
@fkling fkling deleted the fkling/srch-119-chore-fix-playwright-tests-in-web-sveltekit branch May 22, 2024 21:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants