[move-compiler-v2] Add all remaining V1 tests to V2, except evm/async #14578
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
⏱️ 2h 28m total CI duration on this PR
|
brmataptos
changed the title
brmataptos
changed the title
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #14578 +/- ##
=======================================
Coverage 59.8% 59.8%
=======================================
Files 851 851
Lines 207246 207246
=======================================
+ Hits 124004 124038 +34
+ Misses 83242 83208 -34 ☔ View full report in Codecov by Sentry. |
brmataptos
force-pushed
the
brm-issue-13731
branch
2 times, most recently
from
7ddf345 to
b969d80
Compare
vineethk
approved these changes
Sep 18, 2024
| fun receiver(self: &T, _x: u64) { abort 1 } | ||
|
|
||
| fun call_receiver(t: T) { | ||
| t.receiver(1); |
There was a problem hiding this comment.
Should these be run with language version 1?
There was a problem hiding this comment.
I put a copy in move-compiler-v2/tests/checking-lang-v1/. That one looks more like the V1 test.
fEst1ck
approved these changes
Sep 18, 2024
brmataptos
force-pushed
the
brm-issue-13731
branch
from
b969d80 to
3058cf5
Compare
…c., and add final set of V1 tests to V2
and add 2 other variants of the test.
brmataptos
force-pushed
the
brm-issue-13731
branch
from
988f511 to
40ac663
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
✅ Forge suite
|
✅ Forge suite
|
✅ Forge suite
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Fixes #13731.
Fixes #9003.
Add the remaining tests from V1 to V2, except for flavors (evm and async). Most are added in subdirs of a new top-level dir more-v1/, but a few are in checking-lang-v1/, deprecated/, and unused-assignment/v1-locals/.
To maximize comparability, I've also moved around some tests and/or split/duplicated them to check whether V2 catches the same errors as V1, and added bytecode generation to one set of tests, that previously stopped just before that, since it is needed by V2 to issue some errors.
See below for a link to a description of remaining test differences.
How Has This Been Tested?
Running the tests.
Key Areas to Review
Consider .rs files:
BytecodeGenphase, so that is added./more-v1, which runs most passes"expansion"and"dependencies"test dirs.Consider changes to v1.matched and v1.unmatched:
move-compiler-v2/tests/checking/tomove-compiler-v2/tests/ability-check/typing, to enable checks which are needed to find all errors/warnings in those tests./eq_invalid.expwe add/eq_invalid2.exp. These generally comment out parts of the original tests to avoid earlier errors hiding later errors in V2.Look at the changes to V1 test files files
move-compiler/.../*.move, verify that some files were moved around and/or fixed, but nothing was removed:borrow)*2.movesuch asuse_nested_self_as_invalid2.moveduplicateuse_nested_self_as_invalid.movebut with some errrors commented out or functions split to avoid shadowing in V2.acquires_error_msg_inline.moveis an added test for the inlining caseresources_invalid.movewas expanded into 3 tests to explore the original situation as well as 2 others.address_too_long*.movetests were corrected to actually have addresses too long, and new tests for the boundary cases were added.ability_constraint_*_invalid.movetests split a function in two to avoid duplicate error removal in V2.no_receiver_calls.movehas some edits to remove unrelated errors checked elsewhere.Look at the changes to V2 test files
move-compiler-v2/.../*.move, verify that (1) no files are removed, and (2) nothing significant is deleted, but (3) some new tests are added.Look at the following diffs, which compare the differing .move files corresponding to comparable .exp files in v1.matched. Note that 51 files differ a little bit, typically due to changes moving them to V2.
If you're bored, look at the output diffs below to see what is different in outputs. Or view the spreadsheet for a summary.
Analysis enabled by this PR
This PR's landing should be independent of the following, but is enabled by the PR so I mention it here:
Look at the differences in test outputs:
Note that diffs suppress files that are identical (ignoring a few v2isms like "bytecode verifier succeeded" and "Compiler v2 delivered same result).
A summary of significant changes in the test outputs is provided here:
https://docs.google.com/spreadsheets/d/12lWYfk5wIUbK4i9jXBirwv1OU7UYmdYjmwySGGThEwI/edit?usp=sharing
Type of Change
Which Components or Systems Does This Change Impact?
Checklist