cmake: Move IPC tests to ipc/test #33774

1. 
   hebasto commented at 1:10 pm on November 4, 2025: member

   This PR follows up on #33445 and:

   1. Organizes the IPC tests in the same way as the wallet tests.
   2. Removes no longer needed src/test/.clang-tidy.in.

   See the previous discussion:

   • #33445 (review)
   • #33445#pullrequestreview-3411868329

   Additionally, the locality of the bitcoin_ipc_test build target description has been improved.

2. cmake: Move IPC tests to `ipc/test` ae2e438b25
3. hebasto added the label Build system on Nov 4, 2025
4. hebasto added the label Tests on Nov 4, 2025
5. 
   DrahtBot commented at 1:10 pm on November 4, 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/33774.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   Stale ACK	ryanofsky

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

6. 
   hebasto commented at 1:10 pm on November 4, 2025: member

   cc @ryanofsky
7. fanquake requested review from ryanofsky on Nov 4, 2025
8. 
   hebasto commented at 1:56 pm on November 4, 2025: member

   I added a commit with further improvements.
9. hebasto force-pushed on Nov 4, 2025
10. DrahtBot added the label CI failed on Nov 4, 2025
11. DrahtBot removed the label CI failed on Nov 4, 2025
12. 
    janb84 commented at 9:38 am on November 5, 2025: contributor

    should src/test/kernel also be moved to src/kernel/test to keep the project structure Consistent. ?
13. 
    hebasto commented at 10:13 am on November 5, 2025: member

    should src/test/kernel also be moved to src/kernel/test to keep the project structure Consistent. ?

    Maybe.

    Moving IPC tests provides additional benefits. I’m not sure the same justification applies to the kernel tests, so let’s leave them for a follow-up.

14. in src/CMakeLists.txt:334 in f26c8b03f9 outdated

    329-    # bitcoin_ipc_test library target is defined here in src/CMakeLists.txt
    330-    # instead of src/test/CMakeLists.txt so capnp files in src/test/ are able to
    331-    # reference capnp files in src/ipc/capnp/ by relative path. The Cap'n Proto
    332-    # compiler only allows importing by relative path when the importing and
    333-    # imported files are underneath the same compilation source prefix, so the
    334-    # source prefix must be src/, not src/test/
    

    ---

    ---

    

    ryanofsky commented at 1:17 pm on November 5, 2025:

    In commit “cmake, test: Improve locality of bitcoin_ipc_test library description” (f26c8b03f968843e6e1381d1e1721f4379f7e7ab)

    I think this comment is still useful to keep because otherwise there is no explanation of why bitcoin_ipc_test in defined src/ipc/ instead of src/ipc/test where it would make more sense.

    Comment would just need to be tweaked slightly to be kept: src/CMakeLists.txt -> src/ipc/CMakeLists.txt, src/test/CMakeLists.txt -> src/ipc/test/CMakeLists.txt, and src/test/ to src/ipc/test/.

    One of the things I dislike most about working with our build code is that I see a lot of unexpected things done with no explanation and it’s not clear what those things were done intentionally or are accidental sources of complexity that could be cleaned up. I think it is better to call out inconsistencies like this and make intent clear than try to make code that is doing something unusual appear normal.

    ---

    

    hebasto commented at 9:34 pm on November 5, 2025:

    Thanks! The comment has been restored and adjusted.
15. ryanofsky approved
16. 
    ryanofsky commented at 1:26 pm on November 5, 2025: contributor

    Code review ACK f26c8b03f968843e6e1381d1e1721f4379f7e7ab. Nice change avoiding the need to duplicate clang-tidy suppressions. Thanks for the followup!
17. hebasto force-pushed on Nov 5, 2025
18. cmake, test: Improve locality of `bitcoin_ipc_test` library description 866bbb98fd
19. hebasto force-pushed on Nov 5, 2025
20. DrahtBot added the label CI failed on Nov 5, 2025
21. 
    hebasto commented at 9:44 pm on November 5, 2025: member

    The feedback from @ryanofsky has been addressed.

    Additionally, PROJECT_SOURCE_DIR were replaced with CMAKE_CURRENT_SOURCE_DIR.

22. DrahtBot removed the label CI failed on Nov 5, 2025

Contributors
hebasto DrahtBot janb84 ryanofsky

Labels
Build system Tests