search

poanetwork / parity-ethereum

Fast, light, robust Ethereum implementation.
https://parity.io
Other
10 stars 12 forks source link

Fix test build. #192

Closed afck closed 5 years ago

afck commented 5 years ago

190 applies to aura-pos as well, not just to hbbft.

This PR only fixes the build, but the tests still fail.

afck commented 5 years ago

The authority_round test in ethcore/sync/src/tests/consensus.rs broke in the malice reports commit: https://github.com/poanetwork/parity-ethereum/commit/ae01f9b0a2d12f1d626aa7ebc3b2d774064c5997

---- tests::consensus::authority_round stdout ----
thread 'tests::consensus::authority_round' panicked at 'assertion failed: `(left == right)`
  left: `1`,
 right: `2`', ethcore/sync/src/tests/consensus.rs:74:2
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
vkomenda commented 5 years ago

Does also parity-ipfs-api fail to compile? I have

error[E0433]: failed to resolve: could not find `format_args` in `core`
   --> ipfs/src/lib.rs:133:8
    |
133 |         _ => format!("{}:{}", interface, port),
    |              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ could not find `format_args` in `core`
    |
    = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
vkomenda commented 5 years ago

Maybe the failure in parity-ipfs-api is a nightly glitch? I'll try a different nightly...

afck commented 5 years ago

No, I actually fixed it. Will push in a few minutes. (If I don't hit even more errors in the meantime…) The problem here was that something was imported as core, which breaks the format! macro.

I also fixed tests::consensus::authority_round, which was my fault: Seeing that AuthorityRound::step is only used in tests, I marked it #[cfg(test)]. That was a mistake because:

  1. Engine::step has a no-op default implementation.
  2. Dependent crates use step in their own tests, which means cfg(test) is false! For that purpose there's the test-helpers feature. So I guess I should have marked fn step in all three places (the Engine trait, and the impls for AuthorityRound and Clique) as #[cfg(any(test, feature = "test-helpers))] or something.

But I'll just revert my change, and possibly suggest that upstream later.

afck commented 5 years ago

I added some fixes, but there's still a lot to do:

vkomenda commented 5 years ago

Another problem is that a dependent crate fails to compile as well - jsonrpc. It doesn't make sense to correct old versions of other crates. Maybe those problems are fixed in the latest stable, in which case we could rebase on it?

afck commented 5 years ago

I agree! I'll take a look at the upstream branches tomorrow, and see by how many of these bugs they are still affected, then I'll either merge this PR, or we'll rebase.

vkomenda commented 5 years ago

The build errors in parity-ethereum only were fixed in paritytech/parity-ethereum#10991.

vkomenda commented 5 years ago

All tests pass if compiled with stable Rust 1.37.