Skip to content

chore: Give each C data integration test its own temp file - #969

Merged
paleolimbot merged 1 commit into
apache:mainfrom
rustyconover:fix/c-data-integration-test-temp-file
Oct 20, 2026
Merged

paleolimbot merged 1 commit into
apache:mainfrom
rustyconover:fix/c-data-integration-test-temp-file

Conversation

@rustyconover

Copy link
Copy Markdown
Contributor

NanoarrowIntegrationTestSchema and NanoarrowIntegrationTestBatch (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, so ctest -j runs 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:

Expected num_batch between 0 and -1 but got 1

In a full ctest -j 12 run 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 on main ctest -R NanoarrowIntegrationTest -j 2 failed 199 of 200 runs. Run serially, both pass.

With this PR, TempFile includes 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

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-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.24%. Comparing base (e5f3d0a) to head (54f30ee).

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@paleolimbot paleolimbot changed the title test: Give each C data integration test its own temp file chore: Give each C data integration test its own temp file Oct 20, 2026

@paleolimbot paleolimbot left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you! These have always been fast enough that I've never had to run in parallel but it should certainly be supported.

@paleolimbot
paleolimbot merged commit dc36606 into apache:main Oct 20, 2026
43 checks passed
@rustyconover
rustyconover deleted the fix/c-data-integration-test-temp-file branch October 20, 2026 03:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants