test: Rename send_message to send_without_ping #31859

1. 
   maflcko commented at 3:44 pm on February 13, 2025: member

   send_message is problematic, because it is easy to forget a sync_with_ping (or other wait_until), leading to intermittent test failures. (Example: #31837 (review))

   There are more uses of send_and_ping in the codebase than send_message, so in most cases send_and_ping is needed anyway.

   For the remaining cases, clearly document that no sync happens by renaming send_message to send_without_ping.

2. 
   DrahtBot commented at 3:44 pm on February 13, 2025: contributor

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

   Code Coverage & Benchmarks

   For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31859.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	instagibbs
   Concept ACK	i-am-yuvi

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #31829 (p2p: improve TxOrphanage denial of service bounds and increase -maxorphantxs by glozow)
   • #29664 (p2p: When close to the tip, download blocks in parallel from additional peers to prevent stalling by mzumsande)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

3. DrahtBot renamed this:
   test: Rename send_message to send_without_ping
   test: Rename send_message to send_without_ping
   on Feb 13, 2025
4. DrahtBot added the label Tests on Feb 13, 2025
5. 
   maflcko commented at 3:59 pm on February 13, 2025: member

   (Looks like there are 4 conflicts, so best to wait until there are 0. Otherwise there will be useless churn)
6. DrahtBot added the label Needs rebase on Feb 14, 2025
7. maflcko marked this as a draft on Feb 14, 2025
8. maflcko force-pushed on Feb 14, 2025
9. DrahtBot removed the label Needs rebase on Feb 14, 2025
10. 
    instagibbs commented at 2:11 pm on February 14, 2025: member

    concept ACK, many intermittent test failures end up being this
11. 
    i-am-yuvi commented at 7:10 am on February 16, 2025: contributor

    Concept ACK I agree!
12. DrahtBot added the label Needs rebase on Mar 14, 2025
13. maflcko marked this as ready for review on Mar 14, 2025
14. test: Prefer send_and_ping over send_message+sync_with_ping

    Also, add an explanation for the preference in the docs.

    fa4356717d
15. scripted-diff: test: Rename send_message to send_without_ping

    send_message only drops the bytes in a buffer and a sync is needed to
    avoid intermittent test issues. Change the name of the method to make
    this more apparent during review.
    
    -BEGIN VERIFY SCRIPT-
     sed -i 's/send_message(/send_without_ping(/g' $( git grep -l 'send_message(' )
    -END VERIFY SCRIPT-

    fa9cf38ab6
16. maflcko force-pushed on Mar 14, 2025
17. DrahtBot removed the label Needs rebase on Mar 14, 2025
18. 
    maflcko commented at 12:50 pm on March 14, 2025: member

    rebased and taken out of draft. This should be ready for review, as all conflicts are draft pull requests.
19. 
    instagibbs commented at 4:53 pm on March 14, 2025: member

    ACK https://github.com/bitcoin/bitcoin/pull/31859/commits/fa9cf38ab666b50b0d8e82cb17f9e5a8a613547d

    Some one-time churn that should help future-proof testing code a bit.

20. fanquake merged this on Mar 16, 2025
21. fanquake closed this on Mar 16, 2025

    ---

22. maflcko deleted the branch on Mar 16, 2025

Contributors
maflcko DrahtBot instagibbs i-am-yuvi

Labels
Tests