Skip to content

Conversation

@maflcko
Copy link
Member

@maflcko maflcko commented Mar 25, 2022

Seems a bit overkill to spread tests for the generate* methods over several files. Combining them into a single file has also a nice side-effect of requiring less node (re)starts, which are expensive in valgrind.

Can be reviewed with
--color-moved=dimmed-zebra
@fanquake fanquake added the Tests label Mar 25, 2022
@theStack
Copy link
Contributor

Concept ACK

@brunoerg
Copy link
Contributor

Strong concept ACK

Copy link
Member

@glozow glozow left a comment

Choose a reason for hiding this comment

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

utACK 0000ff0

4 leading zeros, nice.

Comment on lines -29 to -30
# Checking mining to an address without a wallet. Generating to a valid address should succeed
# but generating to an invalid address will fail.
Copy link
Member

Choose a reason for hiding this comment

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

Review note - this comment has been removed. Imo it's fine since all of the generate tests are with wallet disabled, so the comment doesn't add much value.

@maflcko maflcko merged commit 9745e18 into bitcoin:master Mar 25, 2022
@maflcko maflcko deleted the 2203-test-gen-🏻 branch March 25, 2022 15:56
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Apr 2, 2022
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Apr 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants