test: Avoid connecting to real network when running tests #21254

pull MarcoFalke wants to merge 2 commits into bitcoin:master from MarcoFalke:2102-testNoTelemetry changing 2 files +30 −22

MarcoFalke commented at 9:42 AM on February 21, 2021: member

Introduced in #19884
fanquake added the label Tests on Feb 21, 2021

test: Assume node is running in subtests

Every (sub)test in the framework assumes the node is running, except for
the (sub)tests in this file. Remove that confusion by stopping the node
at the start of every subtest, instead of at the end.

fa1b713941

test: Avoid connecting to real network when running tests
Can be reviewed with --word-diff-regex=.
fa730e9157
MarcoFalke force-pushed on Feb 21, 2021
DrahtBot commented at 11:05 AM on February 21, 2021: 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:
- #20018 (p2p: ProcessAddrFetch(-seednode) is unnecessary if -connect is specified by dhruv)
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.
Sjors commented at 4:32 PM on February 23, 2021: member

ACK fa730e915766b012ff145e3c2af782668defb21f

Can you add a note which test was connecting to the outside world? I assume it's one of the few tests that uses mainnet?
MarcoFalke commented at 4:38 PM on February 23, 2021: member

I haven't checked if any of the current tests reach out right now, but any non-regtest test might be affected.
practicalswift commented at 5:12 PM on February 24, 2021: contributor

Concept ACK

Good catch! Accidental telemetry is still telemetry and should be removed.
MarcoFalke merged this on Feb 25, 2021
MarcoFalke closed this on Feb 25, 2021
MarcoFalke deleted the branch on Feb 25, 2021
sidhujag referenced this in commit bcf12e4943 on Feb 25, 2021
practicalswift commented at 4:38 PM on February 25, 2021: contributor

To detect accidental introduction of telemetry in the future: could we perhaps guard against external connections in tests via say a seccomp restricted job in Travis? Perhaps nsjail or firejail could be used.
MarcoFalke commented at 5:49 PM on February 25, 2021: member

Sure, pull request welcome
luke-jr referenced this in commit ec7c32bff0 on Jun 27, 2021
Fabcien referenced this in commit a097462a72 on Jan 27, 2022
DrahtBot locked this on Aug 16, 2022

Contributors

Labels

Linked (view graph)

#19884 p2p: No delay in adding fixed seeds if -dnsseed=0 and peers.dat is empty

test: Avoid connecting to real network when running tests #21254

Conflicts