The test suite must always be defined (even when EVENT_SET_MEM_FUNCTIONS_IMPLEMENTED is not defined) so that the test harness doesn’t fail due to not being able to find the raii_event_tests test.
This improves upon 95f97f4 actually fixing https://github.com/bitcoin/bitcoin/issues/9493
Don’t make pull requests into existing releases, make them into master.
Edit: the maintainers pull things into the tagged release versions as appropriate. Pull requests basically always go into master, unless they are specific to some release. Yours seems like a generic fix, although I recognize the gentoo downstream issue.
Don’t make pull requests into existing releases, make them into master.
I’ve rebased this on master, thank you very much in advance
Are there steps to reproduce the bug and see that this commit fixes it?
Have libevent installed so that EVENT_SET_MEM_FUNCTIONS_IMPLEMENTED is not defined (on Gentoo, that’s done by installing dev-libs/libevent with the debug flag unset.).
Then, build bitcoin and run the tests. You’ll get this error output:
0/var/tmp/portage/net-p2p/bitcoind-0.18.0-r1/work/bitcoin-2472733a24a9364e4c6233ccd04166a26a68cc65/src # test/test_bitcoin -l test_suite -t raii_event_creation
1Test setup error: no test cases matching filter or all test cases were disabled
With this change, you don’t get such a failure.
#if guard inside the boost_auto_test_case, so that the suite and test case appear only once in the file? Also, this does not compile.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
No conflicts as of last run.
9+BOOST_FIXTURE_TEST_SUITE(raii_event_tests, BasicTestingSetup)
10+
11+#ifndef EVENT_SET_MEM_FUNCTIONS_IMPLEMENTED
12+BOOST_AUTO_TEST_CASE(raii_event_creation)
13+{
14+ // dummy; do nothing
Suggest replacing with:
0 // It would probably be ideal to report skipped, but boost::test doesn't seem to make that practical (at least not in versions available with common distros)
1 BOOST_TEST_MESSAGE("Skipping raii_event_tess: libevent doesn't support event_set_mem_functions");
3@@ -4,8 +4,18 @@
4
5 #include <event2/event.h>
6
7+#include <boost/test/unit_test.hpp>
8+
9+BOOST_FIXTURE_TEST_SUITE(raii_event_tests, BasicTestingSetup)
10+
11+#ifndef EVENT_SET_MEM_FUNCTIONS_IMPLEMENTED
The test suite must always be defined (even when EVENT_SET_MEM_FUNCTIONS_IMPLEMENTED is not defined) so that the test harness doesn't fail due to not being able to find the raii_event_tests test.
This improves upon 95f97f4 actually fixing https://github.com/bitcoin/bitcoin/issues/9493
3@@ -4,8 +4,18 @@
4
5 #include <event2/event.h>
6
7+#include <boost/test/unit_test.hpp>
8+
9+BOOST_FIXTURE_TEST_SUITE(raii_event_tests, BasicTestingSetup)
10+
11+#ifndef EVENT_SET_MEM_FUNCTIONS_IMPLEMENTED
12+BOOST_AUTO_TEST_CASE(raii_event_creation)
raii_event_tests_SKIPPED
@candrews Here’s a branch that can be force-pushed over this one if you like it:
https://github.com/bitcoin/bitcoin/compare/master...luke-jr:raii_event_test_fix-0.14
(Cleanly merges to 0.14.0 through master)
@candrews Here’s a branch that can be force-pushed over this one if you like it:
master…luke-jr:raii_event_test_fix-0.14
(Cleanly merges to 0.14.0 through master)
Awesome, that looks great!
raii_event_tests (and potentially other test suites) when disabled based on build configuration, instead of working around it by conditionally adding empty tests.
raii_event_tests as requested above?
bumping this to see if you’d like to continue to move this forward towards a solution that doesn’t run
raii_event_testsas requested above?
If someone has a suggestion as to how to do that, I’d be happy to update this PR to implement it.
https://packages.debian.org/jessie/libboost-all-dev is going to die in about a month https://wiki.debian.org/DebianReleases#Production_Releases
https://packages.ubuntu.com/xenial/libboost-all-dev is going to die in less than a year https://ubuntu.com/about/release-cycle
EVENT_SET_MEM_FUNCTIONS_IMPLEMENTED? (I am assuming that EVENT_SET_MEM_FUNCTIONS_IMPLEMENTED is defined with xenial libevent?)
The alternative was closed #16228 (comment), so this seems the best bet right now.
It can be improved upon later by bumping boost #16564 (comment), but that might or might not be controversial.
ACK
Tested ACK:
make check fails as if0$ ./src/test/test_bitcoin -t raii_event_tests
1Test setup error: no test cases matching filter or all test cases were disabled
0$ ./src/test/test_bitcoin -t raii_event_tests
1Running 1 test case...
2
3*** No errors detected
ACK 9a19c9ada53e84d1ee8521b48f656faad48b737a 🎹
Signature:
0-----BEGIN PGP SIGNED MESSAGE-----
1Hash: SHA512
2
3ACK 9a19c9ada53e84d1ee8521b48f656faad48b737a 🎹
4-----BEGIN PGP SIGNATURE-----
5
6iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
7pUgI0wv/eTNXTgQxxtVBd5iJ83l2Nl0JDo6pKMb1HnS7O+RYBuVywr365x3RuiN5
8q+GtEc+od70x0Ln/YcJ3tDYcUYsUg/N39petAgE2vkYnD4hCoGBRuV6ftRWiHSwS
9i099iG5luVWByDfpj/Q7/RJU0r22t2Aa5brUjZE2/yFUZx7EmomK5dWMsRCayigW
105hFKbMRXNkJekqOSrGqfP9LgZesgCbl06BFOPLnG74Uue5Bnc8sPfn1HwzFpuRwK
11Mrgiha3L4wj3Xvq4czcM1mVMDBQtdEH3fY2j0JlNZCxwP4DRBYq+fsg0xXZEqWbu
12w61sW/xKTeAoNTSYqkc1trM1e140xed7Ifc/mle/w2qGrKO0YTrys11z6wDN3lmX
13ETR1xVjUwbqZopZNk4iGiEU2t6/hjyL/LQI2KEjHMAO4fX+2FNvdDrS0Sl/Et7N7
141ywk9uKlqamOZZA2tWL9bPJy3/HV5KZHA/O2GHjypQjRVHl2vormBadiVVwYGmAq
15Te/qqL3D
16=ARMg
17-----END PGP SIGNATURE-----
Timestamp of file with hash 10409394f1acc2ff628bcbe5435062c7affa09d444d74bc8b734e1c9f9ba9f87 -