Skip to content

Conversation

luke-jr
Copy link
Member

@luke-jr luke-jr commented Jun 18, 2019

raii_event_tests is not built if EVENT_SET_MEM_FUNCTIONS_IMPLEMENTED is not defined.
This would previously cause make check to fail.

Side effect: Verbose mode will no longer show the test_bitcoin command line.

Edit: Also added a fix for a regression introduced in #19385

(This is a clean merge to 0.20, 0.21, 22.x, 23.x, and master branches)

@laanwj
Copy link
Member

laanwj commented Jun 18, 2019

Concept ACK, though the ever-complicating knot of shell script is not something I feel confident to review.

@fanquake fanquake requested review from theuni and dongcarl June 18, 2019 08:11
@maflcko
Copy link
Member

maflcko commented Jun 18, 2019

I guess the alternative would be to replace those tests with BOOST_CHECK(true);. But anything seems like a hack. Why is the file compiled at all when none of the tests are compiled?

@luke-jr
Copy link
Member Author

luke-jr commented Jun 18, 2019

My guess is to avoid checking for the boost define in the build system.

@luke-jr
Copy link
Member Author

luke-jr commented Sep 20, 2019

This doesn't work with Boost 1.47 and makes gitian hang because its boost doesn't support the --list_content option.

In search for a better solution, I ended up falling back to basically #16564

Theoretically, we could have configure check the define for us and remove the file entirely, but I'm not sure it's worth the trouble (and doesn't give the user any indication the test was skipped!).

@luke-jr luke-jr closed this Sep 20, 2019
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021
@hebasto
Copy link
Member

hebasto commented Dec 25, 2021

This doesn't work with Boost 1.47 and makes gitian hang because its boost doesn't support the --list_content option.

Now we have minimum Boost 1.64.0.

Also this PR makes make check compatible with https://www.boost.org/doc/libs/1_64_0/libs/test/doc/html/boost_test/tests_organization/enabling.html.

Maybe reopen?

@bitcoin bitcoin unlocked this conversation Dec 25, 2021
@luke-jr luke-jr reopened this Dec 26, 2021
Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approach ACK 97c6bde.

For now there are unresolved conflicts with #18183 and #18472.

@fanquake
Copy link
Member

What is the status of this? Needs conflicts fixed. Needs the CI to run. Also needs the PR description updated.

@luke-jr luke-jr force-pushed the bugfix_raii_check_fail branch from 97c6bde to 20e6745 Compare March 24, 2022 22:50
@luke-jr luke-jr closed this Mar 24, 2022
@luke-jr luke-jr reopened this Mar 24, 2022
@luke-jr
Copy link
Member Author

luke-jr commented Mar 24, 2022

Rebased

@DrahtBot
Copy link
Contributor

🐙 This pull request conflicts with the target branch and needs rebase.

Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a "draft".

luke-jr added 2 commits March 25, 2022 14:01
raii_event_tests is not built if EVENT_SET_MEM_FUNCTIONS_IMPLEMENTED is not defined.
This would previously cause `make check` to fail.

Side effect: Verbose mode will no longer show the test_bitcoin command line.
@luke-jr
Copy link
Member Author

luke-jr commented Mar 25, 2022

Rebased again, and added a fix for a regression introduced by #19385

@luke-jr luke-jr changed the title Bugfix: make check: Only run tests that were compiled Bugfix: make check: Only run tests that were compiled (& print correct log on failure) Mar 25, 2022
@maflcko
Copy link
Member

maflcko commented Mar 25, 2022

This would previously cause make check to fail.

I tried to test this, but failed. make check passes. See also:

./src/test/test_bitcoin -t raii_event_tests -l message && echo $?
Running 1 test case...
Skipping raii_event_tess: libevent doesn't support event_set_mem_functions
Test case raii_event_tests/raii_event_tests_SKIPPED did not check any assertions

*** No errors detected
0

@maflcko
Copy link
Member

maflcko commented Mar 25, 2022

This was already fixed in 091cc4b

@luke-jr luke-jr closed this Mar 25, 2022
@luke-jr
Copy link
Member Author

luke-jr commented Mar 25, 2022

Good point.

@bitcoin bitcoin locked and limited conversation to collaborators Mar 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants