Last riff on Mock Context cleanup #310

adizere · 2020-10-14T15:35:48Z

Closes: #297
Also closes cosmos/ibc-rs#111

For contributor use:

Unit tests written
Added test to CI if applicable
Updated CHANGELOG_PENDING.md
Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
Updated relevant documentation (docs/) and code comments
Re-reviewed Files changed in the Github PR explorer

codecov-io · 2020-10-16T11:24:21Z

Codecov Report

Merging #310 into master will increase coverage by 24.7%.
The diff coverage is 64.3%.

@@            Coverage Diff            @@
##           master    informalsystems/hermes#310      +/-   ##
=========================================
+ Coverage    13.6%   38.4%   +24.7%     
=========================================
  Files          69     118      +49     
  Lines        3752    7626    +3874     
  Branches     1374    2715    +1341     
=========================================
+ Hits          513    2930    +2417     
- Misses       2618    4523    +1905     
+ Partials      621     173     -448

Impacted Files	Coverage Δ
modules/src/events.rs	`0.0% <0.0%> (ø)`
modules/src/ics02_client/events.rs	`0.0% <ø> (ø)`
modules/src/ics02_client/raw.rs	`0.0% <0.0%> (ø)`
modules/src/ics03_connection/error.rs	`15.0% <0.0%> (-18.4%)`	⬇️
modules/src/ics04_channel/error.rs	`75.0% <ø> (+50.0%)`	⬆️
modules/src/ics04_channel/packet.rs	`0.0% <0.0%> (ø)`
modules/src/ics07_tendermint/client_def.rs	`0.0% <0.0%> (ø)`
modules/src/ics07_tendermint/error.rs	`75.0% <ø> (+75.0%)`	⬆️
modules/src/ics18_relayer/error.rs	`0.0% <0.0%> (ø)`
modules/src/ics26_routing/error.rs	`75.0% <0.0%> (ø)`
... and 214 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 88c46fc...bbad223. Read the comment docs.

adizere · 2020-10-16T18:46:05Z

modules/src/ics02_client/height.rs

@@ -119,19 +119,3 @@ impl std::fmt::Display for Height {
        )
    }
 }
-


FYI -- These two methods have moved to modules/src/ics24_host/identifier.rs (see below).

ancazamfir

Looks good! I have some comments, some of them we can address in new issues, I will leave it up to you.

modules/src/ics26_routing/handler.rs

modules/src/ics03_connection/handler/conn_open_ack.rs

modules/src/ics03_connection/handler/conn_open_try.rs

modules/src/ics03_connection/handler/verify.rs

modules/src/mock_context.rs

ancazamfir

Wohooo! Looks great! I think we're good! We can do more in other issues.

adizere · 2020-10-20T17:21:15Z

Thanks for the thorough review Anca!

* Cleaning up ICS26 tests with global mock context & better coverage. * ICS26 adapted (ICS18 and general cleanup left todo) * ICS18 context mock cleanup done. * Nuked inter-dependent mock contexts * Housekeeping, last bits. * Nuked Chain* traits and unused types. * Addressing Anca's comments from prior PR * Ressurect is_epoch_format * Simplify MockContext construction. * Added mock context validation. Better constructor for ChainId. * Fixed a few of Anca's comments * Reintroduced max_history_size in MockContext constructor * Missing test case for conn_open_try * Fix host_header arithmetic, added tests for mock context. * Consistent Height construction. Fixed old comments & error messages

Cleaning up ICS26 tests with global mock context & better coverage.

7ea1352

adizere self-assigned this Oct 14, 2020

adizere added 2 commits October 16, 2020 12:54

Merge branch 'master' into adi/297_mock_cleanup

77a2130

ICS26 adapted (ICS18 and general cleanup left todo)

2c6a8ac

adizere added 5 commits October 16, 2020 16:58

ICS18 context mock cleanup done.

b79f1fa

Nuked inter-dependent mock contexts

f92013d

Housekeeping, last bits.

ee391c8

Nuked Chain* traits and unused types.

f3528f6

Addressing Anca's comments from prior PR

4e55262

adizere marked this pull request as ready for review October 16, 2020 15:52

adizere requested review from ancazamfir and romac as code owners October 16, 2020 15:52

Ressurect is_epoch_format

46cb18e

adizere commented Oct 16, 2020

View reviewed changes

Simplify MockContext construction.

4819dd0

ancazamfir reviewed Oct 17, 2020

View reviewed changes

adizere added 6 commits October 19, 2020 19:11

Added mock context validation. Better constructor for ChainId.

8a8c530

Fixed a few of Anca's comments

93327dd

Reintroduced max_history_size in MockContext constructor

bbad223

Missing test case for conn_open_try

d71a1f2

Fix host_header arithmetic, added tests for mock context.

301c7ba

Consistent Height construction. Fixed old comments & error messages

930595c

ancazamfir approved these changes Oct 20, 2020

View reviewed changes

adizere merged commit f0feb0c into master Oct 20, 2020

adizere deleted the adi/297_mock_cleanup branch October 20, 2020 17:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Last riff on Mock Context cleanup #310

Last riff on Mock Context cleanup #310

adizere commented Oct 14, 2020 •

edited

Loading

codecov-io commented Oct 16, 2020 •

edited

Loading

adizere Oct 16, 2020

ancazamfir left a comment

ancazamfir left a comment

adizere commented Oct 20, 2020

@@ @@ -119,19 +119,3 @@ impl std::fmt::Display for Height { @@
                       )
                   }
               }

Last riff on Mock Context cleanup #310

Last riff on Mock Context cleanup #310

Conversation

adizere commented Oct 14, 2020 • edited Loading

codecov-io commented Oct 16, 2020 • edited Loading

Codecov Report

adizere Oct 16, 2020

Choose a reason for hiding this comment

ancazamfir left a comment

Choose a reason for hiding this comment

ancazamfir left a comment

Choose a reason for hiding this comment

adizere commented Oct 20, 2020

adizere commented Oct 14, 2020 •

edited

Loading

codecov-io commented Oct 16, 2020 •

edited

Loading