Skip to content

[Map] Make renderer tests way easier to maintain, use snapshots #2658

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

Merged
merged 1 commit into from
Mar 27, 2025

Conversation

Kocal
Copy link
Member

@Kocal Kocal commented Mar 25, 2025

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

@Kocal Kocal force-pushed the imp-map-renderer-tests branch from ea302ad to 0f3dc27 Compare March 25, 2025 21:46
@Kocal
Copy link
Member Author

Kocal commented Mar 25, 2025

PHP Fatal error: Declaration of PHPUnit\Framework\TestSuite::run(): void must be compatible with PHPUnit\Framework\Test::run(?PHPUnit\Framework\TestResult $result = null): PHPUnit\Framework\TestResult in /home/runner/work/ux/ux/src/Map/src/Bridge/Google/vendor/phpunit/phpunit/src/Framework/TestSuite.php on line 327

Well, let's move from Symfony's PHPUnit bridge, see #2659

EDIT: let's upgrade Map's Bridges PHPUnit to 11.5.0, that's more simple :D

@Kocal Kocal force-pushed the imp-map-renderer-tests branch 5 times, most recently from e8898f7 to b935b45 Compare March 27, 2025 07:59
@Kocal Kocal changed the title [Map] Make renderer tests way easier to maintain [Map] Make renderer tests way easier to maintain, use snapshots Mar 27, 2025
@Kocal Kocal added the DX label Mar 27, 2025
@Kocal Kocal marked this pull request as ready for review March 27, 2025 08:02
@carsonbot carsonbot added Map Status: Needs Review Needs to be reviewed labels Mar 27, 2025
@Kocal Kocal removed the Status: Needs Review Needs to be reviewed label Mar 27, 2025
@Kocal Kocal requested review from kbond, smnandre and Copilot March 27, 2025 08:02
@carsonbot carsonbot added Status: Needs Review Needs to be reviewed labels Mar 27, 2025
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 10 out of 30 changed files in this pull request and generated no comments.

Files not reviewed (20)
  • src/Map/src/Bridge/Google/composer.json: Language not supported
  • src/Map/src/Bridge/Google/phpunit.xml.dist: Language not supported
  • src/Map/src/Bridge/Google/tests/GoogleRendererTest.php: Language not supported
  • src/Map/src/Bridge/Google/tests/snapshots/GoogleRendererTest__testRenderMap with data set markers with icons__1.txt: Language not supported
  • src/Map/src/Bridge/Google/tests/snapshots/GoogleRendererTest__testRenderMap with data set simple map, with minimum options__1.txt: Language not supported
  • src/Map/src/Bridge/Google/tests/snapshots/GoogleRendererTest__testRenderMap with data set with all markers removed__1.txt: Language not supported
  • src/Map/src/Bridge/Google/tests/snapshots/GoogleRendererTest__testRenderMap with data set with controls enabled__1.txt: Language not supported
  • src/Map/src/Bridge/Google/tests/snapshots/GoogleRendererTest__testRenderMap with data set with custom attributes__1.txt: Language not supported
  • src/Map/src/Bridge/Google/tests/snapshots/GoogleRendererTest__testRenderMap with data set with default map id overridden by option mapId__1.txt: Language not supported
  • src/Map/src/Bridge/Google/tests/snapshots/GoogleRendererTest__testRenderMap with data set with default map id, when passing options (except the mapId)__1.txt: Language not supported
  • src/Map/src/Bridge/Google/tests/snapshots/GoogleRendererTest__testRenderMap with data set with default map id__1.txt: Language not supported
  • src/Map/src/Bridge/Google/tests/snapshots/GoogleRendererTest__testRenderMap with data set with every options__1.txt: Language not supported
  • src/Map/src/Bridge/Google/tests/snapshots/GoogleRendererTest__testRenderMap with data set with marker remove and new ones added__1.txt: Language not supported
  • src/Map/src/Bridge/Google/tests/snapshots/GoogleRendererTest__testRenderMap with data set with markers and infoWindows__1.txt: Language not supported
  • src/Map/src/Bridge/Google/tests/snapshots/GoogleRendererTest__testRenderMap with data set with polygons and infoWindows__1.txt: Language not supported
  • src/Map/src/Bridge/Google/tests/snapshots/GoogleRendererTest__testRenderMap with data set with polylines and infoWindows__1.txt: Language not supported
  • src/Map/src/Bridge/Google/tests/snapshots/GoogleRendererTest__testRenderMap with data set without controls enabled__1.txt: Language not supported
  • src/Map/src/Bridge/Leaflet/composer.json: Language not supported
  • src/Map/src/Bridge/Leaflet/phpunit.xml.dist: Language not supported
  • src/Map/src/Bridge/Leaflet/tests/LeafletRendererTest.php: Language not supported

Copy link
Member

@kbond kbond left a comment

Choose a reason for hiding this comment

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

Cool, I've never used snapshots before.

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

Kocal commented Mar 27, 2025

Cool, I've never used snapshots before.

I think it's something that was made popular years ago with JavaScript, especially with Jest.
Snapshots should be used cautiously, but in this case where we want to quickly assert that outputs didn't change whatever changes we do, that's fine 💯

@Kocal Kocal force-pushed the imp-map-renderer-tests branch from b935b45 to 74b9737 Compare March 27, 2025 17:10
@Kocal Kocal merged commit e653f48 into symfony:2.x Mar 27, 2025
52 checks passed
@Kocal Kocal deleted the imp-map-renderer-tests branch April 15, 2025 08:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DX Map Status: Reviewed Has been reviewed by a maintainer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants