Skip to content

Conversation

@maflcko
Copy link
Member

@maflcko maflcko commented Jun 26, 2019

This prevents scope-leak of symbols that are supposed to be local to one test case.

@DrahtBot DrahtBot added the Tests label Jun 26, 2019
@maflcko maflcko force-pushed the 1906-testNoScopeLeak branch from fab7c68 to 1111451 Compare June 26, 2019 18:08
@practicalswift
Copy link
Contributor

Concept ACK

@gwillen
Copy link
Contributor

gwillen commented Jun 27, 2019

This seems like a great direction to go! There's probably more we can do in terms of managing state that's shared between test cases, but just explicitly spelling out which state is shared with self.foo, and unsharing other state, is going to remove 90% of the footgun potential right off the top.

@DrahtBot
Copy link
Contributor

DrahtBot commented Jun 28, 2019

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

No conflicts as of last run.

Copy link
Contributor

@promag promag left a comment

Choose a reason for hiding this comment

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

ACK 1111451d1, could squash last commit.

Copy link
Contributor

Choose a reason for hiding this comment

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

nit, could nuke these "boxes".

@maflcko maflcko force-pushed the 1906-testNoScopeLeak branch from 1111451 to fa11d69 Compare June 28, 2019 15:04
@jonatack
Copy link
Member

Concept ACK, am updating #15687 to this as well after your review comment. Perhaps document where tests are order-dependent, if any. Will review.

@maflcko maflcko force-pushed the 1906-testNoScopeLeak branch from fa11d69 to faf8318 Compare July 2, 2019 21:19
@maflcko
Copy link
Member Author

maflcko commented Jul 2, 2019

Rebased due to conflict in adjacent line

@maflcko maflcko merged commit faf8318 into bitcoin:master Jul 31, 2019
maflcko pushed a commit that referenced this pull request Jul 31, 2019
faf8318 test: Split fundrawtx test into subtests (MarcoFalke)
fa6fba3 test: Make local symbols in run_test members (MarcoFalke)

Pull request description:

  This prevents scope-leak of symbols that are supposed to be local to one test case.

Top commit has no ACKs.

Tree-SHA512: 9b2a4ca2cdd631ef915d2f7e6cd62375df9a0919448350aa6e5ae4aa8a8fe3ba53870f7a9a25a57736894b4e3a45e861018253ed2d57d9a64c2bb65fa270fad8
@maflcko maflcko deleted the 1906-testNoScopeLeak branch July 31, 2019 22:06
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Aug 1, 2019
faf8318 test: Split fundrawtx test into subtests (MarcoFalke)
fa6fba3 test: Make local symbols in run_test members (MarcoFalke)

Pull request description:

  This prevents scope-leak of symbols that are supposed to be local to one test case.

Top commit has no ACKs.

Tree-SHA512: 9b2a4ca2cdd631ef915d2f7e6cd62375df9a0919448350aa6e5ae4aa8a8fe3ba53870f7a9a25a57736894b4e3a45e861018253ed2d57d9a64c2bb65fa270fad8
jasonbcox pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Sep 30, 2020
Summary:
faf8318c55a6001270a6fc8ed2298767099bafba test: Split fundrawtx test into subtests (MarcoFalke)
fa6fba3bc8013d7f813edd71f152d86eab907e4d test: Make local symbols in run_test members (MarcoFalke)

Pull request description:

  This prevents scope-leak of symbols that are supposed to be local to one test case.

Top commit has no ACKs.

Backport of Core [[bitcoin/bitcoin#16293 | PR16293]]

Test Plan: `test_runner.py rpc_fundrawtransaction`

Reviewers: #bitcoin_abc, majcosta

Reviewed By: #bitcoin_abc, majcosta

Subscribers: majcosta

Differential Revision: https://reviews.bitcoinabc.org/D7688
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
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.

6 participants