Stream: git-wasmtime

Topic: wasmtime / PR #3612 cranelift: Add ability to auto-update...

Wasmtime GitHub notifications bot (Dec 16 2021 at 22:45):

alexcrichton opened PR #3612 from update-tests to main:

One of the problems of the current *.clif testing is that the files
are difficult to update when widespread changes are made (such as
removing modification of the frame pointer). Additionally when changing
register allocation or similar it can cause a large number of changes in
tests but the tests themselves didn't actually break. For this reason
this commit adds the ability to automatically update test expectations.

The idea behind this commit is that tests of the form test compile can
also optionally be flagged with the precise-output flag:

test compile precise-output

and when doing so the compiled form of each function is asserted to 100%
match the following comments and their test expectations. If a match is
not found then a BLESS=1 environment variable can be used to
automatically rewrite the test file itself with the correct assertion.
If the environment variable isn't present and the expectation doesn't
match then the test fails.

It's hoped that, if approved, a follow-up commit can add
precise-output to all current test compile tests (or make it the
default) and all tests can be mass-updated. When developing locally test
expectations need not be written and instead tests can be run with
BLESS=1 and the output can be manually verified. The environment
variable will not be present on CI which means that changes to the
output which don't also change the test expectation will cause CI to
fail. Furthermore this should still make updates to the test output
easily readable in review on CI because the test expectations are
intended to look the same as before.

Closes #1539

<!--

Please ensure that the following steps are all taken care of before submitting
the PR.

• [ ] This has been discussed in issue #..., or if not, please tell us why
  here.

• [ ] A short description of what this does, why it is needed; if the
  description becomes long, the matter should probably be discussed in an issue
  first.

• [ ] This PR contains test cases, if meaningful.

• [ ] A reviewer from the core maintainer team has been assigned for this PR.
  If you don't know who could review this, please indicate so. The list of
  suggested reviewers on the right can help you.

Please ensure all communication adheres to the code of conduct.
-->

Wasmtime GitHub notifications bot (Dec 21 2021 at 15:47):

alexcrichton updated PR #3612 from update-tests to main.

Wasmtime GitHub notifications bot (Dec 21 2021 at 15:49):

alexcrichton updated PR #3612 from update-tests to main.

Wasmtime GitHub notifications bot (Dec 21 2021 at 18:46):

cfallin created PR review comment:

Update "TODO" comment here?

Wasmtime GitHub notifications bot (Dec 21 2021 at 18:46):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Dec 21 2021 at 18:46):

cfallin submitted PR review.

Wasmtime GitHub notifications bot (Dec 21 2021 at 18:46):

cfallin created PR review comment:

tiniest of nits: I think I'd prefer the environment variable to be namespaced a bit just to avoid an edge case where the user has an environment variable in their usual environment that causes accidental weirdness. (BLESS isn't out of the realm of possibility of some other tool also claiming...) Maybe CRANELIFT_TEST_BLESS?

Wasmtime GitHub notifications bot (Dec 22 2021 at 15:58):

alexcrichton updated PR #3612 from update-tests to main.

Wasmtime GitHub notifications bot (Jan 10 2022 at 17:16):

alexcrichton updated PR #3612 from update-tests to main.

Wasmtime GitHub notifications bot (Jan 10 2022 at 17:59):

alexcrichton merged PR #3612.

Last updated: Nov 22 2024 at 16:03 UTC