aptos-labs / aptos-core

Aptos is a layer 1 blockchain built to support the widespread use of blockchain through better technology and user experience.
https://aptosfoundation.org
Other
6.17k stars 3.64k forks source link

[Bug] Intermittent failure in coverage tests #11445

Closed brmataptos closed 10 months ago

brmataptos commented 10 months ago

🐛 Bug

Sometimes coverage tests seem to fail to build, for example:

https://github.com/aptos-labs/aptos-core/actions/runs/7279601867/job/19836253263?pr=11079

Quoting from the build warnings there:

Run cargo llvm-cov --ignore-run-fail -p "move*" -p e2e-move-tests --lcov --jobs 32 --output-path lcov_unit.info
warning: unreachable expression
    --> aptos-move/aptos-vm/src/aptos_vm.rs:2014:79
     |
2014 |                     self.process_validator_transaction(resolver, txn.clone(), log_context);
     |                                                                  -----------  ^^^^^^^^^^^ unreachable expression
     |                                                                  |
     |                                                                  any code following this expression is unreachable
     |
note: this expression has type `aptos_types::validator_txn::ValidatorTransaction`, which is uninhabited
    --> aptos-move/aptos-vm/src/aptos_vm.rs:2014:66
     |
2014 |                     self.process_validator_transaction(resolver, txn.clone(), log_context);
     |                                                                  ^^^^^^^^^^^
     = note: `#[warn(unreachable_code)]` on by default

warning: unused variable: `vm_status`
    --> aptos-move/aptos-vm/src/aptos_vm.rs:20[13](https://github.com/aptos-labs/aptos-core/actions/runs/7279601867/job/19836253263?pr=11079#step:8:14):22
     |
2013 |                 let (vm_status, output) =
     |                      ^^^^^^^^^ help: if this is intentional, prefix it with an underscore: `_vm_status`
     |
     = note: `#[warn(unused_variables)]` on by default

warning: unused variable: `output`
    --> aptos-move/aptos-vm/src/aptos_vm.rs:2013:33
     |
2013 |                 let (vm_status, output) =
     |                                 ^^^^^^ help: if this is intentional, prefix it with an underscore: `_output`

The problem here is that txn here is of type enum ValidatorTransaction, whose definition has no values with certain feature sets:

#[derive(Clone, Debug, Eq, PartialEq, Serialize, Deserialize, CryptoHasher, BCSCryptoHash)]
pub enum ValidatorTransaction {
    #[cfg(any(test, feature = "fuzzing"))]
    DummyTopic1(DummyValidatorTransaction),
    #[cfg(any(test, feature = "fuzzing"))]
    DummyTopic2(DummyValidatorTransaction),
}

So no clone() operation is possible, presumably leading to dead code. But the code above can't be reached if there is no possible ValidatorTransaction value.

We can probably fix these warnings with just an #[allow(unreachable_code)] attribute.

To reproduce

Built without test and fuzzing:

brm@brms-mbp-2 aptos-core % cargo build -p aptos-vm |& tee test-aptos-vm.out1
   Compiling aptos-vm v0.1.0 (/Users/brm/code/aptos-core/aptos-move/aptos-vm)
warning: unused import: `StateViewId`
  --> aptos-move/aptos-vm/src/aptos_vm.rs:44:30
   |
44 |     state_store::{StateView, StateViewId},
   |                              ^^^^^^^^^^^
   |
   = note: `#[warn(unused_imports)]` on by default

warning: unreachable expression
    --> aptos-move/aptos-vm/src/aptos_vm.rs:2014:79
     |
2014 |                     self.process_validator_transaction(resolver, txn.clone(), log_context);
     |                                                                  -----------  ^^^^^^^^^^^ unreachable expression
     |                                                                  |
     |                                                                  any code following this expression is unreachable
     |
note: this expression has type `aptos_types::validator_txn::ValidatorTransaction`, which is uninhabited
    --> aptos-move/aptos-vm/src/aptos_vm.rs:2014:66
     |
2014 |                     self.process_validator_transaction(resolver, txn.clone(), log_context);
     |                                                                  ^^^^^^^^^^^
     = note: `#[warn(unreachable_code)]` on by default

warning: unused variable: `vm_status`
    --> aptos-move/aptos-vm/src/aptos_vm.rs:2013:22
     |
2013 |                 let (vm_status, output) =
     |                      ^^^^^^^^^ help: if this is intentional, prefix it with an underscore: `_vm_status`
     |
     = note: `#[warn(unused_variables)]` on by default

warning: unused variable: `output`
    --> aptos-move/aptos-vm/src/aptos_vm.rs:2013:33
     |
2013 |                 let (vm_status, output) =
     |                                 ^^^^^^ help: if this is intentional, prefix it with an underscore: `_output`
brmataptos commented 10 months ago

Seems fixed by #11452.