Merged
Conversation
Previously, we had logic to omit expensive formatting when running `test_validate_rust_layout` under Miri, but this was typo'd to have the opposite effect as intended - the expensive formatting would run /only/ when running under Miri. This commit fixes that typo, and as a result, the test in question runs fast enough that we can remove other logic designed to speed up execution only under Miri. Closes #391
jswrenn
approved these changes
Sep 19, 2023
joshlf
added a commit
that referenced
this pull request
Sep 19, 2023
In #376, we introduced `test_validate_rust_layout`, which took a long time to run under Miri. In order to prevent it from consuming too much time in CI, we marked it as `#[cfg_attr(miri, ignore)]`, and configured CI to pass `-- --ignored` to Miri only a small percentage of the time. This had the unintended side effect of running `test_validate_cast_and_convert_metadata` under Miri. That test is marked as `#[cfg_attr(miri, ignore)]` for good reason - it takes far too long to run under Miri, and doesn't exercise any `unsafe` code, so doesn't benefit from Miri. Enabling this test has caused CI to timeout. In the interim, in #395, we optimized `test_validate_rust_layout` so that it no longer takes an unreasonably long time to run under Miri. Thus, in this commit, we remove the `#[cfg_attr(miri, ignore)]` annotation from that test, and no longer pass `-- --ignored` to Miri in CI. Closes #397
joshlf
added a commit
that referenced
this pull request
Sep 19, 2023
In #376, we introduced `test_validate_rust_layout`, which took a long time to run under Miri. In order to prevent it from consuming too much time in CI, we marked it as `#[cfg_attr(miri, ignore)]`, and configured CI to pass `-- --ignored` to Miri only a small percentage of the time. This had the unintended side effect of running `test_validate_cast_and_convert_metadata` under Miri. That test is marked as `#[cfg_attr(miri, ignore)]` for good reason - it takes far too long to run under Miri, and doesn't exercise any `unsafe` code, so doesn't benefit from Miri. Enabling this test has caused CI to timeout. In the interim, in #395, we optimized `test_validate_rust_layout` so that it no longer takes an unreasonably long time to run under Miri. Thus, in this commit, we remove the `#[cfg_attr(miri, ignore)]` annotation from that test, and no longer pass `-- --ignored` to Miri in CI. Closes #397
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
2 participants
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.
Previously, we had logic to omit expensive formatting when running
test_validate_rust_layoutunder Miri, but this was typo'd to have the opposite effect as intended - the expensive formatting would run /only/ when running under Miri. This commit fixes that typo, and as a result, the test in question runs fast enough that we can remove other logic designed to speed up execution only under Miri.Closes #391