Fix TestGridIO race condition when running tests in parallel#2157
Fix TestGridIO race condition when running tests in parallel#2157unicomp21 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>
|
Just FYI guys, this code was generated w/ Anthropic/claude-code, I hit the bugs when running openvdb tests in a homegrown emulator. |
|
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>
|
The commits in this PR have not been signed off so the DCO check is failing. See the contributing document for more details. https://github.com/AcademySoftwareFoundation/openvdb/blob/master/CONTRIBUTING.md Feel free to re-open this PR when this has been resolved, thank you. |
2 participants
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