Skip to content

lnwallet/test: increase wait for wallet to sync to 30s to match with the returned error in case of timeout#10160

Merged
guggero merged 1 commit intolightningnetwork:masterfrom
mohamedawnallah:increase-wait-wallet-sync-timeout
Aug 15, 2025
Merged

lnwallet/test: increase wait for wallet to sync to 30s to match with the returned error in case of timeout#10160
guggero merged 1 commit intolightningnetwork:masterfrom
mohamedawnallah:increase-wait-wallet-sync-timeout

Conversation

@mohamedawnallah
Copy link
Contributor

@mohamedawnallah mohamedawnallah commented Aug 15, 2025

Motivation and Context

That test TestLightningWallet/btcwallet/neutrino:reorg_wallet_balance failed recently. It is not clear why it flaky failed. Currently the timeout was set to 10seconds but the error message said it is timeout after 30seconds. Increasing the timeout may give the wallet some time to sync as well as to match the error message:
https://github.com/lightningnetwork/lnd/actions/runs/16982438758/job/48145091023?pr=9455

Pull Request Checklist

Testing

  • Your PR passes all CI checks.
  • Tests covering the positive and negative (error paths) are included.
  • Bug fixes contain tests triggering the bug to prevent regressions.

Code Style and Documentation

📝 Please see our Contribution Guidelines for further guidance.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Summary of Changes

Hello @mohamedawnallah, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses a flakiness issue in the TestLightningWallet/btcwallet/neutrino:reorg_wallet_balance test. The problem stemmed from a mismatch between the test's internal timeout and the timeout indicated in error messages. By increasing the waitForWalletSync timeout from 10 seconds to 30 seconds, this change aims to align the test's waiting period with the expected timeout, thereby stabilizing the test and preventing future intermittent failures.

Highlights

  • Increased Test Timeout: The timeout variable within the waitForWalletSync function, used to wait for wallet synchronization in tests, has been extended from 10 seconds to 30 seconds.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in issue comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request increases the timeout in waitForWalletSync to 30 seconds, aligning it with the error message and addressing a flaky test. The change is correct and improves the reliability of the test suite. While reviewing, I noticed a similar inconsistency in the waitForMempoolTx function, where the timeout is set to 30 seconds but the error message reports a 10-second timeout. It would be beneficial to address that in a follow-up to maintain consistency across the test suite.

@mohamedawnallah mohamedawnallah changed the title lnwallet: increase wait for wallet to sync to 30s to match with the returned error in case of timeout lnwallet/test: increase wait for wallet to sync to 30s to match with the returned error in case of timeout Aug 15, 2025
@mohamedawnallah
Copy link
Contributor Author

mohamedawnallah commented Aug 15, 2025

While reviewing, I noticed a similar inconsistency in the waitForMempoolTx function, where the timeout is set to 30 seconds but the error message reports a 10-second timeout. It would be beneficial to address that in a follow-up to maintain consistency across the test suite.

👍 (EDIT: Addressed)

@mohamedawnallah mohamedawnallah force-pushed the increase-wait-wallet-sync-timeout branch from 9d994a2 to e8283e9 Compare August 15, 2025 06:05
@mohamedawnallah
Copy link
Contributor Author

mohamedawnallah commented Aug 15, 2025

Even though PR btcsuite/btcwallet#1040 looks like it solves the main CI issue. I think this PR still relevant

cc: @yyforyongyu

Copy link
Member

@yyforyongyu yyforyongyu left a comment

Choose a reason for hiding this comment

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

LGTM🏄

@mohamedawnallah
Copy link
Contributor Author

cc @ellemouton

@guggero guggero merged commit fb1adfc into lightningnetwork:master Aug 15, 2025
38 of 39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants