Repository navigation
test: Give each C data integration test its own temp file - #969
Open
rustyconover wants to merge 1 commit into
Open
rustyconover wants to merge 1 commit into
rustyconover wants to merge 1 commit into
Conversation
NanoarrowIntegrationTestSchema and NanoarrowIntegrationTestBatch both wrote
c_data_integration.tmp.json in the working directory. ctest runs each gtest
case as its own process, so under ctest -j the two could overwrite or remove
each other's file; the Batch test then failed reading the Schema test's JSON
("Expected num_batch between 0 and -1 but got 1").
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #969 +/- ##
=======================================
Coverage 78.24% 78.24%
=======================================
Files 106 106
Lines 16901 16901
Branches 2012 2012
=======================================
Hits 13224 13224
Misses 2435 2435
Partials 1242 1242 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This branch has not been deployed
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
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.
NanoarrowIntegrationTestSchemaandNanoarrowIntegrationTestBatch(c_data_integration_test.cc) write their JSON to the same file,c_data_integration.tmp.json, in the working directory.gtest_discover_tests()registers each gtest case as a separate ctest test, soctest -jruns the two as parallel processes. One can then overwrite or remove the other's file. When that happens the Batch test reads the Schema test's JSON and fails with:In a full
ctest -j 12run it fails now and then. I hit it on three unrelated branches in one day. With only these two tests running they start together, and onmainctest -R NanoarrowIntegrationTest -j 2failed 199 of 200 runs. Run serially, both pass.With this PR,
TempFileincludes the current test's name (c_data_integration.<test name>.tmp.json), so each test gets its own file. It's still written to the working directory, as before, which the existing comment keeps for portability. After the change the same loop failed 0 of 200 runs, and no temp files are left behind. Full ctest passes (macOS arm64, Debug, 388 tests, three runs).🤖 Generated with Claude Code