Skip to content

Add additional tests for uri module#578

Merged
spacebear21 merged 1 commit intopayjoin:masterfrom
benalleng:uri_mutants
Mar 24, 2025
Merged

Add additional tests for uri module#578
spacebear21 merged 1 commit intopayjoin:masterfrom
benalleng:uri_mutants

Conversation

@benalleng
Copy link
Copy Markdown
Collaborator

This expands the test coverage for the uri module covering all mutants that currently exist.

This will likely become the narrowest scope of my mutant ci coverage so that we can start with full coverage and expand the scope out as we catch more mutants.

Here is the mutants out for payjoin/src/uri/*.rs as of c1cc487

Details
caught   payjoin/src/uri/mod.rs:190:25: replace && with || in <impl DeserializationState for DeserializationState>::finalize in 12.4s build + 1.2s test
caught   payjoin/src/uri/mod.rs:188:38: replace == with != in <impl DeserializationState for DeserializationState>::finalize in 9.6s build + 1.3s test
caught   payjoin/src/uri/url_ext.rs:116:5: replace get_param -> Option<T> with None in 9.3s build + 1.1s test
caught   payjoin/src/uri/url_ext.rs:32:16: replace != with == in <impl UrlExt for Url>::receiver_pubkey in 10.4s build + 1.2s test
caught   payjoin/src/uri/mod.rs:25:9: replace MaybePayjoinExtras::pj_is_supported -> bool with false in 10.7s build + 1.3s test
caught   payjoin/src/uri/url_ext.rs:130:80: replace + with - in set_param in 11.1s build + 1.0s test
caught   payjoin/src/uri/mod.rs:189:21: replace || with && in <impl DeserializationState for DeserializationState>::finalize in 73.7s build + 0.9s test
caught   payjoin/src/uri/url_ext.rs:60:49: replace <impl UrlExt for Url>::set_ohttp with () in 4.0s build + 0.8s test
caught   payjoin/src/uri/mod.rs:141:53: replace <impl DeserializationState for DeserializationState>::is_param_known -> bool with false in 3.2s build + 0.7s test
caught   payjoin/src/uri/url_ext.rs:42:9: replace <impl UrlExt for Url>::set_receiver_pubkey with () in 4.1s build + 1.0s test
caught   payjoin/src/uri/url_ext.rs:137:8: delete ! in set_param in 4.0s build + 0.9s test
caught   payjoin/src/uri/url_ext.rs:127:5: replace set_param with () in 83.5s build + 0.7s test
caught   payjoin/src/uri/url_ext.rs:84:9: replace <impl UrlExt for Url>::set_exp with () in 3.2s build + 0.8s test
caught   payjoin/src/uri/mod.rs:25:9: replace MaybePayjoinExtras::pj_is_supported -> bool with true in 84.6s build + 0.8s test
caught   payjoin/src/uri/mod.rs:84:61: replace PayjoinExtras::is_output_substitution_disabled -> bool with true in 2.7s build + 0.8s test
caught   payjoin/src/uri/url_ext.rs:130:80: replace + with * in set_param in 3.0s build + 0.7s test
caught   payjoin/src/uri/mod.rs:141:53: replace <impl DeserializationState for DeserializationState>::is_param_known -> bool with true in 3.3s build + 0.8s test
caught   payjoin/src/uri/url_ext.rs:77:39: replace + with - in <impl UrlExt for Url>::exp in 3.3s build + 0.9s test
caught   payjoin/src/uri/mod.rs:189:42: replace == with != in <impl DeserializationState for DeserializationState>::finalize in 3.0s build + 0.8s test
caught   payjoin/src/uri/mod.rs:84:61: replace PayjoinExtras::is_output_substitution_disabled -> bool with false in 2.8s build + 0.7s test
caught   payjoin/src/uri/url_ext.rs:71:16: replace != with == in <impl UrlExt for Url>::exp in 2.6s build + 0.6s test
25 mutants tested in 2m 14s: 21 caught, 4 unviable

Note, this is with the mutants.tomlexclusions from #573 in place

@coveralls
Copy link
Copy Markdown
Collaborator

coveralls commented Mar 13, 2025

Pull Request Test Coverage Report for Build 14042503864

Details

  • 58 of 61 (95.08%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.3%) to 81.343%

Changes Missing Coverage Covered Lines Changed/Added Lines %
payjoin/src/uri/mod.rs 29 32 90.63%
Totals Coverage Status
Change from base Build 14042339636: 0.3%
Covered Lines: 5062
Relevant Lines: 6223

💛 - Coveralls

Comment thread payjoin/src/uri/url_ext.rs
Comment thread payjoin/src/uri/mod.rs Outdated
Comment thread payjoin/src/uri/mod.rs Outdated
Comment thread payjoin/src/uri/mod.rs Outdated
Comment thread payjoin/src/uri/mod.rs Outdated
Comment thread payjoin/src/uri/mod.rs Outdated
Comment thread payjoin/src/uri/mod.rs Outdated
Comment on lines +295 to +300
let enabled = PayjoinExtras { endpoint: url.clone(), disable_output_substitution: true };

let disabled = PayjoinExtras { endpoint: url.clone(), disable_output_substitution: false };

assert!(enabled.is_output_substitution_disabled());
assert!(!disabled.is_output_substitution_disabled());
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This test doesn't look super useful. If the goal is to add mutation coverage for is_output_substitution_disabled a more useful test would be one that parses the pjos parameter and tests the parsed values, like @Gmin2 started here

Copy link
Copy Markdown
Collaborator Author

@benalleng benalleng Mar 14, 2025

Choose a reason for hiding this comment

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

Elected to not include a check for is_output_substitution_disabled() as #546 also has complete coverage for that plus the corrected inverse values. Though I did create a serialization test as you referenced in you last review there (though this will likely need to be updated with the inverse 0 <=> 1 in the future still anyways).

Comment thread payjoin/src/uri/mod.rs Outdated
Comment thread payjoin/src/uri/url_ext.rs Outdated
Comment thread payjoin/src/uri/url_ext.rs Outdated
Comment thread payjoin/src/uri/url_ext.rs
@benalleng benalleng force-pushed the uri_mutants branch 4 times, most recently from fa8b1dd to 9535492 Compare March 14, 2025 20:19
@benalleng benalleng force-pushed the uri_mutants branch 3 times, most recently from 75851a8 to 9e871f6 Compare March 18, 2025 17:37
Copy link
Copy Markdown
Collaborator

@spacebear21 spacebear21 left a comment

Choose a reason for hiding this comment

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

ACK 9e871f6

I think we should wait until #546 gets merged (which should be imminent), then rebase this PR and combine/correct the output substitution tests per #578 (comment).

This expands the test coverage for the uri module covering
all mutants that currently exist.

This will likely become the narrowest scope of my mutant ci coverage
so that we can start with full coverage and expand the scope out as
we catch more mutants.
Copy link
Copy Markdown
Collaborator

@spacebear21 spacebear21 left a comment

Choose a reason for hiding this comment

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

ACK 86f9208

@spacebear21 spacebear21 merged commit d510ace into payjoin:master Mar 24, 2025
7 checks passed
@benalleng benalleng deleted the uri_mutants branch March 27, 2026 15:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants