Repository navigation
test(cuda.core): fold the pairwise cross-type object-protocol tests into one equality test - #3044
Merged
Merged
Conversation
…nto one equality test No linked issue; test-suite maintenance. Refs NVIDIA#2745. Co-Authored-By: Claude Fable 5.1 <[email protected]>
Andy-Jost
requested review from
jpascucci-nv,
juenglin,
leofang,
mdboom and
rwgk
October 7, 2026 16:45
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 7 remain after this review. 📝 SummarySummary by CodeRabbit
WalkthroughEquality protocol tests now use resolved samples, record skipped fixture details, and check equality and inequality across sample pairs. The cross-type hash inequality test was removed. ChangesObject protocol tests
Suggested reviewers: Priority: ⬇️ Low Change: Other Merge Risk: ⚪ Minimal · up to This change only restructures tests to cut CI time and does not alter runtime behavior. No merge-blocking risk was identified.
Comment |
jpascucci-nv
approved these changes
Oct 7, 2026
Andy-Jost
enabled auto-merge (squash)
October 7, 2026 18:30
juenglin
approved these changes
Oct 7, 2026
Contributor
|
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.
Description
No linked issue; test-suite maintenance. Refs #2745.
test_hash_distinct_cross_typeandtest_no_cross_type_equalityincuda_core/tests/test_object_protocols.pywere parametrized overitertools.combinationsof the 25 sample types: 600 of the module's 898 items per CI job, each paying the per-test CUDA context setup and teardown to assert one comparison. On the L4 push row the two tests took about 40 s of a 288 sRun cuda.core testsstep (step timestamps of a recent push run on main). A separate single measurement put the 600 items at 43.7 s with the per-test teardown and 5.1 s without it, so the cost is the item count, not the comparisons.Changes:
test_no_cross_type_equalityis now one unparametrized test over a neweq_samplesfixture. The fixture resolves everyEQ_TYPESsample at setup on the main thread (pytest-run-parallel workers only read), and the test compares all pairs; a failure lists every offending pair by fixture name.skipped_samplesproperty; the remaining types are still compared.test_hash_distinct_cross_typeis removed. Python does not promise distinct hashes for objects of different types (the test's own comment said so).test_hash_consistency,test_hash_distinct_same_type, andtest_equality_same_handleremain.HASH_TYPESandEQ_TYPESare unchanged; a new type is still one table row.Collected items in the module: 898 -> 299 per job. The 600 removed items covered no
cuda.coreline that another test does not also cover (per-test coverage contexts, 2026-09-10 coverage report).Checklist
🤖 Generated with Claude Code