Wasmtime GitHub notifications bot (Jun 20 2023 at 22:28): Wasmtime GitHub notifications bot (Jun 20 2023 at 22:28):alexcrichton commented on issue #6610:
I realize after writing that all out that you've probably already also considered such an idea, so I could also rephrase much of it as "I'm not sure why a copy of
block.wastwas added", but if that's well-motivated them seems fine!
Wasmtime GitHub notifications bot (Jun 21 2023 at 11:52):saulecabrera commented on issue #6610:
I'm not sure why a copy of block.wast was added", but if that's well-motivated them seems fine!
Yeah, the copy of the spec tests is one of the unfortunate bits of this approach. Unfortunately attempting to run all the tests and adding
#[ignore]when a non-supported feature is detected will result in most (if not all) of the official spec tests getting ignored, since as far as I can tell, there's no way to ignore spec tests at a more granular level. Even though Winch is shaping up, it doesn't offer support for finished proposals that are already part of the official spec suite, like multi-value for example, which many of the tests in the official spec test suite use. But it's totally possible that I might have missed a better way of avoiding the duplication, and if that's the case, I'm all in to explore alternatives!
Wasmtime GitHub notifications bot (Jun 21 2023 at 19:22):alexcrichton commented on issue #6610:
One idea perhaps would be to add a test for
strategyin theignorefunction against"Winch"and blanket returntrue(ignore everything) and then add a small allow-list for tests there? Or alternative blanket-ignore entire directories/subtrees but blanket-include others, for exampletests/misc_testsuite/winch/*.wastcould be tested in winch for now?Otherwise though this is a transient state of affairs while support is shored up in winch so while I personally try to minimize the churn during the transient state I don't think it's bad to have churn and/or copies of files, so again I think whatever's easiest for your development should work best. In the long run everything should be run through both Winch/Cranelift and the "weird cases" will be much more defined.
Wasmtime GitHub notifications bot (Jun 30 2023 at 18:58):saulecabrera commented on issue #6610:
I finally got a chance to get back to this! I decided to try the approach of ignoring everything except for the
winchmisc test suite, I believe this will require the least amount of changes once we decide to enable everything by default in the future.This should be ready for a review @alexcrichton; one thing to note is that I didn't add the
iftest suite yet, because I found one bug that I'm currently trying to figure out, once I do, I'll push the fix along with the test suite.
Last updated: Nov 22 2024 at 16:03 UTC