test default PassField impl handles output parameters #221

pull Eunovo wants to merge 2 commits into bitcoin-core:master from Eunovo:test-return-parameter changing 3 files +9 −0

Eunovo commented at 3:33 PM on October 1, 2025: contributor
This PR adds two test cases to ensure that output parameter results from a server call are correctly captured and included in the response. The new tests also serve as practical examples of how to work with output parameters and in|out parameters in the codebase.

Testing

Automated coverage is not yet available, see #198.

As a temporary verification method:
- Comment out these lines and run make check. The new test should fail, confirming it is effective.
- If you then also comment out the newly added tests and rerun make check, it should pass, demonstrating that the lines above were previously uncovered.
DrahtBot commented at 3:33 PM on October 1, 2025: none



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



Reviews

See the guideline for information on the review process.

Type Reviewers

ACK ryanofsky

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

Type	Reviewers
ACK	ryanofsky

in test/mp/test/foo.capnp:16 in f3688fdaed

  12 | @@ -13,6 +13,7 @@ $Proxy.includeTypes("mp/test/foo-types.h");
  13 |  
  14 |  interface FooInterface $Proxy.wrap("mp::test::FooImplementation") {
  15 |      add @0 (a :Int32, b :Int32) -> (result :Int32);
  16 | +    addOut @19 (a :Int32, b :Int32, ret :Int32) -> (ret :Int32);

ryanofsky commented at 6:50 PM on October 1, 2025:

In commit "test default PassField impl handles output parameters" (f3688fdaed1fddd52755857f2b068927f1a428a7)

Since the test is supposed to demonstrate using output parameters and is not using the input value of ret it would be good to make the declaration reflect that by simplifying:

-    addOut [@19](/bitcoin-core-multiprocess/contributor/19/) (a :Int32, b :Int32, ret :Int32) -> (ret :Int32);
+    addOut [@19](/bitcoin-core-multiprocess/contributor/19/) (a :Int32, b :Int32) -> (ret :Int32);

If you wanted to extend the test to cover input/output parameters, too, you could add another function like:

void addInOut(int x, int& sum) { sum += x; }

addInOut [@20](/bitcoin-core-multiprocess/contributor/20/) (x :Int32, sum :Int32) -> (sum :Int32);

Eunovo commented at 8:13 PM on October 1, 2025:

Done

ryanofsky commented at 6:52 PM on October 1, 2025: collaborator

Code review f3688fdaed1fddd52755857f2b068927f1a428a7. Nice test! I'm surprised there wasn't already a test for output parameters. Would suggest making the simplification below but otherwise this looks good.

test default PassField impl handles output parameters

Tests that output parameter result is captured from server call and added to response.

29cf2ada75

test In|Out parameter 6db6696283
Eunovo force-pushed on Oct 1, 2025
ryanofsky approved
ryanofsky commented at 8:25 PM on October 1, 2025: collaborator

Code review ACK 6db669628387b481b0d5726f011eb45025f31ccf. Thanks, both changes look good!
ryanofsky merged this on Oct 2, 2025
ryanofsky closed this on Oct 2, 2025
ryanofsky referenced this in commit 3f74b5aa02 on Oct 2, 2025
ryanofsky referenced this in commit 5bb3cbdfba on Oct 2, 2025
ryanofsky referenced this in commit 5191781886 on Oct 7, 2025
ryanofsky referenced this in commit 0f01e1577f on Oct 7, 2025
ryanofsky referenced this in commit abcd4c4ff9 on Oct 7, 2025
Sjors referenced this in commit 3e34afe882 on Oct 7, 2025
fanquake referenced this in commit becf150013 on Oct 10, 2025
Sjors referenced this in commit 7e61dcfa61 on Oct 10, 2025
fanquake referenced this in commit a14e7b9dee on Oct 16, 2025

Contributors

Eunovo

DrahtBot

ryanofsky

Linked (view graph)

#198 How to generate Test Coverage

test default PassField impl handles output parameters #221

Testing

Reviews