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

Fix test build. #192

Merged
merged 1 commit into from
Sep 5, 2019
Merged

Fix test build. #192

merged 1 commit into from
Sep 5, 2019

Conversation

afck
Copy link
Collaborator

@afck afck commented Sep 4, 2019

No description provided.

@afck
Copy link
Collaborator Author

afck commented Sep 4, 2019

#190 applies to aura-pos as well, not just to hbbft.
This PR only fixes the build, but the tests still fail.

@afck
Copy link
Collaborator Author

afck commented Sep 4, 2019

The authority_round test in ethcore/sync/src/tests/consensus.rs broke in the malice reports commit: ae01f9b

---- 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
Copy link

vkomenda commented Sep 4, 2019

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
Copy link

vkomenda commented Sep 5, 2019

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

@afck
Copy link
Collaborator Author

afck commented Sep 5, 2019

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
Copy link
Collaborator Author

afck commented Sep 5, 2019

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

  • v1::tests::eth::eth_get_block_by_hash still fails.
  • I couldn't fix evmbin/benches, so I disabled it for now. It looks horribly outdated.

@vkomenda
Copy link

vkomenda commented Sep 5, 2019

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?

Copy link

@vkomenda vkomenda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All ethcore tests pass, which is the main thing. There are problems outside ethcore due to the code becoming outdated. Maybe we could rebase onto the latest stable instead of starting to fix failures one by one.

@afck
Copy link
Collaborator Author

afck commented Sep 5, 2019

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
Copy link

vkomenda commented Sep 5, 2019

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

@vkomenda
Copy link

vkomenda commented Sep 5, 2019

All tests pass if compiled with stable Rust 1.37.

@vkomenda vkomenda merged commit 4eaa4a3 into aura-pos Sep 5, 2019
@afck afck deleted the afck-compile-tests branch September 6, 2019 08:26
afck added a commit that referenced this pull request Oct 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants