Skip to content

Use Cleanup and RenderFrame helpers for better resource allocation and release. #8594

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

granade-work
Copy link

This builds on top of #8562 to continue test boilerplate removal.
For the distinct changes in this PR just check the last commit.
This falls into three groupings of changes.

  1. RenderFrame usage to streamline the ubiquitous BeginFrame(0,0,0) and EndFrame(0) calls throughout the tests. This is a trivial refactor and needs little more than a glance.
  2. Using Cleanup helper to handle pairing api.startCapture() calls with api.stopCapture() at the end of scope. The only question here is I may have overthought it a bit because I'm not clear exactly what the capture feature is doing. Does it matter if it is halted at test teardown time or at the specific locations it initially appeared in the tests? I assumed the latter, but if the former is the case then I can simplify a few places where I added additional scopes to have stopCapture() called in the same location it was previously.
  3. Insuring BackendTest->flushAndWait() or similar calls occur at end of test. As far as I can tell every test case uses a BackendTest fixture, which calls flushAndWait() in its destructor. As such, with a few exceptions such as tests where it was called before assertions in order to trigger work, I simply removed these calls as redundant. I will look some more for reasons that this would not be safe, but so far I have failed to find any. Please let me know if I missed something here.
@granade-work granade-work added the internal Issue/PR does not affect clients label Apr 4, 2025
@@ -290,31 +290,35 @@ TEST_F(BackendTest, UpdateImage2D) {

api.startCapture();

Cleanup cleanup(api);
cleanup.addPostCall([&]() { api.finish(); });
Copy link
Contributor

Choose a reason for hiding this comment

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

I think I prefer the api.finish(); call to be at the end of the function rather than handled through Cleanup.

This is "what feels better in my gut", but I think it doesn't have enough reason to justify breaking the more general rule of "don't have a non-linear control flow in tests".

@granade-work granade-work force-pushed the granade/cleanup_for_raii branch from 3ce9374 to debd39b Compare April 16, 2025 23:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal Issue/PR does not affect clients
2 participants