test: Fail if InitBlockIndex fails #9904

pull laanwj wants to merge 1 commits into bitcoin:master from laanwj:2017_03_test_check_blkindex_result changing 1 files +2 −2

laanwj commented at 1:55 PM on March 2, 2017: member

If InitBlockIndex fails, then it will segfault later. Same for the later ActivateBestChain. BOOST_REQUIRE the result, so that an error will be reported and the test case aborted.

test: Report InitBlockIndex result

If InitBlockIndex fails, then it will segfault later. Same for the later
ActivateBestChain. BOOST_REQUIRE the result, so that an error will be
reported and the test case aborted.

64854666f5

laanwj added the label Tests on Mar 2, 2017
MarcoFalke commented at 2:05 PM on March 2, 2017: member

utACK 64854666f57d9f94269000f10897858e8c60d99f. Thanks for introducing the first BOOST_REQUIRE, this was overdue.
in src/test/test_bitcoin.cpp:None in 64854666f5
```
  73 | +        BOOST_REQUIRE(InitBlockIndex(chainparams));
  74 |          {
  75 |              CValidationState state;
  76 |              bool ok = ActivateBestChain(state, chainparams);
  77 | -            BOOST_CHECK(ok);
  78 | +            BOOST_REQUIRE(ok);
```
jnewbery commented at 9:56 PM on March 2, 2017:

Why not BOOST_REQUIRE(ActivateBestChain(state, chainparams)); on one line?

MarcoFalke commented at 9:59 PM on March 2, 2017:

smaller diff and we don't want to imply assertions with side effects are our default coding style :P

benma commented at 11:00 PM on March 2, 2017:

Then you should also do the same with the above BOOST_REQUIRE :)

MarcoFalke commented at 11:16 PM on March 2, 2017:

Yeah, it is not consistent, but I don't think it matters too much.

jnewbery commented at 4:50 AM on March 3, 2017:

we don't want to imply assertions with side effects are our default coding style

:thumbsup:

laanwj commented at 6:14 AM on March 3, 2017:

Meh, in the tests it's pretty normal to check something with side effects. This is only an "assertion" in the abstract sense, not the C/C++ sense that it will potentially be disabled (new: tests in release mode, with disabled tests!).

Anyhow let's not nitpick this two-line test change to death.
jnewbery approved
jnewbery commented at 9:57 PM on March 2, 2017: member

Looks good. Tested ACK 64854666f57d9f94269000f10897858e8c60d99f. One nit.
laanwj merged this on Mar 3, 2017
laanwj closed this on Mar 3, 2017
laanwj referenced this in commit 58861ad91b on Mar 3, 2017
PastaPastaPasta referenced this in commit d1e6b421f5 on Dec 29, 2018
PastaPastaPasta referenced this in commit 4e0ed46416 on Dec 31, 2018
PastaPastaPasta referenced this in commit d1a5280f7d on Dec 31, 2018
PastaPastaPasta referenced this in commit a0fb8e5218 on Dec 31, 2018
PastaPastaPasta referenced this in commit f98e93bfa7 on Jan 2, 2019
PastaPastaPasta referenced this in commit d471d719a9 on Jan 3, 2019
PastaPastaPasta referenced this in commit 1ee1c360c1 on Jan 5, 2019
PastaPastaPasta referenced this in commit aa1e362e0a on Jan 7, 2019
PastaPastaPasta referenced this in commit dfdf27798a on Jan 7, 2019
PastaPastaPasta referenced this in commit 682a3b993b on Jan 23, 2019
PastaPastaPasta referenced this in commit 5ed57b6749 on Jan 25, 2019
MarcoFalke locked this on Sep 8, 2021

Contributors

Labels