Valid TrustThresholdFraction and further simplify tests code #142
Conversation
Codecov Report
@@ Coverage Diff @@
## master #142 +/- ##
==========================================
+ Coverage 36.95% 37.01% +0.06%
==========================================
Files 91 91
Lines 3277 3277
Branches 510 510
==========================================
+ Hits 1211 1213 +2
+ Misses 1777 1775 -2
Partials 289 289
Continue to review full report at Codecov.
|
|
@ebuchman when can I start adding @Shivani912 as a reviewer? :-) |
tendermint/src/lite/verifier.rs
Outdated
| // these are (signed) headers and validators will be returned by the Requester. | ||
| fn init_requester(vals_for_height: Vec<Vec<usize>>) -> MockRequester { | ||
| fn init_requester(vals_for_height: Vec<Vec<usize>>, final_state: &MockState) -> MockRequester { |
There was a problem hiding this comment.
I think I meant make vals_for_height one height longer here, which would make this fn simpler, and would also decouple the expected final_state from the last thing the requester reports, since they can be different if the requester is faulty.
There was a problem hiding this comment.
The way we currently init the requester would mean, the vector vals_for_height needs to be 2 heights longer (e.g. if we want to bisect to height 5, we need vals up-to 6, and to write those into the requester, we need the next_vals for them, i.e. 7 here). Does that make sense?
* added bisection test for insufficient commits * refactored init_requester, TODO: update tests * fixed tests * Added bisection test for insufficient commits * fixed failures * cargo fmt * Changed tests to use ValsAndCommit struct * satisfy clippy
|
I'll merge in master shortly to resolve merge conflicts. @Shivani912 do you feel comfortable to take over the remaining test related issues? |
…141_followup # Conflicts: # tendermint/src/lite/types.rs # tendermint/src/lite/verifier.rs
|
Actually, I think we should merge #151 first, as it touches upon error handling and then resolve merge conflicts. |
|
resolved all merge conflicts |
There was a problem hiding this comment.
After talking with @liamsi, let's merge this PR as is and other test related issues/code can come up as separate PRs.
ref: #141 (comment) and #132 (comment)
Also added check for increasing time and a few more tests.