Skip to content

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

Merged
Kocal merged 1 commit into
symfony:2.xfrom
Kocal:imp-map-renderer-tests
Mar 27, 2025
Merged

[Map] Make renderer tests way easier to maintain, use snapshots#2658
Kocal merged 1 commit into
symfony:2.xfrom
Kocal:imp-map-renderer-tests

Conversation

@Kocal

@Kocal Kocal commented Mar 25, 2025

Copy link
Copy Markdown
Member
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

Kocal commented Mar 25, 2025

Copy link
Copy Markdown
Member Author

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 Copilot, kbond and smnandre March 27, 2025 08:02
@carsonbot carsonbot added Status: Needs Review Needs to be reviewed labels Mar 27, 2025

Copilot AI left a comment

Copy link
Copy Markdown

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

@kbond kbond left a comment

Copy link
Copy Markdown
Member

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

Kocal commented Mar 27, 2025

Copy link
Copy Markdown
Member Author

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
@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.

4 participants