Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open up more test for Miri #6812

Open
4 tasks
tiif opened this issue Sep 1, 2024 · 8 comments
Open
4 tasks

Open up more test for Miri #6812

tiif opened this issue Sep 1, 2024 · 8 comments
Labels
A-ci Area: The continuous integration setup A-tokio Area: The main tokio crate C-feature-request Category: A feature request.

Comments

@tiif
Copy link
Contributor

tiif commented Sep 1, 2024

In the latest rustc nightly rustc 1.82.0-nightly (a7399ba69 2024-08-31), tokio API that requires epoll_wait can now run successfully in Miri. For reference, the tests that passed and were added to the Miri test suite can be found here: rust-lang/miri#3848. Is there any interest in opening up more tests to be run with Miri in tokio's CI?

TODO:

  • Bump miri version to get miri fix on some failed tests
  • Annotate why the tests are ignored for miri
  • Run docs test on miri
  • Gate the miri test if it gets too slow
@tiif tiif added A-tokio Area: The main tokio crate C-feature-request Category: A feature request. labels Sep 1, 2024
@Darksonn Darksonn added the A-ci Area: The continuous integration setup label Sep 1, 2024
@Darksonn
Copy link
Contributor

Darksonn commented Sep 1, 2024

Absolutely. Currently we only run tests inside src/ with miri, but if we can start to run tests/ too, that'd be great, as the majority of our tests are located there.

@tiif
Copy link
Contributor Author

tiif commented Sep 1, 2024

Thanks! :)

I'd like to work on this so I'd know sooner if anything ICE'd miri, or what syscall should be supported next. But I have never contributed to tokio yet, so I might request for more guidance along the way.

@Darksonn
Copy link
Contributor

Darksonn commented Sep 1, 2024

You're definitely welcome to reach out to me on zulip or discord for that.

@tiif
Copy link
Contributor Author

tiif commented Sep 3, 2024

I found an ICE rust-lang/miri#3858 on miri side, so the test that ICE'd might be skipped until the fix landed on nightly, or this change should only be approved after all fixes are landed on nightly.

@Darksonn
Copy link
Contributor

Darksonn commented Sep 3, 2024

It's of course nice to enable as many tests as possible, but I suspect going from zero to all tests would be difficult, so I am okay with PRs that enable miri only on some of the tests.

@RalfJung
Copy link

RalfJung commented Sep 22, 2024

What is the best way to enable Miri on only some of the tests? My first guess would be to enable only some of the feature gates (rather than full), as then we can easily exclude e.g. everything network- and process-related, which will clearly not work yet.

@tiif
Copy link
Contributor Author

tiif commented Sep 22, 2024

What is the best way to enable Miri on only some of the tests? My first guess would be to enable only some of the feature gates (rather than full), as then we can easily exclude e.g. everything network- and process-related, which will clearly not work yet.

Right now I am running cargo +nightly miri test --feature full and manually add #![cfg(not(miri))] on tests that hit unsupported_format.

@Darksonn
Copy link
Contributor

Darksonn commented Sep 22, 2024

Our tests usually don't specify the exact set of features they work on, and just require the full feature to be enabled. This means that enabling some feature flags will not work well.

Instead, you can either use --test file_name in the CI run to enable specific files, or enable all files. Individual tests can be disabled with #[cfg_attr(miri, ignore)]. Whether it makes sense to use --test arguments depends on how many tests need to be disabled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ci Area: The continuous integration setup A-tokio Area: The main tokio crate C-feature-request Category: A feature request.
Projects
None yet
Development

No branches or pull requests

3 participants