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

Refactor the #[wasmtime_test] macro #9627

Merged

Conversation

alexcrichton
Copy link
Member

  • Start tests with a blank slate of features instead of with the default set of features enables (ensures each test explicitly specifies required features)

  • Reuse test features from wasmtime_wast_util to avoid duplicating listings of features. Also shares logic for "should this compiler fail this test because of unsupported features".

  • Move logic in tests/wast.rs to apply test configuration to a Config to a new location that can be shared across suites.

  • Add a new feature for simd and flag tests that need it with the feature.

This is done in preparation for adding a new compiler strategy of Pulley to be able to flag tests as passing for pulley or not.

* Start tests with a blank slate of features instead of with the default
  set of features enables (ensures each test explicitly specifies
  required features)

* Reuse test features from `wasmtime_wast_util` to avoid duplicating
  listings of features. Also shares logic for "should this compiler fail
  this test because of unsupported features".

* Move logic in `tests/wast.rs` to apply test configuration to a
  `Config` to a new location that can be shared across suites.

* Add a new feature for `simd` and flag tests that need it with the feature.

This is done in preparation for adding a new compiler strategy of Pulley
to be able to flag tests as passing for pulley or not.
@alexcrichton alexcrichton requested review from a team as code owners November 20, 2024 15:53
@alexcrichton alexcrichton requested review from pchickey and removed request for a team November 20, 2024 15:53
Copy link
Contributor

@abrown abrown left a comment

Choose a reason for hiding this comment

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

This improvement looks good!

}
});
let ignore = if strategy.should_fail(&test_config.flags) {
quote!(#[ignore])
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this should be #[should_panic]? I get that this keeps the original behavior but more just wondering: if we check for panics and then it stops panicking then we know more quickly which Winch tests can be enabled.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point! Works well 👍

@github-actions github-actions bot added the fuzzing Issues related to our fuzzing infrastructure label Nov 20, 2024
Copy link

Subscribe to Label Action

cc @fitzgen

This issue or pull request has been labeled: "fuzzing"

Thus the following users have been cc'd because of the following labels:

  • fitzgen: fuzzing

To subscribe or unsubscribe from this label, edit the .github/subscribe-to-label.json configuration file.

Learn more.

@alexcrichton alexcrichton added this pull request to the merge queue Nov 20, 2024
Merged via the queue into bytecodealliance:main with commit 4f386b7 Nov 20, 2024
40 checks passed
@alexcrichton alexcrichton deleted the refactor-wasmtime-test branch November 20, 2024 19:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fuzzing Issues related to our fuzzing infrastructure
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants