Add additional tests for v2 send module by benalleng · Pull Request #554 · payjoin/rust-payjoin

benalleng · 2025-02-25T18:38:27Z

Relates to #539

This is an attempt to both add some additional test vectors and to create some headway on the tests deficiencies caught by cargo mutants ahead of time.
Currently missing is the test for process_response which I was unable to make significant progress on as I was not able to parse what I anticipated as expected values for the ohttp encapsulation/decapsulation. Ideally we should have a test for that added so I have created a TODO there.

I have tried my best to keep the tests in accordance with the design laid out in #487

Below is the cargo mutants output for send/v2/mod.rs as of df30f3b

Details

caught   payjoin/src/send/v2/mod.rs:203:5: replace serialize_v2_body -> Result<Vec<u8>, CreateRequestError> with Ok(vec![]) in 8.7s build + 1.1s test
caught   payjoin/src/send/v2/mod.rs:203:5: replace serialize_v2_body -> Result<Vec<u8>, CreateRequestError> with Ok(vec![1]) in 5.9s build + 1.1s test
***MISSED   payjoin/src/send/v2/mod.rs:146:45: replace > with == in Sender::extract_v2 in 10.2s build + 1.5s test
caught   payjoin/src/send/v2/mod.rs:203:5: replace serialize_v2_body -> Result<Vec<u8>, CreateRequestError> with Ok(vec![0]) in 8.5s build + 1.1s test
***MISSED   payjoin/src/send/v2/mod.rs:289:9: replace V2GetContext::process_response -> Result<Option<Psbt>, ResponseError> with Ok(None) in 8.6s build + 1.2s test
caught   payjoin/src/send/v2/mod.rs:146:45: replace > with < in Sender::extract_v2 in 66.0s build + 0.6s test
17 mutants tested in 1m 32s: 2 missed, 4 caught, 11 unviable

Note the 2 main improvements that can be made are for creating a test for process_response and catching the test_extract_v2_fails_when_expired slightly better

coveralls · 2025-02-25T18:42:57Z

Pull Request Test Coverage Report for Build 13789274853

Details

88 of 90 (97.78%) changed or added relevant lines in 3 files are covered.
4 unchanged lines in 2 files lost coverage.
Overall coverage increased (+0.5%) to 80.165%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
payjoin/src/send/v2/mod.rs	80	82	97.56%

Files with Coverage Reduction	New Missed Lines	%
payjoin/src/send/v2/mod.rs	1	96.59%
payjoin-test-utils/src/lib.rs	3	97.01%

Totals
Change from base Build 13751979319:	0.5%
Covered Lines:	4652
Relevant Lines:	5803

💛 - Coveralls

spacebear21

Currently missing is the test for process_response which I was unable to make significant progress on as I was not able to parse what I anticipated as expected values for the ohttp encapsulation/decapsulation. Ideally we should have a test for that added so I have created a TODO there.

Since the response must come from the OHTTP relay, I think the only way to do this in a unit test would be to get a "real" OHTTP response from e.g. an integration test and all known parameters, and hardcode these into the unit test.

Until the test actually exists, we should remove the placeholder. Currently it results in a test "success" which is misleading since it doesn't actually test anything.

payjoin/src/send/v2/mod.rs

benalleng · 2025-02-26T17:28:32Z

Ok! Yeah I was exploring the do_v2_send_receive integration test but short of passing in a fixed response which is ~8000 bytes long I couldn't think of a way to realistically test this as it's all currently async tokio tests there.
I'll go ahead and remove the placeholder

payjoin/src/send/v2/mod.rs

payjoin/src/receive/v2/mod.rs

spacebear21

ACK a81b432

DanGould

Nice additional coverage

ACK a81b432

Message matching must be replaced with variant matching upon #403

payjoin/src/send/v2/mod.rs

Anunayj · 2025-03-05T23:23:53Z

payjoin/src/send/v2/mod.rs

+        let sender = create_sender_context()?;
+        let ohttp_relay = EXAMPLE_URL.clone();
+        let result = sender.extract_v2(ohttp_relay);
+        assert!(result.is_ok(), "Expected Ok result, got: {:#?}", result.err());


This is great! but I think unwrap() on the result is going to be more verbose, and better for debugging when the test does fail.

I am doing my best to avoid .unwrap() in these tests just to keep in line with the remove unwrap even in tests issue I referenced at the beginning of this PR

FWIW I think this line is equivalent to doing something likelet (request, context) = result.expect("result shoud be Ok"); - this avoids unwrap, saves a line, and provides a more verbose output on failure.

payjoin/src/send/v2/mod.rs

DanGould

@elnosh caught a real problem

payjoin/src/send/v2/mod.rs

This is an attempt to both add some additional test vectors and to create some headway on the tests deficiencies caught by cargo mutants. Currently missing is the test for process_response which I was unable to make significant progress on as that test currently expects an actual response from an ohttp relay. Until we can make any progress on that test I have left that test absent for now.

Addressed in follow-up

spacebear21

ACK 53db3f8

@benalleng thank you very much for your diligent work on this PR, and thanks to all who reviewed and provided great feedback!

DanGould · 2025-03-13T14:39:49Z

Wow, great to see this merged this morning. Thanks Ben, Spacebear, and the many others who came to review 🔥

spacebear21 reviewed Feb 26, 2025

View reviewed changes

payjoin/src/send/v2/mod.rs Outdated Show resolved Hide resolved

payjoin/src/send/v2/mod.rs Outdated Show resolved Hide resolved

benalleng force-pushed the send-v2-module-tests branch from c68713f to 0dd746b Compare February 26, 2025 18:53

benalleng changed the title ~~Add tests for send v2 module~~ Add additional tests for send v2 module Feb 26, 2025

benalleng requested a review from spacebear21 February 26, 2025 19:04

benalleng force-pushed the send-v2-module-tests branch 2 times, most recently from 88f1cba to 7df2f64 Compare February 26, 2025 23:22

benalleng changed the title ~~Add additional tests for send v2 module~~ Add additional tests for v2 send module Feb 27, 2025

spacebear21 reviewed Feb 28, 2025

View reviewed changes

payjoin/src/send/v2/mod.rs Outdated Show resolved Hide resolved

benalleng force-pushed the send-v2-module-tests branch 2 times, most recently from f88b4d8 to 23bbd82 Compare March 4, 2025 15:16

deadmanoz reviewed Mar 4, 2025

View reviewed changes

payjoin/src/receive/v2/mod.rs Outdated Show resolved Hide resolved

benalleng force-pushed the send-v2-module-tests branch from 23bbd82 to a81b432 Compare March 4, 2025 16:42

spacebear21 approved these changes Mar 4, 2025

View reviewed changes

DanGould approved these changes Mar 4, 2025

View reviewed changes

payjoin/src/send/v2/mod.rs Outdated Show resolved Hide resolved

benalleng force-pushed the send-v2-module-tests branch from a81b432 to 46f4642 Compare March 4, 2025 18:30

elnosh reviewed Mar 5, 2025

View reviewed changes

payjoin/src/send/v2/mod.rs Outdated Show resolved Hide resolved

Anunayj reviewed Mar 5, 2025

View reviewed changes

DanGould previously requested changes Mar 6, 2025

View reviewed changes

payjoin/src/send/v2/mod.rs Outdated Show resolved Hide resolved

benalleng force-pushed the send-v2-module-tests branch 3 times, most recently from f7331ca to 728227d Compare March 7, 2025 17:50

benalleng force-pushed the send-v2-module-tests branch from 728227d to 53db3f8 Compare March 11, 2025 13:30

spacebear21 approved these changes Mar 13, 2025

View reviewed changes

spacebear21 merged commit 47cbbe9 into payjoin:master Mar 13, 2025
7 checks passed

Conversation

benalleng commented Feb 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coveralls commented Feb 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Test Coverage Report for Build 13789274853

Details

💛 - Coveralls

Uh oh!

spacebear21 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

benalleng commented Feb 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

spacebear21 left a comment

Choose a reason for hiding this comment

Uh oh!

DanGould left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Anunayj Mar 5, 2025

Choose a reason for hiding this comment

Uh oh!

benalleng Mar 6, 2025

Choose a reason for hiding this comment

Uh oh!

spacebear21 Mar 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

DanGould left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

spacebear21 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

DanGould commented Mar 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

benalleng commented Feb 25, 2025 •

edited

Loading

coveralls commented Feb 25, 2025 •

edited

Loading

benalleng commented Feb 26, 2025 •

edited

Loading