Fixed miri tests #646
Merged
Merged
Conversation
Signed-off-by: Marvin Hansen <marvin.hansen@gmail.com>
Signed-off-by: Marvin Hansen <marvin.hansen@gmail.com>
…leaves Restructure tests/quantities/ to the kernels convention: each quantity is a folder (no _tests suffix) with a declarations-only mod.rs plus self-contained *_tests.rs leaves. Split the seven files over 250 lines by quantity type; extract the fourteen single-file folders into dedicated leaves. Export shared SolenoidalField test fixtures to deep_causality_physics:: utils_tests so each leaf compiles standalone under Bazel's per-file rust_test_suite. Update the quantities glob to quantities/*/*_tests.rs. cargo test: 1563 pass. bazel test //...: 865 pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Marvin Hansen <marvin.hansen@gmail.com>
Signed-off-by: Marvin Hansen <marvin.hansen@gmail.com>
Contributor
There was a problem hiding this comment.
10 issues found across 129 files
Note: This PR contains a large number of files. cubic only reviews up to 100 files per PR, so some files may not have been reviewed. cubic prioritizes the most important files to review.
On a pro plan you can use ultrareview for larger PRs.
Re-trigger cubic
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #646 +/- ##
=======================================
Coverage 97.56% 97.57%
=======================================
Files 1114 1115 +1
Lines 57243 57313 +70
=======================================
+ Hits 55851 55921 +70
Misses 1392 1392 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Address P3 review findings — mostly pre-existing inconsistent test rigor surfaced by the quantities test reorganization, where Mass was authored more thoroughly than its sibling scalars. - dynamics/mass: assert the PhysicalInvariantBroken/"finite" variant on the NaN and Infinity paths, and tighten the negative-value check to require "negative" (it previously also passed on any "Mass" message, so the finite-error message would have satisfied it). - dynamics/moment_of_inertia: assert the stored value on the valid path and the PhysicalInvariantBroken/"negative" variant on rejection, matching the Mass exemplar. - dynamics/length: add the missing new(0.0) zero-length case that Mass and Speed already cover. - condensed/trait_coverage: exercise PartialOrd for all seven scalars rather than only Conductance, in a file dedicated to trait coverage. - condensed/vector_potential: split the combined default+new test in two and assert the default multivector's value is 0.0, not only its length. Also: - makefile: document the `miri` target in `make help`. - deep_causality_algorithms/tests: fix "foreever" comment typo. cargo test -p deep_causality_physics: 1565 pass. clippy and fmt clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: Marvin Hansen <marvin.hansen@gmail.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
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.
Describe your changes
Fixed miri tests
Issue ticket number and link
closes #643
Code checklist before requesting a review
For details on make, please see BUILD.md
Note: The CI runs all of the above and fixing things before they hit CI speeds
up the review and merge process. Thank you.
Summary by cubic
Stabilizes Miri test runs across the workspace by adding a Miri runner and conditionally skipping unsupported or long-running tests. Tightens physics quantity assertions and trait coverage to prevent regressions.
Bug Fixes
build/scripts/miri.shto runcargo miri testacross all crates.#[cfg(not(miri))]/#[cfg_attr(miri, ignore)]indeep_causality_algorithmsanddeep_causality_cfd(DAG sampling, IO, solver/coupling).deep_causality_physicsquantity tests with stricter invariants and trait coverage (e.g.,Mass,MomentOfInertia, zero-length inLength,VectorPotentialdefault).Refactors
deep_causality_physicsquantity tests into per-quantity folders with leaf*_tests.rsand lightweightmod.rs.deep_causality_physics::utils_tests(behindalloc) for reuse across leaves.quantities/*/*_tests.rsto match the new layout.Written for commit 97f82c2. Summary will update on new commits.