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

Option to treat timeouts as non-failure #1291

Open
Robbepop opened this issue Feb 4, 2024 · 4 comments
Open

Option to treat timeouts as non-failure #1291

Robbepop opened this issue Feb 4, 2024 · 4 comments
Labels
enhancement New feature or request

Comments

@Robbepop
Copy link

Robbepop commented Feb 4, 2024

I am currently using nextest via miri in Wasmi to run the entire Wasm spec testsuite (which is huge) nightly. Since miri execution with all of its checks is rather slow and since we use default GitHub action runners most tests timeout. Locally I ran the entire testsuite via miri with a timeout of 4h per test case and even then I had some timeouts, so that's how bad it really is. However, this is not as bad as it sounds since even timed out tests produced a ton of checks and validated their input until their timeout.

So what would be nice is to make it possible via config to treat timed out tests as non-failures since I only really want to be notified by this CRON miri run if it actually found an actual bug.

Is this maybe even already possible? In case you do not want to implement this feature what would be the best way to handling this on my side?

Link to the cron job if that is of interest: https://github.com/paritytech/wasmi/actions/runs/7771389011/job/21192633974

Thanks!

@Robbepop Robbepop changed the title Treat timeouts as non-failure Option to treat timeouts as non-failure Feb 5, 2024
@sunshowers
Copy link
Member

sunshowers commented Feb 5, 2024

Hi, and thanks for the feature request!

This isn't something that's supported at the moment -- timeouts with nextest are meant to be for unexpected issues.

Would you be able to add a timeout within your test logic instead? If it's useful, I'm open to adding a NEXTEST_TIMEOUT_SECONDS environment variable so you can (e.g.) multiply that by 90% to figure out when to cancel tests.

@sunshowers
Copy link
Member

Hmm, I see, you're actually using miri.

I guess we can add an on-timeout = "success"/"failure" config, and then you can set that in the Miri-specific config. I guess it'll also be useful for tests that are meant to be run for a while and then cancelled (I've come across a few such tests over the years)

@Robbepop
Copy link
Author

Robbepop commented Feb 7, 2024

tldr; An on-timeout = "success"|"failure" option is exactly what I need here. :) Though it would be nice to know which test had a timeout still in the final summary table.

Yes, I am using nextest via miri. And yes, even running into timeouts for these kinds of tests is already extremely valuable since the tests have checked various safety properties of the program along the way and as long as they haven't found issues before running out of time, I am happy.
Obviously it would be preferred if no test has a time out, but that is simply infeasible with the current hardware and in particular the current performance of miri with all of its checks.

I ran those miri tests locally on 12 cores via nextest and a timeout of 4 hours per test case and still some test cases ran out of time.

@sunshowers sunshowers added the enhancement New feature or request label Feb 23, 2024
@sunshowers
Copy link
Member

Yeah, makes sense!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants