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: improve common package coverage #1903

Merged
merged 32 commits into from
Mar 22, 2024
Merged

test: improve common package coverage #1903

merged 32 commits into from
Mar 22, 2024

Conversation

skosito
Copy link
Contributor

@skosito skosito commented Mar 15, 2024

Description

note: too many files changed in diff because i moved some files (81 receipts.json files) and added files for transactions data (81 tx.json files)

  • increases codecov for common package from 36.1% to 91.6% (some parts are tricky to test, but should be good enough coverage)
  • adds tx files for ethereum proof tests, same block as receipts
  • moves some data to testdata to avoid duplication, but still there is todo with existing issue to centralize it further
  • remove some unused code
  • simplifies some small parts

Closes: #1833

Type of change

New tests

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Include instructions and any relevant details so others can reproduce.

  • Tested CCTX in localnet
  • Tested in development environment
  • Go unit tests
  • Go integration tests
  • Tested via GitHub Actions

Checklist:

  • I have added unit tests that prove my fix feature works

Copy link

codecov bot commented Mar 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 52.12%. Comparing base (35acb4a) to head (952dc0a).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1903      +/-   ##
===========================================
+ Coverage    48.82%   52.12%   +3.30%     
===========================================
  Files          235      235              
  Lines        13381    13347      -34     
===========================================
+ Hits          6533     6957     +424     
+ Misses        6405     5953     -452     
+ Partials       443      437       -6     
Files Coverage Δ
common/address.go 100.00% <ø> (+31.57%) ⬆️
common/chain.go 96.69% <ø> (+65.24%) ⬆️
common/coin.go 100.00% <100.00%> (+60.00%) ⬆️
common/gas_limits.go 100.00% <100.00%> (+100.00%) ⬆️
common/pubkey.go 82.41% <ø> (+48.78%) ⬆️
common/tss.go 82.60% <ø> (+82.60%) ⬆️

... and 10 files with indirect coverage changes

common/coin.go Show resolved Hide resolved
@skosito skosito changed the title test: common package test: improve common package coverage Mar 18, 2024
@skosito skosito marked this pull request as ready for review March 18, 2024 23:03
Copy link
Member

@lumtis lumtis left a comment

Choose a reason for hiding this comment

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

This looks good to me, great 👍
I think we can also do #1905 for the code structure. But this can be done in another PR

@skosito
Copy link
Contributor Author

skosito commented Mar 20, 2024

This looks good to me, great 👍 I think we can also do #1905 for the code structure. But this can be done in another PR

sure, i can do this structure task once this is merged

common/tss_test.go Outdated Show resolved Hide resolved
Copy link
Contributor

@kingpinXD kingpinXD left a comment

Choose a reason for hiding this comment

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

left minor comments
LGTM

@skosito
Copy link
Contributor Author

skosito commented Mar 22, 2024

left minor comments LGTM

tnx, fixed

@skosito skosito merged commit 3e7fd99 into develop Mar 22, 2024
21 checks passed
@skosito skosito deleted the test-common-package branch March 22, 2024 18:03
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.

Improve code coverage in the common package
3 participants