test: Move MakeNoLogFileContext to libtest

MarcoFalke commented at 10:43 am on January 25, 2021: member

To avoid verbose code duplication, which may lead to accidental mishaps https://github.com/bitcoin/bitcoin/pull/20998/files#r563624041.

Also fix a nit I found in #20946 (review).

fanquake added the label Tests on Jan 25, 2021

MarcoFalke renamed this:
~~test: Move MakeNoLogFileContext to common libtest_util, and use it in bench~~
test: Move MakeNoLogFileContext to libtest_util, and use it in bench
on Jan 25, 2021

MarcoFalke closed this on Jan 25, 2021

MarcoFalke deleted the branch on Jan 25, 2021

MarcoFalke restored the branch on Jan 25, 2021

MarcoFalke commented at 7:17 pm on January 25, 2021: member

Worked around #21005

MarcoFalke reopened this on Jan 25, 2021

MarcoFalke force-pushed on Jan 25, 2021

DrahtBot commented at 2:15 am on January 26, 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:

#20773 (refactor: split CWallet::Create by S3RK)

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.

MarcoFalke force-pushed on Feb 11, 2021

DrahtBot added the label Needs rebase on Feb 11, 2021

MarcoFalke force-pushed on Feb 11, 2021

DrahtBot removed the label Needs rebase on Feb 11, 2021

DrahtBot added the label Needs rebase on Feb 11, 2021

MarcoFalke force-pushed on Feb 11, 2021

MarcoFalke commented at 2:06 pm on February 11, 2021: member

Rebased (3x)

DrahtBot removed the label Needs rebase on Feb 11, 2021

DrahtBot added the label Needs rebase on Feb 16, 2021

MarcoFalke force-pushed on Feb 17, 2021

fanquake commented at 10:24 am on February 17, 2021: member

concept ack (1x)

DrahtBot removed the label Needs rebase on Feb 17, 2021

DrahtBot added the label Needs rebase on Feb 19, 2021

MarcoFalke force-pushed on Feb 19, 2021

MarcoFalke commented at 11:12 am on February 19, 2021: member

Rebased (4x)

DrahtBot removed the label Needs rebase on Feb 19, 2021

DrahtBot added the label Needs rebase on Feb 20, 2021

MarcoFalke force-pushed on Feb 22, 2021

MarcoFalke commented at 9:25 am on February 22, 2021: member

Rebased (5x)

DrahtBot removed the label Needs rebase on Feb 22, 2021

MarcoFalke referenced this in commit cac10e66d2 on Feb 25, 2021

MarcoFalke force-pushed on Feb 25, 2021

sidhujag referenced this in commit ea890fd13a on Feb 25, 2021

practicalswift commented at 4:03 pm on February 28, 2021: contributor

Concept ACK (2x)

DrahtBot added the label Needs rebase on Mar 1, 2021

MarcoFalke force-pushed on Mar 3, 2021

Move MakeNoLogFileContext to common libtest_util, and use it in bench

Can be reviewed with --color-moved=dimmed-zebra

fa576b4532

MarcoFalke force-pushed on Mar 3, 2021

DrahtBot removed the label Needs rebase on Mar 3, 2021

in src/bench/rpc_blockchain.cpp:18 in fa576b4532

14@@ -15,7 +15,7 @@
15 namespace {
16 
17 struct TestBlockAndIndex {
18-    TestingSetup test_setup{};
19+    const std::unique_ptr<const TestingSetup> testing_setup{MakeNoLogFileContext<const TestingSetup>(CBaseChainParams::MAIN)};

fanquake commented at 10:02 am on March 3, 2021:

This seems kind of unruly.

MarcoFalke commented at 11:52 am on March 3, 2021:

unruly

C++ requires this. (You can’t use auto for non-static members)

fanquake approved

fanquake commented at 10:05 am on March 3, 2021: member

ACK fa576b4532814b4bca1936d170cabd01fbc51960

MarcoFalke commented at 11:52 am on March 3, 2021: member

@dongcarl Mind to take a look here, as this builds on top of #20946?

dongcarl commented at 7:23 pm on March 3, 2021: member

Light Code-Review ACK fa576b4532814b4bca1936d170cabd01fbc51960

Looks reasonable. Always good to coalesce these init codepaths.

fanquake commented at 0:50 am on March 4, 2021: member

Failed merged (1x)

This need fixing up post #19203 & #19288:

0test/fuzz/socks5.cpp:22:66: error: unknown type name 'BasicTestingSetup'
1    static const auto testing_setup = MakeNoLogFileContext<const BasicTestingSetup>();
2                                                                 ^
31 error generated.
4gmake[2]: *** [Makefile:13693: test/fuzz/fuzz-socks5.o] Error 1
5gmake[2]: *** Waiting for unfinished jobs....
6test/fuzz/torcontrol.cpp:38:39: error: use of undeclared identifier 'MakeNoLogFileContext'
7    static const auto testing_setup = MakeNoLogFileContext<>();
8                                      ^
91 error generated.

 0diff --git a/src/test/fuzz/socks5.cpp b/src/test/fuzz/socks5.cpp
 1index 123ee042e..72c3e8f71 100644
 2--- a/src/test/fuzz/socks5.cpp
 3+++ b/src/test/fuzz/socks5.cpp
 4@@ -6,6 +6,7 @@
 5 #include <test/fuzz/FuzzedDataProvider.h>
 6 #include <test/fuzz/fuzz.h>
 7 #include <test/fuzz/util.h>
 8+#include <test/util/setup_common.h>
 9 
10 #include <cstdint>
11 #include <string>
12diff --git a/src/test/fuzz/torcontrol.cpp b/src/test/fuzz/torcontrol.cpp
13index b7a42ea7f..a8e69dee3 100644
14--- a/src/test/fuzz/torcontrol.cpp
15+++ b/src/test/fuzz/torcontrol.cpp
16@@ -5,6 +5,7 @@
17 #include <test/fuzz/FuzzedDataProvider.h>
18 #include <test/fuzz/fuzz.h>
19 #include <test/fuzz/util.h>
20+#include <test/util/setup_common.h>
21 #include <torcontrol.h>
22 
23 #include <cstdint>

MarcoFalke commented at 7:54 am on March 4, 2021: member

Not going to invalidate the precious reviews over an unrelated issue. Fixed in #21358, which can go in first.

fanquake merged this on Mar 4, 2021

fanquake closed this on Mar 4, 2021

MarcoFalke deleted the branch on Mar 4, 2021

sidhujag referenced this in commit 0ac2c0e46a on Mar 4, 2021

DrahtBot locked this on Aug 16, 2022

test: Move MakeNoLogFileContext to libtest_util, and use it in bench #21003

Conflicts