Stream: git-wasmtime

Topic: wasmtime / PR #8789 Initial migration to `wasmtime_test`

Wasmtime GitHub notifications bot (Jun 12 2024 at 22:22):

saulecabrera opened PR #8789 from saulecabrera:start-migration-to-wasmtime-test to bytecodealliance:main:

This commit introduces the initial migration to the wasmtime_test macro.

This change starts by migrating all the applicable func.rs integration tests and at the same time it removes all the duplicated integration tests in winch.rs.

Additionally, this change introduces a slight change to how the macro works. Inspired by https://github.com/bytecodealliance/wasmtime/pull/8622#pullrequestreview-2083046911 it defaults to including all the known configuration combinations and allows the macro user to opt-out where applicable. This makes the usage of the macro less verbose.

The intention is to follow-up with subsequent PRs to migrate the all the applicable tests.

<!--
Please make sure you include the following information:

• If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

• Explain why this change is needed. If the details are in an issue already,
  this can be brief.

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

Wasmtime GitHub notifications bot (Jun 12 2024 at 22:22):

saulecabrera requested alexcrichton for a review on PR #8789.

Wasmtime GitHub notifications bot (Jun 12 2024 at 22:22):

saulecabrera requested wasmtime-core-reviewers for a review on PR #8789.

Wasmtime GitHub notifications bot (Jun 12 2024 at 22:29):

saulecabrera updated PR #8789.

Wasmtime GitHub notifications bot (Jun 12 2024 at 22:33):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Jun 12 2024 at 22:33):

alexcrichton submitted PR review.

Wasmtime GitHub notifications bot (Jun 12 2024 at 22:33):

alexcrichton created PR review comment:

Similar to below adding features(simd) here might make sense to auto-exclude Winch

Wasmtime GitHub notifications bot (Jun 12 2024 at 22:33):

alexcrichton created PR review comment:

Thought on this, the reason that this test isn't supported on Winch is because it requires wasm gc, right? If that's the case it might be reasonable to have something like features(gc) for this macro. The macro would have built-in knowledge that Winch doesn't support gc but it would also additionally pre-configure the input Config to have wasm_gc(true) for example.

That might make things easier to un-ignore for winch in the future as it would only involve changing one location for "ok now winch supports gc" as opposed to hunting down the various exceptions

Wasmtime GitHub notifications bot (Jun 13 2024 at 20:48):

saulecabrera updated PR #8789.

Wasmtime GitHub notifications bot (Jun 13 2024 at 20:50):

saulecabrera commented on PR #8789:

Thanks for the review @alexcrichton. Your suggestions make sense to me, I've pushed an update in https://github.com/bytecodealliance/wasmtime/pull/8789/commits/9638a94bda3cc6dfa23101b393ded52f4fd0bbdd. Let me know what you think. I also removed some redundant config.wasm_<feature> for features that are now enabled by default (e.g. reference_types).

Wasmtime GitHub notifications bot (Jun 13 2024 at 20:55):

alexcrichton submitted PR review:

Looks great, and thanks!

Wasmtime GitHub notifications bot (Jun 13 2024 at 20:55):

alexcrichton has enabled auto merge for PR #8789.

Wasmtime GitHub notifications bot (Jun 13 2024 at 21:11):

alexcrichton merged PR #8789.

Last updated: Nov 22 2024 at 16:03 UTC