Fix TestGridIO race condition when running tests in parallel#2157
Open
unicomp21 wants to merge 4 commits intoAcademySoftwareFoundation:masterfrom
Open
Fix TestGridIO race condition when running tests in parallel#2157unicomp21 wants to merge 4 commits intoAcademySoftwareFoundation:masterfrom
unicomp21 wants to merge 4 commits intoAcademySoftwareFoundation:masterfrom
Conversation
When running multiple OpenVDB test processes in parallel, TestGridIO tests would fail with "KeyError: Map is not registered" errors. This was caused by all tests writing to the same hardcoded filename "something.vdb2", leading to file collisions where one process would overwrite another's test file. This change: - Adds a uniqueFilename() helper that generates filenames using the process ID and test name suffix (e.g., "test_gridio_12345_bool.vdb2") - Updates readAllTest() to accept a test name parameter for unique files - Updates testCreateWriteReadHalf() to use unique filenames - Updates all test invocations to pass appropriate test name suffixes This allows TestGridIO tests to run safely in parallel test runners. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Author
|
Just FYI guys, this code was generated w/ Anthropic/claude-code, I hit the bugs when running openvdb tests in a homegrown emulator. |
Contributor
|
The change itself looks ok, although I would recommend removing the process id. Running multiple tests concurrently across multiple processes isn't a use case we typically support. If we decided to add support for that, we'd likely have to make more sweeping changes across many of the other unit tests to do so, so I think it's best to keep it simple and to just adjust the names of the files within TestGridIO so that they are unique across the tests in this file. Finally, you need to sign off the git commit for us to be able to merge this. |
The CheckClassSize static_assert was hardcoded for 64-bit pointer sizes (96 and 88 bytes). On 32-bit systems, the accessor classes are smaller due to 4-byte pointers instead of 8-byte pointers. This change makes the size check architecture-aware by checking both 64-bit and 32-bit expected sizes based on sizeof(void*). 64-bit sizes: 96 bytes (with leaf cache), 88 bytes (without) 32-bit sizes: 56 bytes (with leaf cache), 52 bytes (without)
…ancedGrids Use unique filenames per test to avoid file conflicts when running tests in parallel: - testWriteFloatAsHalf: use testWriteFloatAsHalf.vdb2 - testWriteInstancedGrids: use testWriteInstancedGrids.vdb2 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Changed testGetMetadata, testReadAll, and testWriteOpenFile to use unique temp filenames instead of shared "something.vdb2" to prevent file conflicts when tests run in parallel. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When running multiple OpenVDB test processes in parallel, TestGridIO tests fail with "KeyError: Map is not registered" errors. This is caused by all tests writing to the same hardcoded filename
something.vdb2, leading to file collisions where one process overwrites another's test file mid-operation.Changes
uniqueFilename()helper that generates filenames using process ID and test name suffix (e.g.,test_gridio_12345_bool.vdb2)readAllTest()to accept a test name parameter for unique filestestCreateWriteReadHalf()to use unique filenamesTesting
This fix was discovered while running OpenVDB tests in a parallel test runner. The race condition manifested as:
With unique filenames per process, tests can safely run in parallel without file collisions.
Checklist
🤖 Generated with Claude Code