add the existing compute unit tests to the build #179689
Conversation
There was a problem hiding this comment.
Code Review
The pull request successfully integrates existing compute unit tests into the build system, conditional on impeller_enable_compute. Additionally, it refactors the creation of HostBuffer instances in compute_unittests.cc into a dedicated helper function, CreateHostBufferFromContext. This refactoring significantly improves code maintainability by centralizing the HostBuffer initialization logic and reducing duplication across multiple test cases. The use of an anonymous namespace for the helper function is appropriate for its file-local scope.
| namespace { | ||
| std::shared_ptr<impeller::HostBuffer> CreateHostBufferFromContext( | ||
| const std::shared_ptr<impeller::Context>& context) { | ||
| return impeller::HostBuffer::Create( | ||
| context->GetResourceAllocator(), context->GetIdleWaiter(), | ||
| context->GetCapabilities()->GetMinimumUniformAlignment()); | ||
| } | ||
| } // namespace |
There was a problem hiding this comment.
Extracting the HostBuffer creation logic into CreateHostBufferFromContext is a good practice. It improves maintainability by centralizing the initialization details, making the code cleaner and easier to update if the HostBuffer::Create signature changes in the future. This also aligns with the Google C++ Style Guide's emphasis on readability and avoiding duplication.
Reference: Google C++ Style Guide - Function Names
References
- C++ code should follow the Google C++ Style Guide, which promotes readability and maintainability through practices like extracting repetitive logic into functions. (link)
|
autosubmit label was removed for flutter/flutter/179689, because The base commit of the PR is older than 7 days and can not be merged. Please merge the latest changes from the main into this branch and resubmit the PR. |
95eec19 to
f6439ab
Compare
There are existing compute unit tests in
compute_unittests.ccbut they were not added to the build. Add them ifimpeller_enable_computeis enabled.Closes #179709
Pre-launch Checklist
///).