[Map] Add E2E tests, close #3022 #3038
Conversation
| 'react-dom/client' => [ | ||
| 'version' => '18.3.1', | ||
| ], |
|
|
||
| await expect(await page.getByTestId('map')).toBeVisible(); | ||
|
|
||
| // Since we can't test Google Maps rendering due to API costs, we only assert that Google Maps API is (wrongly) loaded |
There was a problem hiding this comment.
I would not do this test.
i think this could be a (tiny but still very real) problem in term of privacy, as we are making requests on behalf of the user... (even if it's on its own local machine)..
If someone want Google Maps fully tested, they probably already pay for GoogleMaps .....
so maybe they can invest some coins around here to support the work....
In the meantime I clearly would not spend too much time or energy to test a (very) costly (and "not-always" privacy-compliant) solution your our own time and/or money 😅
There was a problem hiding this comment.
Yeah I see what you mean, but since #2146 Map Bridges tests were run inside a browser and so requests were already made to Google.
I think that's fine if we make it opt-out explicitly:
pnpm run test:browser: run browsers tests on all UX packages withtest:browserpnpm script except UX Google Mappnpm run test:browser:no-privacy(or something similar): run browsers tests on all UX packages withtest:browserpnpm script including UX Google Map
If someone want Google Maps fully tested, they probably already pay for GoogleMaps .....
so maybe they can invest some coins around here to support the work....
About writing real tests for Google Map Bridge too, I thought about putting my own API key as a GitHub secret (so no one can see it), but if someone decide to trigger 10k times the CI, I'm bankrupt 🤯
Running a test that ensure the Google Maps API is loaded is the best of both worlds imo
| import * as fs from 'fs'; | ||
|
|
||
| export function getSymfonyKernelVersionId(): number { | ||
| const kernelPath = path.join(import.meta.dirname, '../test_apps/e2e-app/vendor/symfony/http-kernel/Kernel.php'); |
There was a problem hiding this comment.
Would use an ENV_VAR here (as its done for KERNEL_CLASS in a way)
There was a problem hiding this comment.
I thought about it too but re-using SYMFONY_REQUIRE env var would makes things counterproductive IMHO:
- it's not very intuitive to pass this env var while running Playwright, I'm sure everyone including me will forgot it
- if you change your Symfony version through
SYMFONY_REQUIRE=x.y.* composer update, you must close Playwright and re-open Playwright withSYMFONY_REQUIRE=x.y.* - there is no native
version_compare()function in Node.js, we must re-implement the logic ourselves with https://github.com/npm/node-semver for example
The VERSION_ID from test_apps/e2e-app/vendor/symfony/http-kernel/Kernel.php is the best source of truth we can get when being inside Playwright context
carsonbot
added
Status: Reviewed
I only tested Leaflet map with various features, including custom icons.
Google Maps won't be tested as it requires a paid API Key.