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: add pay test over unannounced channels #7844

Conversation

JssDWt
Copy link
Contributor

@JssDWt JssDWt commented Nov 19, 2024

This test fails with cln v24.08.2. Add this test, so it doesn't happen again.

Checklist

Before submitting the PR, ensure the following tasks are completed. If an item is not applicable to your PR, please mark it as checked:

  • The changelog has been updated in the relevant commit(s) according to the guidelines.
  • Tests have been added or modified to reflect the changes.
  • Documentation has been reviewed and updated as needed.
  • Related issues have been listed and linked, including any that this PR closes.

@JssDWt JssDWt force-pushed the jssdwt-add-single-hop-routehint-test branch from 29266fd to aa7a2a3 Compare November 19, 2024 21:49
Copy link
Collaborator

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

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

left a comment, otherwise looks good to me

Concept ACK aa7a2a3

tests/test_pay.py Outdated Show resolved Hide resolved
@JssDWt JssDWt force-pushed the jssdwt-add-single-hop-routehint-test branch 2 times, most recently from c0a99a2 to f3c98b7 Compare November 19, 2024 22:19
@JssDWt
Copy link
Contributor Author

JssDWt commented Nov 19, 2024

The test was flaky
Added another random public node to ensure the route hint is added.
Added a check that the recipient sees the public channels.
Added a check that the payment completed.
Rebased to resolve a conflict.

@JssDWt JssDWt force-pushed the jssdwt-add-single-hop-routehint-test branch from f3c98b7 to 9b3c143 Compare November 20, 2024 11:03
vincenzopalazzo

This comment was marked as off-topic.

Copy link
Collaborator

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

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

I just rember that pay si returning the status too, so probably there is a clean way to check

tests/test_pay.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

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

sorry for the floppy review, adding another consideration

tests/test_pay.py Outdated Show resolved Hide resolved
@vincenzopalazzo vincenzopalazzo added this to the v24.11 milestone Nov 20, 2024
@vincenzopalazzo vincenzopalazzo self-assigned this Nov 20, 2024
@JssDWt JssDWt force-pushed the jssdwt-add-single-hop-routehint-test branch 2 times, most recently from 42d56b4 to 7971199 Compare November 21, 2024 09:06
This test fails with cln v24.08.2. Add this test, so it doesn't happen
again.

Changelog-None
@JssDWt JssDWt force-pushed the jssdwt-add-single-hop-routehint-test branch from 7971199 to abd2201 Compare November 21, 2024 09:16
Copy link
Collaborator

@vincenzopalazzo vincenzopalazzo left a comment

Choose a reason for hiding this comment

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

ACK abd2201

@vincenzopalazzo vincenzopalazzo enabled auto-merge (rebase) November 21, 2024 09:23
@vincenzopalazzo vincenzopalazzo merged commit a90d9c9 into ElementsProject:master Nov 21, 2024
14 of 39 checks passed
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