test: Limit scope of id global which is shared between subtests #24627

pull MarcoFalke wants to merge 1 commits into bitcoin:master from MarcoFalke:2203-gid-🥒 changing 1 files +10 −7

MarcoFalke commented at 3:30 PM on March 21, 2022: member

Globals aren't too nice when testing, as leak state between subtests run in the same process. For example, when checking peer ids in the tests, they might pass/fail depending on other tests run in the same process.

Fix this by making id not a global.

test: Limit scope of id global which is shared between subtests

This is needed to use ASSERT_DEBUG_LOG, which may include a fixed node
number

fa9086d085

MarcoFalke added the label Refactoring on Mar 21, 2022
DrahtBot commented at 7:33 PM on March 21, 2022: member


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



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #24543 (net processing: Move remaining globals into PeerManagerImpl by dergoegge)
- #23352 (test: Extend stale_tip_peer_management test by MarcoFalke)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
promag commented at 9:41 PM on March 21, 2022: member

Code review ACK fa9086d085f664a96561eeb5dd29fc1a4e4f926a.
fanquake merged this on Mar 22, 2022
fanquake closed this on Mar 22, 2022
MarcoFalke deleted the branch on Mar 22, 2022
jnewbery commented at 11:13 AM on March 22, 2022: member

Concept ACK. Globals are evil.
sidhujag referenced this in commit 88039ee535 on Mar 23, 2022
DrahtBot locked this on Mar 22, 2023

Contributors

Labels