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

Added test for client.GetChainTxStatsAsync() in rpcclient. #2049

Conversation

ClaytonNorthey92
Copy link
Contributor

Added test for client.GetChainTxStatsAsync() in rpcclient. This sets up a test websocket server to run the tests. Also, ensure these are run within a timeout, since they rely on concurrency.

Similar to this other pull request, these changes increase the code coverage for rpcclient tests to 5.3%

go-acc ./rpcclient/
ok  	github.com/btcsuite/btcd/rpcclient	0.255s	coverage: 5.3% of statements in ./rpcclient/

Copy link
Member

@jcvernaleo jcvernaleo left a comment

Choose a reason for hiding this comment

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

Love the addition of a test but looks like the actions are failing.

Could you fix those please?

@ClaytonNorthey92 ClaytonNorthey92 force-pushed the rpcclient-GetChainTxStatsAsync-tests branch from 9da4049 to 62f4a20 Compare November 1, 2023 21:15
@ClaytonNorthey92
Copy link
Contributor Author

Hey @jcvernaleo , my apologies, I had forgotten to add the websocket module to btcutil/go.sum, resulting in errors from btcutil in the pipeline.

errors found here and here both result from that not being found in the go.sum file.

I have run both make unit-race and make unit-cover locally with this fix, they both work. I think this is good to re-run the pipelines 🤞

@coveralls
Copy link

Pull Request Test Coverage Report for Build 6725208065

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 10 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.6%) to 55.933%

Files with Coverage Reduction New Missed Lines %
connmgr/connmanager.go 1 86.27%
peer/peer.go 9 73.49%
Totals Coverage Status
Change from base Build 6724971003: 0.6%
Covered Lines: 27254
Relevant Lines: 48726

💛 - Coveralls

Copy link
Member

@jcvernaleo jcvernaleo left a comment

Choose a reason for hiding this comment

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

OK

Code looks good but there is a conflict after the previous PR went in. If you could fix conflict, I'll approve and merge.

Sorry about that!

@ClaytonNorthey92 ClaytonNorthey92 force-pushed the rpcclient-GetChainTxStatsAsync-tests branch from 62f4a20 to ae1dde5 Compare November 2, 2023 19:48
…up a test websocket server to run the tests. Also, ensure these are run within a timeout, since they rely on concurrency
@ClaytonNorthey92 ClaytonNorthey92 force-pushed the rpcclient-GetChainTxStatsAsync-tests branch from ae1dde5 to 9dde0b7 Compare November 2, 2023 19:58
@ClaytonNorthey92
Copy link
Contributor Author

Hey @jcvernaleo , I just rebased and pushed, everything should be up-to-date now 👍 thanks!

Copy link
Member

@jcvernaleo jcvernaleo left a comment

Choose a reason for hiding this comment

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

OK

@jcvernaleo jcvernaleo merged commit d988b86 into btcsuite:master Nov 2, 2023
3 checks passed
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