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

Increase test coverage for chain #629

Merged
merged 8 commits into from
Sep 22, 2021

Conversation

pinheadmz
Copy link
Member

@pinheadmz pinheadmz commented Aug 27, 2021

@coveralls
Copy link

coveralls commented Aug 27, 2021

Pull Request Test Coverage Report for Build 1259118998

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 17 unchanged lines in 8 files lost coverage.
  • Overall coverage increased (+0.2%) to 62.054%

Files with Coverage Reduction New Missed Lines %
lib/node/fullnode.js 1 69.83%
lib/node/http.js 1 55.01%
lib/node/rpc.js 1 30.03%
lib/primitives/tx.js 2 69.05%
lib/script/script.js 2 63.44%
lib/wallet/nodeclient.js 2 67.57%
lib/wallet/walletdb.js 2 79.02%
lib/mempool/mempool.js 6 62.82%
Totals Coverage Status
Change from base Build 1230772309: 0.2%
Covered Lines: 21074
Relevant Lines: 31726

💛 - Coveralls

@pinheadmz
Copy link
Member Author

Added test for checkpoints. What we do is generate a chain with a variety of transactions (auctions, name claims, undependable outputs, etc) then add each block into a fresh chain object. The second time we do this, the fresh chain has a checkpoint just below the tip. Before rebasing on master branch which includes the checkpoints fix (#597) the test fails with this output:

  1) Checkpoints
       Sync with checkpoints
         should sync chain:

      Error: Sync failure at height 204 (register reserved): Database inconsistency.

      throw new Error(
            ^

      at Context.<anonymous> (/Users/matthewzipkin/Desktop/work/hsd/test/chain-checkpoints-test.js:246:17)

Note that the failure is labeled! During the initial chain-generation process, we also build an array of strings that label every single block! If sync fails, the label for the failed block is looked up and printed in the error message ;-)

@pinheadmz pinheadmz marked this pull request as ready for review September 6, 2021 20:54
@nodech
Copy link
Contributor

nodech commented Sep 21, 2021

I noticed CSV tests are duplicated in test/node-test.js as well, I think all related tests for nSequence and nLocktime could be moved out from both node-test and chain-test.js into chain-cltv-csv-test.js or chain-locktime-test.js (even chain-sequence-locktime-test.js?)

@nodech
Copy link
Contributor

nodech commented Sep 21, 2021

  • Sigop comments
  • More CSV test refactor

Other than these two, everything LGTM !
Great tests !!! 👏

test/chain-test.js Outdated Show resolved Hide resolved
@pinheadmz
Copy link
Member Author

@nodech I cleaned up the nits you found and refactored the tests so all RPC tests are in the node-rpc-test file, leaving node-test empty and deleted it!

I also moved the CSV tests to a new file... for the old file the tests that check chain height and wallet balance. Well, you know those broke and you can probably guess how I fixed them 😬

@pinheadmz pinheadmz merged commit 4c43c22 into handshake-org:master Sep 22, 2021
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.

3 participants