Improve test determinism with configurable connect style and deterministic hash maps #4296

joostjager · 2026-01-05T10:45:31Z

Tests previously selected a random block connect style to improve coverage. However, this randomness made test output non-deterministic and significantly cluttered diffs when comparing logs before and after changes.

To address this, an LDK_TEST_CONNECT_STYLE environment variable is added to override the random selection and enable deterministic test runs.

Note that broader coverage may be better achieved via targeted tests per connect style or a test matrix cycling through all styles, but this change focuses on improving reproducibility and debuggability.

Additionally, this PR makes hash map iteration deterministic in tests by using a fixed-seed hasher, eliminating another source of non-determinism in test output.

ldk-reviews-bot · 2026-01-05T10:45:34Z

👋 I see @valentinewallace was un-assigned.
If you'd like another reviewer assignment, please click here.

codecov · 2026-01-05T12:07:35Z

Codecov Report

❌ Patch coverage is 50.00000% with 20 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.63%. Comparing base (62c5849) to head (075bc34).
⚠️ Report is 24 commits behind head on main.

Files with missing lines	Patch %	Lines
lightning/src/ln/functional_test_utils.rs	11.76%	15 Missing ⚠️
lightning/src/util/hash_tables.rs	78.26%	4 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4296      +/-   ##
==========================================
- Coverage   89.38%   86.63%   -2.75%     
==========================================
  Files         180      158      -22     
  Lines      139834   102066   -37768     
  Branches   139834   102066   -37768     
==========================================
- Hits       124985    88422   -36563     
+ Misses      12262    11233    -1029     
+ Partials     2587     2411     -176

Flag	Coverage Δ
fuzzing	`36.83% <0.00%> (+1.61%)`	⬆️
tests	`85.91% <50.00%> (-2.81%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

valentinewallace · 2026-01-05T16:29:21Z

lightning/src/ln/functional_test_utils.rs

 	let payment_count = Rc::new(RefCell::new(0));
-	let connect_style = Rc::new(RefCell::new(ConnectStyle::random_style()));
+
+	let connect_style = Rc::new(RefCell::new(match std::env::var("LDK_TEST_CONNECT_STYLE") {


I'm confused how this works in our no-std tests, like how CI is passing

no-std tests are actually (secretly) built with std enabled. Its useful lol.

Interesting. What was the original reason for that?

IIRC cargo actually forces it on us, but we take advantage of it. Its possible we screwed up our feature flags at some point and accidentally did it, but either way its useful.

valentinewallace · 2026-01-05T16:29:40Z

lightning/src/ln/functional_test_utils.rs

 	let payment_count = Rc::new(RefCell::new(0));
-	let connect_style = Rc::new(RefCell::new(ConnectStyle::random_style()));
+
+	let connect_style = Rc::new(RefCell::new(match std::env::var("LDK_TEST_CONNECT_STYLE") {


Is there some way to make this more discoverable? Maybe CONTRIBUTING.md? I think a comment would also be useful to describe why a dev would want to set this.

Good point to add more docs. It is a bit confronting that we add and document an env var that underlines that there are non-deterministic unit tests, instead of making them deterministic.

We could also think about alternatives such as a 'deterministic_test' cfg flag, which we can then also use to add for example sorting when we iterate over hashmap/set keys?

I've added more docs and also added a commit for deterministic hash table iteration.

ldk-reviews-bot · 2026-01-05T16:30:07Z

👋 The first review has been submitted!

Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer.

TheBlueMatt · 2026-01-07T13:40:45Z

lightning/README.md

+
+### Environment Variables
+
+* `LDK_TEST_CONNECT_STYLE` - Override the random block connect style used in tests for deterministic runs. Valid values:


This should be in CONTRIBUTING.md, not a README.md. Its useful information that devs working on LDK might want to consider, but its not something that downstream devs using LDK should consider, and it definitely doesn't need to show up on our crates.io page.

Indeed, not something for downstream devs. I've moved it to CONTRIBUTING, but still that doc feels more like process guidance. Maybe at some point we want a new doc that describes design/architecture/internals?

Indeed, not something for downstream devs. I've moved it to CONTRIBUTING, but still that doc feels more like process guidance. Maybe at some point we want a new doc that describes design/architecture/internals?

FWIW, there is ARCH.md, though not sure if it makes sense to extend that to cover stuff like test env variables.

Tests previously selected a random block connect style to improve coverage. However, this randomness made test output non-deterministic and significantly cluttered diffs when comparing logs before and after changes. To address this, an LDK_TEST_CONNECT_STYLE environment variable is added to override the random selection and enable deterministic test runs. Note that broader coverage may be better achieved via targeted tests per connect style or a test matrix cycling through all styles, but this change focuses on improving reproducibility and debuggability.

TheBlueMatt · 2026-01-08T12:41:00Z

lightning/src/util/hash_tables.rs

 	mod hasher {
 		pub use std::collections::hash_map::RandomState;
 	}
+	#[cfg(all(feature = "std", test))]


Hmm, test determinism is great, but test coverage is better. I'm a bit skeptical that the solution to "sometimes my tests are flaky due to hashmap iteration order" is to fix the hashmap iteration order, rather than fixing the tests. Having an option in the env like you do above to fix hashmap iteration order to make debugging easier seems reasonable, however.

Fair enough. It's not about flakey tests btw, but about making test output comparable across runs.

Heh, I've def had it make tests flaky before :)

Add a `RandomState` hasher implementation for tests that supports deterministic behavior via the `LDK_TEST_DETERMINISTIC_HASHES=1` environment variable. When set, hash maps use fixed keys ensuring consistent iteration order across test runs. By default, tests continue to use std's RandomState for random hashing, keeping test behavior close to production. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

TheBlueMatt · 2026-01-08T14:12:47Z

Simple enough, thanks.

joostjager marked this pull request as ready for review January 5, 2026 11:33

ldk-reviews-bot requested a review from valentinewallace January 5, 2026 11:33

valentinewallace reviewed Jan 5, 2026

View reviewed changes

joostjager force-pushed the connect-style-var branch from 6972818 to eaef28f Compare January 7, 2026 08:25

joostjager changed the title ~~Make test block connect style configurable for deterministic runs~~ Improve test determinism with configurable connect style and deterministic hash maps Jan 7, 2026

joostjager mentioned this pull request Jan 7, 2026

Support generic HTLC interception #4300

Open

TheBlueMatt reviewed Jan 7, 2026

View reviewed changes

joostjager mentioned this pull request Jan 8, 2026

Follow-ups to #4227 (Part 1) #4289

Open

joostjager force-pushed the connect-style-var branch from 8da55dd to ef25534 Compare January 8, 2026 08:32

joostjager requested review from TheBlueMatt and valentinewallace January 8, 2026 08:37

TheBlueMatt reviewed Jan 8, 2026

View reviewed changes

joostjager force-pushed the connect-style-var branch from ef25534 to 075bc34 Compare January 8, 2026 14:04

TheBlueMatt approved these changes Jan 8, 2026

View reviewed changes

TheBlueMatt merged commit 94cc358 into lightningdevkit:main Jan 8, 2026
16 of 17 checks passed

valentinewallace removed their request for review January 8, 2026 15:21


		### Environment Variables

		* `LDK_TEST_CONNECT_STYLE` - Override the random block connect style used in tests for deterministic runs. Valid values:

Improve test determinism with configurable connect style and deterministic hash maps #4296

Improve test determinism with configurable connect style and deterministic hash maps #4296

Conversation

joostjager commented Jan 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ldk-reviews-bot commented Jan 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jan 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ldk-reviews-bot commented Jan 5, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TheBlueMatt commented Jan 8, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

joostjager commented Jan 5, 2026 •

edited

Loading

ldk-reviews-bot commented Jan 5, 2026 •

edited

Loading

codecov bot commented Jan 5, 2026 •

edited

Loading