test: Split overly large util_tests.cpp file #26489

pull maflcko wants to merge 1 commits into bitcoin:master from maflcko:2211-test-split-🌐 changing 3 files +1044 −1021

maflcko commented at 11:02 AM on November 11, 2022: member

The file has ~3kLOC and is slow to compile.

Fix both issues by splitting it. (On my machine the compilation goes from 25 seconds previously to 17+10 seconds for the two smaller files)

To review, --color-moved=dimmed-zebra can be used.
DrahtBot added the label Tests on Nov 11, 2022
DrahtBot commented at 11:37 PM on November 11, 2022: contributor


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



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #26213 (univalue: Remove confusing getBool/isTrue/isFalse by MarcoFalke)
- #26028 (More verbose warning for multiple network argument error. by amovfx)
- #17783 (util: Fix -norpcwhitelist, -norpcallowip, and similar corner case behavior by ryanofsky)
- #17581 (refactor: Remove settings merge reverse precedence code by ryanofsky)
- #17580 (refactor: Add ALLOW_LIST flags and enforce usage in CheckArgFlags by ryanofsky)
- #17493 (util: Forbid ambiguous multiple assignments in config file by ryanofsky)
- #16545 (refactor: Implement missing error checking for ArgsManager flags by ryanofsky)
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.
in src/test/argsman_tests.cpp:17 in fae2420f16 outdated
```
  12 | +#include <univalue.h>
  13 | +
  14 | +#include <array>
  15 | +#include <optional>
  16 | +#include <cstdint>
  17 | +#include <cstring>
```
shaavan commented at 8:05 AM on November 12, 2022:

nit: These dependencies can be removed
maflcko commented at 8:14 AM on November 12, 2022:

No, they are needed, see https://en.cppreference.com/w/cpp/types/integer

shaavan commented at 1:34 PM on November 14, 2022:

Okay, I verified that two dependencies are indeed needed for the file.

maflcko commented at 1:37 PM on November 14, 2022:

I hope we can use iwyu one day on the tests :sweat_smile:

shaavan commented at 1:40 PM on November 14, 2022:

Thanks a lot for the name. I was searching for this tool, but I forgot its name. I will remember to use it from now on. 😄
shaavan commented at 8:11 AM on November 12, 2022: contributor
Concept ACK

I successfully compiled the branch and ran the test on it.

I think some dependencies would be redundant and can be removed.

in src/test/util_tests.cpp
```
#include <test/util/logging.h>
#include <test/util/str.h>
```
and, in src/test/argsman_tests.cpp
test: Split overly large util_tests.cpp file fa4ec1be51
maflcko force-pushed on Nov 14, 2022
shaavan approved
shaavan commented at 1:37 PM on November 14, 2022: contributor
ACK fa4ec1be513c80d6db644f1e3170e980035e7306
- The redundant dependencies are removed from the src/test/util_tests.cpp file.

in src/test/argsman_tests.cpp:5 in fa4ec1be51

   0 | @@ -0,0 +1,1043 @@
   1 | +// Copyright (c) 2011-2022 The Bitcoin Core developers
   2 | +// Distributed under the MIT software license, see the accompanying
   3 | +// file COPYING or http://www.opensource.org/licenses/mit-license.php.
   4 | +
   5 | +#include <util/system.h>

aureleoules commented at 2:42 PM on November 15, 2022:

nit

diff --git a/src/test/argsman_tests.cpp b/src/test/argsman_tests.cpp
index d00876bc7..76d64e0d5 100644
--- a/src/test/argsman_tests.cpp
+++ b/src/test/argsman_tests.cpp
@@ -2,13 +2,13 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
-#include <util/system.h>
 #include <fs.h>
 #include <sync.h>
 #include <test/util/logging.h>
 #include <test/util/setup_common.h>
 #include <test/util/str.h>
 #include <util/strencodings.h>
+#include <util/system.h>
 #include <univalue.h>
 
 #include <array>

aureleoules approved
aureleoules commented at 2:43 PM on November 15, 2022: member

ACK fa4ec1be513c80d6db644f1e3170e980035e7306
RandyMcMillan commented at 4:25 PM on November 15, 2022: contributor

ACK fa4ec1be513c80d6db644f1e3170e980035e7306 for:

macOS x84_64: Darwin ₿ 19.6.0 Darwin Kernel Version 19.6.0: Tue Jun 21 21:18:39 PDT 2022; root:xnu-6153.141.66~1/RELEASE_X86_64 x86_64

macOS Arm64: Darwin DeepSpaceM1.local 21.6.0 Darwin Kernel Version 21.6.0: Thu Sep 29 20:13:46 PDT 2022; root:xnu-8020.240.7~1/RELEASE_ARM64_T8101 arm64
maflcko merged this on Nov 15, 2022
maflcko closed this on Nov 15, 2022
maflcko deleted the branch on Nov 15, 2022
sidhujag referenced this in commit 068335a15c on Nov 16, 2022
bitcoin locked this on Nov 15, 2023

Contributors

Labels

test: Split overly large util_tests.cpp file #26489

Conflicts