Skip to content

Upgrade Vitest, use a real browser for Map Bridges tests #2146

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.

Sign up for GitHub

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

Jump to bottom
Merged
Kocal merged 2 commits into from
Sep 10, 2024
Merged

Upgrade Vitest, use a real browser for Map Bridges tests #2146

Kocal merged 2 commits into from
Sep 10, 2024

Conversation

@Kocal
Copy link
Member

@Kocal Kocal commented Sep 10, 2024 •
edited
Loading

Q A
Bug fix? no
New feature? no
Issues Fix #...
License MIT

Follow #2138 (comment).

It looks like Google Maps changed a bit their implementation and now manipulates SVGImageElement and SVGAnimatedString:
image

But, this API is not supported by https://github.com/capricorn86/happy-dom.

Instead of reimplementing things ourselves, I think it's more clever to use a real browser (here Chromium) through Playwright, when using Vitest's Browser mode.

This PR also upgrade Vitest from ^0.34 to ^2.0, and Vite from 4.5.0 to 5.4.3, which comes with https://vitejs.dev/guide/troubleshooting.html#vite-cjs-node-api-deprecated (that I fixed in a second commit)

@carsonbot carsonbot added the Status: Needs Review Needs to be reviewed label Sep 10, 2024
@Kocal
Copy link
Member Author

Kocal commented Sep 10, 2024

For failing tests in tests-php, see #2144 (comment)

@smnandre
Copy link
Member

smnandre commented Sep 10, 2024

Symfony UX / tests-js (pull_request) Successful in 2m

Could you isolate them ? I don't think we need to run all tests in Browser every time we update a README file (i may exagerate a little bit here 👼 )

@Kocal
Copy link
Member Author

Kocal commented Sep 10, 2024 •
edited
Loading

Symfony UX / tests-js (pull_request) Successful in 2m

Could you isolate them ? I don't think we need to run all tests in Browser every time we update a README file (i may exagerate a little bit here 👼 )

I don't get what you mean, Playwright/browser tests are ran only for Map's bridges when their code change.
Vitest's Browser mode is explicitly enabled for Map's bridges, and the CI run tests-js when code (outside documentation) has been modified. 🤔

Note that, in previous tests-js CI jobs, the duration is a bit higher than before but that's fine IMO, see https://github.com/symfony/ux/actions/runs/10644425330/job/29509168600?pr=2113

@smnandre
Copy link
Member

smnandre commented Sep 10, 2024

Well i did not know... i'm sorry for asking, and... thank you very very much for all this :) 💐

@carsonbot carsonbot added Status: Reviewed Has been reviewed by a maintainer and removed Status: Needs Review Needs to be reviewed labels Sep 10, 2024
@Kocal
Copy link
Member Author

Kocal commented Sep 10, 2024

c520963a50d5847453f3395aaa0d4ffb

@Kocal Kocal merged commit 6da1135 into symfony:2.x Sep 10, 2024
@Kocal Kocal deleted the fix/map-google-test branch September 12, 2024 08:47
@Kocal Kocal mentioned this pull request Aug 22, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@smnandre smnandre smnandre approved these changes

@kbond kbond Awaiting requested review from kbond

@WebMamba WebMamba Awaiting requested review from WebMamba

Assignees

No one assigned

Labels

Status: Reviewed Has been reviewed by a maintainer

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Kocal @smnandre @carsonbot