Skip to content

Conversation

@RmbRT
Copy link
Contributor

@RmbRT RmbRT commented Feb 1, 2022

TestFunding_PeerTimeout failed sometimes, this was due to an event race in the logic of the funder's waitForFundingConfirmation function: it was handling the same context's Done() event in two different select branches.
In this MR, I properly prioritize the select branches so that the race is resolved.

@RmbRT RmbRT added bug Something isn't working race refactor No logic but only implementation changes. labels Feb 1, 2022
@RmbRT RmbRT requested a review from matthiasgeihs February 1, 2022 23:28
@RmbRT RmbRT force-pushed the fix-funder-timeout-tests branch 2 times, most recently from b4db420 to 2d534a4 Compare February 1, 2022 23:43
@matthiasgeihs
Copy link
Contributor

@RmbRT

Can you insert a link to a pipeline failing because of this?
I don't fully understand the problem yet. Maybe we can also briefly discuss in a call.

@RmbRT
Copy link
Contributor Author

RmbRT commented Feb 2, 2022

Can you insert a link to a pipeline failing because of this? I don't fully understand the problem yet. Maybe we can also briefly discuss in a call.

I don't have a link, sorry. The problem was that the funder sometimes failed to properly report all failed fundings, instead, it returned an error that the context was closed, as a subscription error. Since the tests expect that all missing assets are reported in the error, the tests sometimes failed.

matthiasgeihs
matthiasgeihs previously approved these changes Feb 2, 2022
matthiasgeihs
matthiasgeihs previously approved these changes Feb 2, 2022
matthiasgeihs
matthiasgeihs previously approved these changes Feb 2, 2022
RmbRT added 2 commits February 2, 2022 13:38
Signed-off-by: Steffen Rattay <steffen@perun.network>
Signed-off-by: Steffen Rattay <steffen@perun.network>
@matthiasgeihs matthiasgeihs merged commit 4cb6cbc into main Feb 2, 2022
@matthiasgeihs matthiasgeihs deleted the fix-funder-timeout-tests branch February 2, 2022 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working race refactor No logic but only implementation changes.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants