test: Drop no longer needed workarounds #30847

pull hebasto wants to merge 1 commits into bitcoin:master from hebasto:240908-tests changing 3 files +1 −16

hebasto commented at 8:14 AM on September 8, 2024: member

This PR deletes the workarounds introduced in #16564 and #15382, as ctest skips these cases gracefully: https://github.com/bitcoin/bitcoin/blob/5c80192ff6b982ee3a75be4142fe942b8206f2cd/src/test/CMakeLists.txt#L201-L203

test: Drop no longer needed workarounds

`ctest` skips "no test cases matching filter" tests gracefully.

5c80192ff6

hebasto added the label Tests on Sep 8, 2024
DrahtBot commented at 8:14 AM on September 8, 2024: contributor



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



Code Coverage

For detailed information about the code coverage, see the test coverage report.



Reviews

See the guideline for information on the review process.

Type Reviewers

ACK fanquake, kevkevinpal

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
DrahtBot added the label CI failed on Sep 8, 2024
fanquake approved

Type	Reviewers
ACK	fanquake, kevkevinpal

fanquake commented at 3:44 PM on September 10, 2024: member

ACK 5c80192ff6b982ee3a75be4142fe942b8206f2cd. Looks correct:

cmake -B build -DENABLE_EXTERNAL_SIGNER=OFF (and commented out `EVENT_SET_MEM_FUNCTIONS_IMPLEMENTED` tests)
cmake --build build
<snip>
        Start  74: raii_event_tests
 63/133 Test  [#74](/bitcoin-bitcoin/74/): raii_event_tests .....................***Skipped   0.01 sec
        Start  99: system_tests
 88/133 Test  [#99](/bitcoin-bitcoin/99/): system_tests .........................***Skipped   0.01 sec
<snip>
Total Test time (real) =  80.97 sec

The following tests did not run:
	 74 - raii_event_tests (Skipped)
	 99 - system_tests (Skipped)

kevkevinpal commented at 11:00 PM on September 10, 2024: contributor

ACK 5c80192

cmake -B build -DENABLE_EXTERNAL_SIGNER=OFF (and commented out banman_tests) cmake --build build ctest --test-dir build

Got the following snippit

<snip>
        Start  13: banman_tests
 13/133 Test  [#13](/bitcoin-bitcoin/13/): banman_tests .........................***Skipped   0.01 sec
 </snip>
 
 <snip>
         Start  99: system_tests
 99/133 Test  [#99](/bitcoin-bitcoin/99/): system_tests .........................***Skipped   0.01 sec
 </snip>
 
<snip>
The following tests did not run:
         13 - banman_tests (Skipped)
         99 - system_tests (Skipped)
</snip>

DrahtBot removed the label CI failed on Sep 11, 2024
hebasto commented at 1:41 PM on September 11, 2024: member

cc @maflcko
fanquake merged this on Sep 12, 2024
fanquake closed this on Sep 12, 2024
hebasto deleted the branch on Sep 12, 2024

in src/test/CMakeLists.txt:202 in 5c80192ff6

 198 | @@ -199,7 +199,7 @@ function(add_boost_test source_file)
 199 |        COMMAND test_bitcoin --run_test=${test_suite_name} --catch_system_error=no
 200 |      )
 201 |      set_property(TEST ${test_suite_name} PROPERTY
 202 | -      SKIP_REGULAR_EXPRESSION "no test cases matching filter" "Skipping"
 203 | +      SKIP_REGULAR_EXPRESSION "no test cases matching filter"

maflcko commented at 12:38 PM on September 12, 2024:

Just for reference. This never worked in the first place, because "Skipping" was never picked up, because logging was disabled until it was re-added in commit f03c9420958de31fdfecec5fa3e23134aac61803

maflcko commented at 12:39 PM on September 12, 2024: member

lgtm
TheCharlatan referenced this in commit 69282950aa on Sep 16, 2024
TheCharlatan referenced this in commit dfe0cd4ec5 on Sep 16, 2024
bitcoin locked this on Sep 12, 2025

Contributors

Labels

Linked (view graph)

#15382 util: add RunCommandParseJSON #16564 test: Always define the raii_event_tests test suite

test: Drop no longer needed workarounds #30847

Code Coverage

Reviews