Skip to content

Skip default config path in tests#125

Merged
tnull merged 1 commit intolightningdevkit:mainfrom
benthecarman:fix-test
Feb 18, 2026
Merged

Skip default config path in tests#125
tnull merged 1 commit intolightningdevkit:mainfrom
benthecarman:fix-test

Conversation

@benthecarman
Copy link
Collaborator

Tests calling load_config without an explicit config_file would fall back to ~/.ldk-server/config.toml if it existed locally, causing test failures.

@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Feb 16, 2026

I've assigned @tankyleo as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

Some(PathBuf::from(path))
} else {
get_default_config_path().filter(|path| path.exists())
// Skip the default config path during tests to avoid picking up a real ~/.ldk-server/config.toml locally
Copy link
Collaborator

Choose a reason for hiding this comment

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

Makes sense, just wondering if this/a similar override should happen in get_default_config_path itself to ensure that it always applies in tests, independent of where it's called?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was the only call site but yeah that'll be safer in the long run

Tests calling `load_config` without an explicit `config_file` would fall
back to ~`/.ldk-server/config.toml` if it existed locally, causing test
failures.
@ldk-reviews-bot
Copy link

🔔 1st Reminder

Hey @tankyleo! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

@tnull tnull merged commit c9fb6f9 into lightningdevkit:main Feb 18, 2026
7 checks passed
@benthecarman benthecarman deleted the fix-test branch February 18, 2026 08:43
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