wasmtime: Numerous assert_malformed failures in test suite output in CI
Repro steps
cargo test --all --exclude lightbeam --exclude wasmtime-wasi-c --exclude wasmtime-py -- --nocapture
Expected results
The Wasm test suite tests should all pass.
Actual results
The Wasm test suites do pass, but most of the tests that check for malformed modules print error messages resulting from mismatched error strings (but otherwise pass).
In addition to looking like errors are being ignored by a non-failing assertion, the verbosity of these messages makes it hard to see which test suite tests ran in CI build logs.
About this issue
- Original URL
- State: closed
- Created 5 years ago
- Comments: 15 (15 by maintainers)
Commits related to this issue
- Ensure that our error messages match `assert_invalid`'s The bulk of this work was done in https://github.com/bytecodealliance/wasmparser/pull/186 but now we can test it at the `wasmtime` level as wel... — committed to fitzgen/wasmtime by fitzgen 4 years ago
- Stop feeling guilty about not matching `assert_malformed` messages Remove the "TODO" and stop printing warning messages. These would just be busy work to implement, and getting all the messages the e... — committed to fitzgen/wasmtime by fitzgen 4 years ago
- Ensure that our error messages match `assert_invalid`'s The bulk of this work was done in https://github.com/bytecodealliance/wasmparser/pull/186 but now we can test it at the `wasmtime` level as wel... — committed to fitzgen/wasmtime by fitzgen 4 years ago
- Stop feeling guilty about not matching `assert_malformed` messages Remove the "TODO" and stop printing warning messages. These would just be busy work to implement, and getting all the messages the e... — committed to fitzgen/wasmtime by fitzgen 4 years ago
Understood! And I know that you’d not be in any way cavalier about us passing tests we really should be failing 😃 I’m slightly concerned about whether there’s a risk of us really getting things wrong and that not being caught, but I admit that the circumstances under which that could happen are quite unlikely: AFAICT it’d require that all tests that’d cover a certain type of malformed content to also fail because of another type of malformed content. This is very different from typical JS tests, where you often have a
TypeErrorand a description, and theTypeErrorreally could be about basically anything 😃That, together with the above-described unlikeliness of anything concerning being papered over convinces me that there’s nothing to do here once you’ve landed the patch to ignore the error messages. Thank you for the thorough explanation!