Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug][CLI] Coverage testing is broken for inline functions #9154

Open
alnoki opened this issue Jul 19, 2023 · 0 comments
Open

[Bug][CLI] Coverage testing is broken for inline functions #9154

alnoki opened this issue Jul 19, 2023 · 0 comments
Assignees
Labels
bug Something isn't working stale-exempt Prevents issues from being automatically marked and closed as stale

Comments

@alnoki
Copy link
Contributor

alnoki commented Jul 19, 2023

@0xjinn @banool @davidiw @gregnazario @movekevin @wrwg

Summary

The coverage tool does not work for inline functions:

  1. It gives incorrect coverage statistics for inlined functions.
  2. It inconsistently panics when trying to compare coverage results against a source file.

Steps to reproduce

git clone https://github.com/econia-labs/econia.git
cd econia
# Commit with commented out inline keyword
git checkout 30e2fe1
cd src/move/econia
aptos move test --coverage

Note that pending the resolution of #7295 this will probably take on the order of 10 minutes to complete:

...
370 tests run
...
+-------------------------+
| % Move Coverage: 100.00 |
+-------------------------+
Please use `aptos move coverage -h` for more detailed source or bytecode test coverage of this package
{
  "Result": "Success"
}

For this commit, where the inline keyword is commented out, there is no issue comparing against source:

aptos move coverage source --module user
aptos move coverage source --module market

However, when the inline keyword is uncommented (the only change in commit econia-labs/econia@464a7c9b), the coverage tool reports different coverage statistics:

# The only diff is uncommenting the `inline` keyword
git checkout 464a7c9b
aptos move test --coverage

(Again, pending the resolution of #7295 this will probably take on the order of 10 minutes to complete):

Module c0deb00c405f84c85dc13442e305df75d1288100cdd82675695f6148c7ece51c::user
>>> % Module coverage: 99.60
Module c0deb00c405f84c85dc13442e305df75d1288100cdd82675695f6148c7ece51c::market
>>> % Module coverage: 99.65
+-------------------------+
| % Move Coverage: 99.83  |
+-------------------------+

The tool also panics, inconsistently, when trying to compare against source:

RUST_BACKTRACE=full aptos move coverage source --module user
thread 'main' panicked at 'attempt to subtract with overflow', /rustc/90743e7298aca107ddaa0c202a4d3604e29bfeb6/library/core/src/ops/arith.rs:219:1
stack backtrace:
   0:        0x101edc600 - __mh_execute_header
   1:        0x101749630 - __mh_execute_header
   2:        0x101ed4dec - __mh_execute_header
   3:        0x101edc424 - __mh_execute_header
   4:        0x101ee0210 - __mh_execute_header
   5:        0x101edff80 - __mh_execute_header
   6:        0x101ee07e0 - __mh_execute_header
   7:        0x101ee05d0 - __mh_execute_header
   8:        0x101edeef4 - __mh_execute_header
   9:        0x101ee0384 - __mh_execute_header
  10:        0x102360a38 - __ZNSt3__111char_traitsIcE2eqEcc
  11:        0x102360acc - __ZNSt3__111char_traitsIcE2eqEcc
  12:        0x101a56438 - __mh_execute_header
  13:        0x101a57ed0 - __mh_execute_header
  14:        0x100db9150 - __mh_execute_header
  15:        0x100d7aa00 - __mh_execute_header
  16:        0x100d6f43c - __mh_execute_header
  17:        0x100d8853c - __mh_execute_header
  18:        0x100dba6ec - __mh_execute_header
  19:        0x100e237c0 - __mh_execute_header
  20:        0x100e23b70 - __mh_execute_header
  21:        0x100e215e8 - __mh_execute_header
  22:        0x100e21638 - __mh_execute_header
  23:        0x101ecec00 - __mh_execute_header
  24:        0x100e2161c - __mh_execute_header
  25:        0x100e244b0 - __mh_execute_header

There is no such panic in the other module, though its coverage statistics have been reported incorrectly:

aptos move coverage source --module market
@alnoki alnoki added the bug Something isn't working label Jul 19, 2023
@lbmeiyi lbmeiyi moved this from 🆕 New to For Grabs in Move Language and Runtime Jul 19, 2023
@wrwg wrwg moved this from For Grabs to 📋 Backlog in Move Language and Runtime Jul 19, 2023
alnoki added a commit to econia-labs/econia that referenced this issue Jul 19, 2023
The inline modifier needs to be commented out for
coverage testing per

aptos-labs/aptos-core#9154
@rahxephon89 rahxephon89 self-assigned this Jul 19, 2023
alnoki added a commit to econia-labs/econia that referenced this issue Jul 20, 2023
alnoki added a commit to econia-labs/econia that referenced this issue Jul 20, 2023
bogberry pushed a commit to econia-labs/econia that referenced this issue Jul 21, 2023
* Remove cancel event for limit order with full fill

* Link PR in changelog, update docs, inline keywords

The inline modifier needs to be commented out for
coverage testing per

aptos-labs/aptos-core#9154

* Update remaining size, enable posts below min size

* Comment out inline directive for coverage testing

Per aptos-labs/aptos-core#9154

* [Coverage] Uncomment inline keyword

Per aptos-labs/aptos-core#9154

* Alphabetize view func index
@sausagee sausagee added the stale-exempt Prevents issues from being automatically marked and closed as stale label Jul 22, 2023
alnoki added a commit to econia-labs/econia that referenced this issue Jul 24, 2023
alnoki added a commit to econia-labs/econia that referenced this issue Jul 28, 2023
The last commit tested Move code to 100% coverage, which
was only possible by commenting out inline keywords and
using a non-mainnet dependency per:

aptos-labs/aptos-core#9154
aptos-labs/aptos-core#9181
elliottdehn pushed a commit to econia-labs/econia that referenced this issue Aug 1, 2023
elliottdehn pushed a commit to econia-labs/econia that referenced this issue Aug 1, 2023
The last commit tested Move code to 100% coverage, which
was only possible by commenting out inline keywords and
using a non-mainnet dependency per:

aptos-labs/aptos-core#9154
aptos-labs/aptos-core#9181
alnoki added a commit to econia-labs/econia that referenced this issue Oct 26, 2023
alnoki added a commit to econia-labs/econia that referenced this issue Oct 26, 2023
alnoki added a commit to econia-labs/econia that referenced this issue Oct 26, 2023
alnoki added a commit to econia-labs/econia that referenced this issue Oct 26, 2023
alnoki added a commit to econia-labs/econia that referenced this issue Oct 28, 2023
alnoki added a commit to econia-labs/econia that referenced this issue Oct 28, 2023
alnoki added a commit to econia-labs/econia that referenced this issue Nov 6, 2023
alnoki added a commit to econia-labs/econia that referenced this issue Nov 6, 2023
alnoki added a commit to econia-labs/econia that referenced this issue Nov 6, 2023
alnoki added a commit to econia-labs/econia that referenced this issue Nov 18, 2023
alnoki added a commit to econia-labs/econia that referenced this issue Nov 18, 2023
alnoki added a commit to econia-labs/econia that referenced this issue Mar 15, 2024
alnoki added a commit to econia-labs/econia that referenced this issue Mar 15, 2024
alnoki added a commit to econia-labs/emojicoin-dot-fun that referenced this issue May 4, 2024
alnoki added a commit to econia-labs/emojicoin-dot-fun that referenced this issue May 19, 2024
alnoki added a commit to econia-labs/emojicoin-dot-fun that referenced this issue May 19, 2024
alnoki added a commit to econia-labs/emojicoin-dot-fun that referenced this issue Jun 3, 2024
alnoki added a commit to econia-labs/emojicoin-dot-fun that referenced this issue Jun 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working stale-exempt Prevents issues from being automatically marked and closed as stale
Projects
Status: 📋 Backlog
Development

No branches or pull requests

4 participants