Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: coverage for ipex exn recipient endpoint for multi-sig IPEX #288

Merged

Conversation

iFergal
Copy link
Collaborator

@iFergal iFergal commented Sep 1, 2024

Coverage for #286

@lenkan Since these are unit tests, they don't rely on real exn messages being passed back and forth over HTTP. The messages are just cloned on the other agent, which is why the tests didn't catch it.

Added some assertions for where the messages would be sent on a real deployment.

Copy link

codecov bot commented Sep 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.20%. Comparing base (18d3ad7) to head (fee687a).
Report is 16 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #288      +/-   ##
==========================================
+ Coverage   93.06%   93.20%   +0.13%     
==========================================
  Files          36       36              
  Lines        7121     7766     +645     
==========================================
+ Hits         6627     7238     +611     
- Misses        494      528      +34     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@iFergal
Copy link
Collaborator Author

iFergal commented Sep 2, 2024

By the way @lenkan there's 2 ways to do multi-sig IPEX.

e.g. for admit

  • Create IPEX admit and call submitAdmit
  • Create /multisig/exn with admit embedded and send exn to group members individually with sendFromEvents

OR

  • Create IPEX admit but don't send
  • Create /multisig/exn with admit embedded and call submitAdmit with the /multisig/exn

This PR tackles the second, more efficient route but the Signify-TS tests all seem to use the first method. Not sure what you are using on your end. Might be why no one has spotted this bug until now

@lenkan
Copy link
Collaborator

lenkan commented Sep 2, 2024

By the way @lenkan there's 2 ways to do multi-sig IPEX.

Thanks for the hint. I have been using the first approach (as that is what the Signify-TS examples were using).

Copy link
Collaborator

@2byrds 2byrds left a comment

Choose a reason for hiding this comment

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

LGTM

src/keria/app/ipexing.py Show resolved Hide resolved
@2byrds 2byrds merged commit 5e91152 into WebOfTrust:main Sep 2, 2024
5 checks passed
@iFergal iFergal deleted the test/multiSigIpexReceiver branch September 2, 2024 20:39
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