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

tests: basic cli/server integration tests #171

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

orbitalturtle
Copy link
Collaborator

This PR puts a basic test of the server in place, testing that the cli + server can properly make a payment.

To do so required a little refactoring:

  • IIUC initially a RefCell was added to the LndNodeSigner because the original version of tonic_lnd returns a mutable reference to the signer. However I don't think this reference needs to be mutable. For this PR, this was needed so we could spin up the onion messenger and server in a separate thread in the integration tests. But it's a change that could help with future efficiency as well now the onion messenger can be used in a multi-threaded way.
  • Split off the server formation logic from main.rs into a separate function so that we can call it in the integration tests as well.

Initially a RefCell was added to the LndNodeSigner because the original version
of tonic_lnd returns a mutable reference to the signer. However this reference
doesn't need to be mutable:
orbitalturtle/tonic_lnd#4

For this PR, this was needed so we could spin up the onion messenger and server
in a separate thread in the integration tests. But it's a change that could
help with future efficiency as well because the onion messenger can now be used
in a multi-threaded way.
It's just a more suitable location.
Copy link

codecov bot commented Sep 6, 2024

Codecov Report

Attention: Patch coverage is 0% with 8 lines in your changes missing coverage. Please review.

Project coverage is 0.00%. Comparing base (1cf2906) to head (63b0bd7).

Files with missing lines Patch % Lines
src/main.rs 0.00% 8 Missing ⚠️
Additional details and impacted files
@@          Coverage Diff           @@
##           master    #171   +/-   ##
======================================
  Coverage    0.00%   0.00%           
======================================
  Files           1       1           
  Lines         126      92   -34     
======================================
+ Misses        126      92   -34     

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

pubkey: PublicKey,
secp_ctx: Secp256k1<secp256k1::All>,
signer: RefCell<&'a mut tonic_lnd::SignerClient>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Finally lol

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.

2 participants