Skip to content

feat: Add miri testing support#1226

Open
jayvdb wants to merge 2 commits intofoyer-rs:mainfrom
jayvdb:miri
Open

feat: Add miri testing support#1226
jayvdb wants to merge 2 commits intofoyer-rs:mainfrom
jayvdb:miri

Conversation

@jayvdb
Copy link
Contributor

@jayvdb jayvdb commented Dec 15, 2025

What's changed and what's your intention?

Please explain IN DETAIL what the changes are in this PR and why they are needed. :D

Checklist

  • I have written the necessary rustdoc comments
  • I have added the necessary unit tests and integration tests
  • I have passed cargo x (or cargo x --fast instead if the old tests are not modified) in my local environment.

Related issues or PRs (optional)

Signed-off-by: John Vandenberg <jayvdb@gmail.com>
@jayvdb jayvdb changed the title Add miri testing support feat: Add miri testing support Dec 16, 2025
Signed-off-by: John Vandenberg <jayvdb@gmail.com>
}

#[cfg(feature = "tracing")]
#[cfg(all(feature = "tracing", not(feature = "tokio-console")))]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is unrelated; it allows running cargo test --all-features

Copy link
Member

@MrCroxx MrCroxx left a comment

Choose a reason for hiding this comment

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

Hi, Jhon. Thank you for your attempt to introduce miri tests.

However, from this PR it seems that bringing in miri will introduce too many special-case interferences into the current codebase. I’m doubtful whether, under these interferences, miri can still effectively verify foyer’s behavior.

@jayvdb
Copy link
Contributor Author

jayvdb commented Dec 16, 2025

I dont understand what you are saying.

Most of the codebase is now being reached with miri after the fixes to #1224 - I need to update that other PR to fix a few more bits to do with usage of the buffer.

To be more clear, locally I have all tests with an "ignore" of "issue 1224" working. And that provides very good coverage.

If there is something specific in this PR which you dont like, please let me know so we can discuss alternatives.

Not doing miri testing at all is a very bad idea.

@MrCroxx
Copy link
Member

MrCroxx commented Dec 17, 2025

I dont understand what you are saying.

Most of the codebase is now being reached with miri after the fixes to #1224 - I need to update that other PR to fix a few more bits to do with usage of the buffer.

To be more clear, locally I have all tests with an "ignore" of "issue 1224" working. And that provides very good coverage.

If there is something specific in this PR which you dont like, please let me know so we can discuss alternatives.

Not doing miri testing at all is a very bad idea.

Hi John. I am very willing to introduce miri tests, and I also support using miri to check for UB issues that are difficult for humans to detect.

What worries me is that, just as many ignore cases were added in this PR, if we can’t easily introduce miri and pass all the tests, then I’m very concerned that miri will have difficulty
fully play its role.

At the same time, if we leave too many cases that need to be ignored, it may also hinder other contributors’ development in future work.

This is just what I’m worried about and hasn’t prevented the foyer miri tests from being introduced into foyer. I’m on call this week, so my replies may be slower; please understand.

cc @tisonkun for some idea.

@codecov
Copy link

codecov bot commented Dec 17, 2025

Codecov Report

❌ Patch coverage is 50.00000% with 5 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
foyer-storage/src/io/device/file.rs 25.00% 3 Missing ⚠️
foyer-storage/src/io/device/fs.rs 33.33% 2 Missing ⚠️
Files with missing lines Coverage Δ
foyer-bench/src/main.rs 68.19% <ø> (+0.22%) ⬆️
foyer-common/src/runtime.rs 44.11% <ø> (ø)
foyer-memory/src/cache.rs 70.81% <ø> (ø)
foyer-memory/src/eviction/lru.rs 95.45% <ø> (ø)
foyer-memory/src/raw.rs 87.50% <ø> (ø)
foyer-storage/src/engine/block/buffer.rs 96.72% <ø> (ø)
foyer-storage/src/engine/block/engine.rs 89.61% <ø> (ø)
foyer-storage/src/engine/block/eviction.rs 95.79% <ø> (ø)
foyer-storage/src/engine/block/scanner.rs 95.55% <ø> (ø)
foyer-storage/src/engine/block/tombstone.rs 95.80% <ø> (ø)
... and 9 more

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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.

2 participants