Skip to content

perf: Streamline leverage CLI tests#987

Merged
toteki merged 7 commits intomainfrom
adam/misc1
May 30, 2022
Merged

perf: Streamline leverage CLI tests#987
toteki merged 7 commits intomainfrom
adam/misc1

Conversation

@toteki
Copy link
Copy Markdown
Contributor

@toteki toteki commented May 28, 2022

Description

While this doesn't eliminate the need to run a network entirely, it does reduce the average duration of leverage CLI tests from about 320s to 35s locally.

Two main reasons the speed has increased:

  • Liquidation transaction working without a governance proposal to adjust collateral weight (saves ~100sec)
  • Test transactions and queries with prerequisites (e.g. can only borrow after lending + setting collateral) grouped into a single scenario, to eliminate setup/cleanup transactions which each cost 1 block time. (saves ~200sec)

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • included the correct type prefix in the PR title
  • added appropriate labels to the PR
  • added ! to the type prefix if API or client breaking change
  • targeted the correct branch (see PR Targeting)
  • provided a link to the relevant issue or specification
  • added a changelog entry to CHANGELOG.md
  • included comments for documenting Go code
  • updated the relevant documentation or specification
  • reviewed "Files changed" and left comments if necessary
  • confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed ! in the type prefix if API or client breaking change
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic
  • reviewed API design and naming
  • reviewed documentation is accurate
  • reviewed tests and test coverage
  • manually tested (if applicable)

@toteki toteki marked this pull request as ready for review May 28, 2022 21:44
@toteki toteki requested review from a team as code owners May 28, 2022 21:44
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

Merging #987 (0a3c3e1) into main (d30b0f8) will decrease coverage by 6.99%.
The diff coverage is 95.39%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #987      +/-   ##
==========================================
- Coverage   50.95%   43.96%   -7.00%     
==========================================
  Files          64       64              
  Lines        9490     8332    -1158     
==========================================
- Hits         4836     3663    -1173     
- Misses       4401     4414      +13     
- Partials      253      255       +2     
Impacted Files Coverage Δ
x/leverage/client/cli/tx.go 0.00% <0.00%> (ø)
x/leverage/client/tests/suite.go 69.41% <58.33%> (-30.59%) ⬇️
x/leverage/client/tests/tests.go 100.00% <100.00%> (ø)

@toteki toteki changed the title test: Streamline leverage CLI tests perf: Streamline leverage CLI tests May 28, 2022
Copy link
Copy Markdown
Contributor

@RafilxTenfen RafilxTenfen left a comment

Choose a reason for hiding this comment

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

The only thing is that codecov is still counting the files x/leverage/client/tests/suite.go and x/leverage/client/tests/tests.go

I tried to change the name from .codecov.yml to codecov.yml in #989 but it didn't work :/

@toteki
Copy link
Copy Markdown
Contributor Author

toteki commented May 30, 2022

I also tried renaming the two files to end with _test.go but the suite consistently errors in that case

    suite_test.go:36: 
        	Error Trace:	suite_test.go:36
        	            				suite.go:118
        	            				cli_test.go:45
        	Error:      	Received unexpected error:
        	            	timeout exceeded waiting for block
        	Test:       	TestIntegrationTestSuite

So for now, just leaving the coverage situation as it was.

@toteki toteki merged commit 2042f6d into main May 30, 2022
@toteki toteki deleted the adam/misc1 branch May 30, 2022 13:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants