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

test(common): add docs for testutil and increase test coverage #1527

Merged
merged 49 commits into from
Aug 3, 2023

Conversation

Unique-Divine
Copy link
Member

@Unique-Divine Unique-Divine commented Jul 27, 2023

In x/common

  • test(common): document and test BankersRound
  • test(common): address utils test- docs(testutil/cli): documentation for the Network and Validator structs
  • refactor(testutil): Organize network files

In other modules

commit dc5559c
Author: Unique-Divine <realuniquedivine@gmail.com>
Date:   Tue Jul 25 20:46:27 2023 -0500

    test(inflation): run the suite

commit d526b2a
Author: Unique-Divine <realuniquedivine@gmail.com>
Date:   Tue Jul 25 20:36:36 2023 -0500

    linter

commit 062dbfc
Merge: 392d080 e793851
Author: Unique-Divine <realuniquedivine@gmail.com>
Date:   Tue Jul 25 20:25:22 2023 -0500

    Merge branch 'realu/test-inflation' into realu/test-inflation-2

commit e793851
Author: Unique-Divine <realuniquedivine@gmail.com>
Date:   Tue Jul 25 20:10:32 2023 -0500

    test(inflation): test getters in params.go

commit 392d080
Author: Unique-Divine <realuniquedivine@gmail.com>
Date:   Tue Jul 25 19:21:07 2023 -0500

    test(inflation): param set pairs

commit 2aa65ed
Author: Unique-Divine <realuniquedivine@gmail.com>
Date:   Tue Jul 25 19:01:00 2023 -0500

    refactor(simapp): simapp fns are not used outside the simapp dir or tests

commit 68c47ad
Author: Unique-Divine <realuniquedivine@gmail.com>
Date:   Tue Jul 25 13:22:01 2023 -0500

    refactor: remove unused fn

commit 8a01ec1
Author: Unique-Divine <realuniquedivine@gmail.com>
Date:   Tue Jul 25 12:37:34 2023 -0500

    small refactor for better var names

commit d263ffb
Author: Unique-Divine <realuniquedivine@gmail.com>
Date:   Tue Jul 25 12:07:56 2023 -0500

    changelog

commit aa92edd
Author: Unique-Divine <realuniquedivine@gmail.com>
Date:   Tue Jul 25 12:06:51 2023 -0500

    deps: remove usage of cosmossdk.io/depinject since it doesn't work outside the cosmos-sdk

commit b774c48
Author: Unique-Divine <realuniquedivine@gmail.com>
Date:   Tue Jul 25 12:05:47 2023 -0500

    refactor: make it possible to use the encoding config without the app

commit 2e7192a
Author: Unique-Divine <realuniquedivine@gmail.com>
Date:   Tue Jul 25 10:54:18 2023 -0500

    linter

commit 773dee4
Merge: efac0d8 0c571c6
Author: Unique-Divine <realuniquedivine@gmail.com>
Date:   Tue Jul 25 10:53:20 2023 -0500

    Merge branch 'master' into realu/test-inflation

commit efac0d8
Author: Unique-Divine <realuniquedivine@gmail.com>
Date:   Tue Jul 25 10:52:20 2023 -0500

    feat(wasm): no op handler + tests with updated shifter.wasm

commit 2ed7490
Author: Unique-Divine <realuniquedivine@gmail.com>
Date:   Mon Jul 24 15:45:12 2023 -0500

    test(inflation): types coverage
@codecov
Copy link

codecov bot commented Jul 27, 2023

Codecov Report

Merging #1527 (19a33bd) into master (b1477cd) will increase coverage by 2.49%.
Report is 3 commits behind head on master.
The diff coverage is 96.00%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1527      +/-   ##
==========================================
+ Coverage   67.44%   69.94%   +2.49%     
==========================================
  Files         162      163       +1     
  Lines       13967    13957      -10     
==========================================
+ Hits         9420     9762     +342     
+ Misses       3927     3554     -373     
- Partials      620      641      +21     
Files Changed Coverage Δ
x/common/dec.go 74.13% <75.00%> (+48.21%) ⬆️
x/common/testutil/cli/network.go 74.05% <97.05%> (+14.27%) ⬆️
x/common/testutil/cli/tx.go 66.34% <97.22%> (+66.34%) ⬆️
x/common/asset/pair.go 80.45% <100.00%> (+34.30%) ⬆️
x/common/testutil/cli/network_config.go 100.00% <100.00%> (ø)
x/common/testutil/cli/util.go 72.16% <100.00%> (+15.78%) ⬆️
x/perp/v2/types/amm.go 87.78% <100.00%> (ø)
x/spot/client/testutil/suite.go 97.55% <100.00%> (ø)

... and 6 files with indirect coverage changes

@Unique-Divine Unique-Divine changed the title test(common): docs and tests test(common): add docs for testutil and increase test coverage Jul 27, 2023
@Unique-Divine Unique-Divine marked this pull request as ready for review August 2, 2023 21:16
@Unique-Divine Unique-Divine requested a review from a team as a code owner August 2, 2023 21:16
@@ -149,49 +149,19 @@ func (pairKeyEncoder) Decode(b []byte) (int, Pair) {
return i, MustNewPair(s)
}

//-----------------------------------------------------------------------------
Copy link
Member Author

@Unique-Divine Unique-Divine Aug 2, 2023

Choose a reason for hiding this comment

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

Searched with the LSP and and found that this type alias was largely unused, so I removed some of the dead code.

@@ -10,7 +10,7 @@ import (

Copy link
Member Author

Choose a reason for hiding this comment

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

Made the code more descriptive here and added tests for it. Since the Sqrt is used in x/perp, it's important to make sure these functions don't panic.

@@ -0,0 +1,16 @@
package cli
Copy link
Member Author

Choose a reason for hiding this comment

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

Previously, the Network, Config (of the network), and Logger interface were all defined in one file, which made things hard to follow. I added documentation for parts that weren't obvious, separated these structs to their own files, added tests, and deleted code that was unused in integration tests.


// AbsorbListenAddresses ensures that the listening addresses for an active
// node are set on the network config.
func (cfg *Config) AbsorbListenAddresses(val *Validator) {
Copy link
Member Author

@Unique-Divine Unique-Divine Aug 2, 2023

Choose a reason for hiding this comment

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

The listen addresses for RPC, GRPC, and API should be set on the Config but weren't by default. This made the API unintuitive when trying to grab the raw grpc.Conn objects or rpc.RPCClient.

@matthiasmatt matthiasmatt merged commit ee53dae into master Aug 3, 2023
22 checks passed
@matthiasmatt matthiasmatt deleted the realu/test-determinism branch August 3, 2023 20:02
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.

bug(amm.go): should we check over flow on the product of quote and base?
2 participants