[Linux] Use fixture tests in remaining unit tests#188142
Conversation
github-actions
Bot
added
a: text input
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors numerous Linux platform test files by migrating standalone TEST cases to TEST_F fixture-based tests, consolidating common setup and teardown logic. The review feedback identifies a recurring issue across several new test fixtures where member pointers are not initialized to nullptr. If an assertion fails during SetUp(), the destructors will attempt to unreference these uninitialized pointers, potentially causing segmentation faults and crashing the test runner. To prevent this, all member pointers in the test fixtures should be initialized to nullptr and safely checked before being unreferenced.
b592a74 to
b688e71
Compare
b688e71 to
b9d62d3
Compare
Refactor the Linux embedder C++ unit tests to use GoogleTest fixture classes (TEST_F) instead of repeating object setup boilerplate in each TEST. Common setup (engine, project, mock binary messenger, mock GTK/ epoxy, settings portal, etc.) is moved into fixture SetUp()/destructors. Also rename the misplaced FlValue tests that were under the FlDartProjectTest suite to FlValueTest to avoid a suite-name clash with the new fixture.
b9d62d3 to
aaf02b7
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the Linux platform unit tests by introducing GTest test fixtures across multiple test files, centralizing common setup and teardown operations such as engine initialization and messenger creation. The review feedback suggests further simplifying these test fixtures by removing the redundant engine parameter from the StartEngine helper methods, as well as moving fl_binary_messenger_shutdown calls into the destructors of the respective fixtures to eliminate repetitive cleanup boilerplate.
|
I will let you consider Gemini's feedback :) |
Move the fl_binary_messenger_shutdown calls from individual tests into the fixture destructor for the four test fixtures that hold a messenger member, removing the repeated per-test calls.
Move the view created in every FlAccessibilityHandlerTest test, and the platform handler created in the FlPlatformHandlerTest tests, into their fixture SetUp so they are created once rather than in each test.
In fixtures where the engine is a member, StartEngine always used the fixture engine, so drop the argument and use the member directly.
Move the task runner, OpenGL manager, renderable and compositor that were created identically in every test into the fixture SetUp.
Move the task runner and compositor that were created identically in every test into the fixture SetUp.
StartEngine() now defaults to starting the fixture's engine member when no engine is given, so most tests no longer need to pass it explicitly. Tests that start a different engine (e.g. a view-owned engine) still pass it.
3 participants
Refactor the Linux embedder C++ unit tests to use GoogleTest fixture classes (TEST_F) instead of repeating object setup boilerplate in each TEST. Common setup (engine, project, mock binary messenger, mock GTK/ epoxy, settings portal, etc.) is moved into fixture SetUp()/destructors.
Also rename the misplaced FlValue tests that were under the FlDartProjectTest suite to FlValueTest to avoid a suite-name clash with the new fixture.