Skip to content

Reduce the HNSW churn tests to the properties they assert - #4687

Draft
normen662 wants to merge 1 commit into
apple/normen662/hnsw/4-entry-node-promotionfrom
apple/normen662/hnsw/5-trim-churn-tests
Draft

normen662 wants to merge 1 commit into
apple/normen662/hnsw/4-entry-node-promotionfrom
apple/normen662/hnsw/5-trim-churn-tests

Conversation

@normen662

Copy link
Copy Markdown
Contributor

Problem

ChurnReachabilityTest grew two methods that measured rather than asserted. One tallied
every delete against the candidate set it should have used; the other counted node writes
and bytes written per delete. Both were written to answer specific questions about the
delete path and they answered them. Neither asserts anything that a regression would trip,
and together they accounted for about 450 lines.

The class also logged at warn level on every round and once per seed regardless of outcome,
so a successful run printed hundreds of lines.

Change

Remove both methods and the helpers only they used.

Ten tests remain, each asserting a property: that a search centered on a node's own vector
finds it, that no node runs out of usable outgoing edges, that some node can still reach all
the others, that a delete removes references naming deleted nodes, and that the
corresponding callback reports even when it removed nothing.

Move the progress logging behind isDebugEnabled. The warn level is left to the cases that
precede an assertion failure, where the context is worth having. A successful run of the
suite now prints no warn lines at all.

The class grew two methods that measured rather than asserted: one tallied every
delete against the candidate set it should have used, the other counted node
writes and bytes per delete. Both were written to answer questions about the
delete path and answered them; neither asserts anything a regression would trip,
and together they were about 450 lines. Removed, along with the helpers only they
used.

The remaining ten tests each assert a property: that a search centered on a node's
own vector finds it, that no node runs out of usable outgoing edges, that some node
can still reach all the others, that a delete reaps references naming deleted
nodes, and that the reap callback reports even when it reaped nothing.

Also quieted the logging. Progress lines that fired every round or once per seed
regardless of outcome are now behind isDebugEnabled; the warn level is left to the
cases that precede an assertion failure, where the context is worth having.
@normen662 normen662 added the testing improvement Change that improves our testing label Sep 29, 2026
@normen662
normen662 added this pull request to stack #4688 September 29, 2026 09:14

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

testing improvement Change that improves our testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant