Stream: git-wasmtime

Topic: wasmtime / issue #6111 Add `precise_output` argument to `...

view this post on Zulip Wasmtime GitHub notifications bot (Mar 28 2023 at 01:40):

jameysharp commented on issue #6111:

This is neat! It's clearly handy to be able to regenerate test expectations like this.

I don't know for sure that precise output tests are a good idea for optimizations though. Writing assertions about only the specific aspects we're trying to test makes the tests more resilient to unrelated changes in the optimizer.

I'm curious if @cfallin or @fitzgen have opinions on this.

view this post on Zulip Wasmtime GitHub notifications bot (Mar 28 2023 at 02:11):

Kmeakin commented on issue #6111:

I don't know for sure that precise output tests are a good idea for optimizations though. Writing assertions about only the specific aspects we're trying to test makes the tests more resilient to unrelated changes in the optimizer.

Could we do a script to automatically insert the correct filecheck commands? LLVM has something similar: https://github.com/llvm/llvm-project/blob/main/llvm/utils/update_any_test_checks.py

view this post on Zulip Wasmtime GitHub notifications bot (Mar 28 2023 at 16:39):

cfallin commented on issue #6111:

I don't know for sure that precise output tests are a good idea for optimizations though. Writing assertions about only the specific aspects we're trying to test makes the tests more resilient to unrelated changes in the optimizer.

I'm curious if @cfallin or @fitzgen have opinions on this.

Yes, I think we want to retain this property: many of the tests are checking just "this one value becomes X" rather than freezing the entire output. There is still some unnecessary capture of implementation details (e.g. the value number in the output exposes how many rewrites we went through) but less coupling is better here.

It would be nice to try to find ways to automate updates though!

view this post on Zulip Wasmtime GitHub notifications bot (Mar 28 2023 at 17:20):

fitzgen commented on issue #6111:

Yeah for tests that are asserting one particular peephole applied, it is nice to not have the full precise output. There are certainly other tests that are written in a less targeted way, and could be migrated to precise output tests on a case-by-case basis. Because of this, it would be nice to support, even if we don't blanket enable it for every test optimize test.

Last updated: Oct 23 2024 at 20:03 UTC