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.

Sign up for GitHub

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

Jump to bottom

Move tests and associated data types into their own crate #10

Merged
merged 5 commits into from
Oct 24, 2023
Merged

Move tests and associated data types into their own crate #10

merged 5 commits into from
Oct 24, 2023

Conversation

romac
Copy link
Member

@romac romac commented Oct 23, 2023 •
edited
Loading

Doing this ensure that we publicly export all the necessary definitions from the main crates to test them exhaustively, so that when we move to MBT we'll be in a good shape and won't have to rework the codebase.

@codecov
Copy link

codecov bot commented Oct 23, 2023 •
edited
Loading

Codecov Report

Merging #10 (2168413) into romac/data-generic (ef913ce) will decrease coverage by 7.24%.
The diff coverage is 94.20%.

@@                  Coverage Diff                   @@
##           romac/data-generic      #10      +/-   ##
======================================================
- Coverage               84.61%   77.37%   -7.24%     
======================================================
  Files                      16       17       +1     
  Lines                    1150      782     -368     
======================================================
- Hits                      973      605     -368     
  Misses                    177      177
Files Coverage Δ
Code/common/src/vote.rs 100.00% <ø> (ø)
Code/consensus/src/executor.rs 76.74% <100.00%> (-13.67%) ⬇️
Code/round/src/state_machine.rs 48.54% <ø> (-12.00%) ⬇️
Code/test/src/proposal.rs 100.00% <100.00%> (ø)
Code/test/src/vote.rs 100.00% <100.00%> (ø)
Code/vote/src/count.rs 80.65% <ø> (-9.98%) ⬇️
Code/vote/src/keeper.rs 65.91% <ø> (-20.45%) ⬇️
Code/test/src/consensus.rs 90.00% <90.00%> (ø)
Code/test/src/height.rs 57.14% <57.14%> (ø)
Code/test/src/value.rs 86.96% <86.96%> (ø)
... and 1 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@romac romac force-pushed the romac/data-generic branch from 4e36931 to 94c8651 Compare October 24, 2023 07:28
@romac romac force-pushed the romac/data-generic branch 2 times, most recently from a9af4d9 to 1d0ec33 Compare October 24, 2023 08:00
@romac romac requested a review from ancazamfir October 24, 2023 08:02
@romac romac marked this pull request as ready for review October 24, 2023 08:02
@romac
Copy link
Member Author

romac commented Oct 24, 2023

Drop in coverage is likely because a bunch of code was moved to the test crate, ie. deleted from the main crates and added to the test crate.

@romac romac merged commit 60c20f6 into romac/data-generic Oct 24, 2023
7 checks passed
@romac romac deleted the romac/test-crate branch October 24, 2023 13:30
romac added a commit that referenced this pull request Oct 24, 2023
@romac
Move tests and associated data types into their own crate (#10)
59e04b3 
* Move tests and associated data types into their own crate

* Add small threshold to codecov

* Small cleanup

* Move tests into `tests` folder

* Adjust base when computing coverage in the presence of removed code

See: https://docs.codecov.com/docs/commit-status#removed_code_behavior
romac added a commit that referenced this pull request Oct 25, 2023
@romac
Abstract over the various data types needed by the consensus engine (#8)
3fea113 
Closes: #11

This gist of this PR is that it adds a `Consensus` trait which defines the various datatypes that the consensus engine operates over, as well as a trait per such datatype that defines the requirement for said datatype.

```rust
pub trait Consensus
where
    Self: Sized,
{
    type Address: Address;
    type Height: Height;
    type Proposal: Proposal<Self>;
    type PublicKey: PublicKey;
    type Validator: Validator<Self>;
    type ValidatorSet: ValidatorSet<Self>;
    type Value: Value;
    type Vote: Vote<Self>;
```

```rust
pub trait Height
where
    Self: Clone + Debug + PartialEq + Eq + PartialOrd + Ord,
{
}
```

etc.

In test code, we define our own simple versions of these datatypes as well as a `TestConsensus` struct, for which we implement the `Consensus` trait.


```rust
#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
pub struct Address(u64);

impl Address {
    pub const fn new(value: u64) -> Self {
        Self(value)
    }
}

impl malachite_common::Address for Address {}
```

```rust
pub struct TestConsensus;

impl Consensus for TestConsensus {
    type Address = Address;
    type Height = Height;
    type Proposal = Proposal;
    type PublicKey = PublicKey;
    type ValidatorSet = ValidatorSet;
    type Validator = Validator;
    type Value = Value;
    type Vote = Vote;
}
```

---

* Attempt to abstract over the various data types needed by the consensus engine

* Fix tests

* Fix lints names

* Move tests and associated data types into their own crate (#10)

* Move tests and associated data types into their own crate

* Add small threshold to codecov

* Small cleanup

* Move tests into `tests` folder

* Adjust base when computing coverage in the presence of removed code

See: https://docs.codecov.com/docs/commit-status#removed_code_behavior

* Document the `Round` type and rename `Round::None` to `Round::Nil`

* More doc comments

* Remove `PublicKey::hash` requirement

* Few more comments

* Add propose+precommit timeout test
romac added a commit that referenced this pull request Nov 8, 2023
@romac
Abstract over the various data types needed by the consensus engine (#8)
cf00c46 
Closes: #11

This gist of this PR is that it adds a `Consensus` trait which defines the various datatypes that the consensus engine operates over, as well as a trait per such datatype that defines the requirement for said datatype.

```rust
pub trait Consensus
where
    Self: Sized,
{
    type Address: Address;
    type Height: Height;
    type Proposal: Proposal<Self>;
    type PublicKey: PublicKey;
    type Validator: Validator<Self>;
    type ValidatorSet: ValidatorSet<Self>;
    type Value: Value;
    type Vote: Vote<Self>;
```

```rust
pub trait Height
where
    Self: Clone + Debug + PartialEq + Eq + PartialOrd + Ord,
{
}
```

etc.

In test code, we define our own simple versions of these datatypes as well as a `TestConsensus` struct, for which we implement the `Consensus` trait.


```rust
#[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
pub struct Address(u64);

impl Address {
    pub const fn new(value: u64) -> Self {
        Self(value)
    }
}

impl malachite_common::Address for Address {}
```

```rust
pub struct TestConsensus;

impl Consensus for TestConsensus {
    type Address = Address;
    type Height = Height;
    type Proposal = Proposal;
    type PublicKey = PublicKey;
    type ValidatorSet = ValidatorSet;
    type Validator = Validator;
    type Value = Value;
    type Vote = Vote;
}
```

---

* Attempt to abstract over the various data types needed by the consensus engine

* Fix tests

* Fix lints names

* Move tests and associated data types into their own crate (#10)

* Move tests and associated data types into their own crate

* Add small threshold to codecov

* Small cleanup

* Move tests into `tests` folder

* Adjust base when computing coverage in the presence of removed code

See: https://docs.codecov.com/docs/commit-status#removed_code_behavior

* Document the `Round` type and rename `Round::None` to `Round::Nil`

* More doc comments

* Remove `PublicKey::hash` requirement

* Few more comments

* Add propose+precommit timeout test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ancazamfir ancazamfir Awaiting requested review from ancazamfir

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@romac