test: (refactor) Use datadir from options in chainstatemanager test #27876

pull TheCharlatan wants to merge 1 commits into bitcoin:master from TheCharlatan:chainmanTestDir changing 1 files +7 −7

TheCharlatan commented at 1:22 PM on June 13, 2023: contributor

This should make the test less reliant on argument state from the test setup. This is a follow-up PR as requested in #27576 (review).

scripted-diff: Use datadir from options in chainstatemanager test

This should make the test less reliant on details of the test setup

-BEGIN VERIFY SCRIPT-
sed -i 's/m_args.GetDataDirNet()/chainman.m_options.datadir/g' src/test/validation_chainstatemanager_tests.cpp
-END VERIFY SCRIPT-

d54819d74e

DrahtBot commented at 1:22 PM on June 13, 2023: contributor



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



Reviews

See the guideline for information on the review process.

Type Reviewers

ACK MarcoFalke, kevkevinpal, ryanofsky, achow101

Concept ACK hebasto

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
DrahtBot renamed this:
~~refactor(test): Use datadir from options in chainstatemanager test~~
test: (refactor) Use datadir from options in chainstatemanager test
on Jun 13, 2023
DrahtBot added the label Tests on Jun 13, 2023

Type	Reviewers
ACK	MarcoFalke, kevkevinpal, ryanofsky, achow101
Concept ACK	hebasto

in src/test/validation_chainstatemanager_tests.cpp:385 in d54819d74e

 381 | @@ -382,7 +382,7 @@ struct SnapshotTestSetup : TestChain100Setup {
 382 |              m_node.notifications = std::make_unique<KernelNotifications>();
 383 |              const ChainstateManager::Options chainman_opts{
 384 |                  .chainparams = ::Params(),
 385 | -                .datadir = m_args.GetDataDirNet(),
 386 | +                .datadir = chainman.m_options.datadir,

maflcko commented at 1:34 PM on June 13, 2023:

review note, this creates a copy and is not a reference, thus memory-safe

maflcko approved
maflcko commented at 1:35 PM on June 13, 2023: member

lgtm ACK d54819d74e04e6105c1f0362755f5bcfa845eefd
hebasto commented at 2:38 PM on June 13, 2023: member

Concept ACK.
kevkevinpal approved
kevkevinpal commented at 6:01 PM on June 13, 2023: contributor

ACK https://github.com/bitcoin/bitcoin/commit/d54819d74e04e6105c1f0362755f5bcfa845eefd
ryanofsky approved
ryanofsky commented at 7:31 PM on June 13, 2023: contributor

Code review ACK d54819d74e04e6105c1f0362755f5bcfa845eefd
achow101 commented at 8:26 PM on June 13, 2023: member

ACK d54819d74e04e6105c1f0362755f5bcfa845eefd
achow101 merged this on Jun 13, 2023
achow101 closed this on Jun 13, 2023
bitcoin deleted a comment on Jun 14, 2023
sidhujag referenced this in commit 0cdc15f1aa on Jun 15, 2023
bitcoin locked this on Jun 13, 2024

Contributors

Labels

Linked (view graph)

#27576 kernel: Remove args, settings, chainparams, chainparamsbase from kernel library

test: (refactor) Use datadir from options in chainstatemanager test #27876

Reviews