[Bug] Useful move-cli tests should be added to aptos code base #13119
Comments
|
To regenerate these files, we can run |
aptos-move/move-examples are run with `MOVE_COMPILER_V2=true cargo test`. Change compiler-v2 sources and dependencies from `Vec<String>` filenames to `Vec<PackagePaths>` full package descriptors. This attaches a package name to input files, allowing a filter to limit unit tests to the current package. Since these are bigger structures, we pass `&` instead of object. There is some messiness due to `PackagePaths` really being a generic type with default instantiation using `move_symbol_pool::Symbol` which isn't common in the move-model code. Also, it turns out that disassembler uses script module names whose name mangling has changed, so there is a change to `move-cli/src/base/disassemble.rs`, along with a more verbose error (listing the available functions) in the case of a failure to find the requested module, which was useful debugging and was left in. Since related code was touched in EVM-related code in `third_party/move/tools/move-unit-test/src/test_runner.rs` I fixed it to compile but there is still an issue with the `named_address_map` which I haven't debugged. (See Issue #13119.)
|
After #13125 lands these will at least compile. More work is required to fix the test outputs. Notably, the attribute maps don't seem to be readily available in |
aptos-move/move-examples are run with `MOVE_COMPILER_V2=true cargo test`. Change compiler-v2 sources and dependencies from `Vec<String>` filenames to `Vec<PackagePaths>` full package descriptors. This attaches a package name to input files, allowing a filter to limit unit tests to the current package. Since these are bigger structures, we pass `&` instead of object. There is some messiness due to `PackagePaths` really being a generic type with default instantiation using `move_symbol_pool::Symbol` which isn't common in the move-model code. Also, it turns out that disassembler uses script module names whose name mangling has changed, so there is a change to `move-cli/src/base/disassemble.rs`, along with a more verbose error (listing the available functions) in the case of a failure to find the requested module, which was useful debugging and was left in. Since related code was touched in EVM-related code in `third_party/move/tools/move-unit-test/src/test_runner.rs` I fixed it to compile but there is still an issue with the `named_address_map` which I haven't debugged. (See Issue #13119.)
|
Those tests are dead since a while and the .exp where left there. These are smoke tests for evm integration into the move cli, and not relevant for for testing of the evm compiler. Basically all tests in move-cli are obsolete, the entire crate is deprecated and should be removed, which requires dealing with a few upstream dependencies. |
wrwg
changed the title
|
I reopened and renamed the bug to remove that crate. Lets not invest any time in trying to fix anything here. |
|
We should not remove tests until we have equivalent tests in the Aptos
world if we have similar functionality. Disassembler, etc. Evm can die,
sure.
…On Mon, Apr 29, 2024, 11:35 PM Wolfgang Grieskamp ***@***.***> wrote:
I reopened and renamed the bug to remove that crate. Lets not invest any
time in trying to fix anything here.
—
Reply to this email directly, view it on GitHub
<#13119 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A7S3T4YTLZFUL44GVOXYINDY743RZAVCNFSM6AAAAABG7BSOZKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOBUGQ4DENJQGQ>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
These tests were deactivated like one year ago or so. |
|
The move-cli tests were the only way I was able to reproduce and debug the V2 disassembler problem. We need a replacement. |
brmataptos
changed the title
aptos-move/move-examples are run with `MOVE_COMPILER_V2=true cargo test`. Change compiler-v2 sources and dependencies from `Vec<String>` filenames to `Vec<PackagePaths>` full package descriptors. This attaches a package name to input files, allowing a filter to limit unit tests to the current package. Since these are bigger structures, we pass `&` instead of object. There is some messiness due to `PackagePaths` really being a generic type with default instantiation using `move_symbol_pool::Symbol` which isn't common in the move-model code. Also, it turns out that disassembler uses script module names whose name mangling has changed, so there is a change to `move-cli/src/base/disassemble.rs`, along with a more verbose error (listing the available functions) in the case of a failure to find the requested module, which was useful debugging and was left in. Since related code was touched in EVM-related code in `third_party/move/tools/move-unit-test/src/test_runner.rs` I fixed it to compile but there is still an issue with the `named_address_map` which I haven't debugged. (See Issue #13119.)
🐛 Bug
The move-cli crate is in an inconsistent state and should be removed, which however, requires cutting some upstream dependencies.
For example:
I don't seem able to trigger the tests that are recorded in exp files such as
third_party/move/tools/move-cli/tests/build_tests/unbound_dependency/args.evm.expIn particular,
runs other tests, but not the EVM tests.
It seems likely that tests with evm-backend feature are not being run, so probably also the ones corresponding to outputs
third_party/move/tools/move-unit-test/tests/test_sources/address_args.evm.exp
The text was updated successfully, but these errors were encountered: