Skip to content

Fix matching updates (#26)#27

Closed
brainiac-five wants to merge 1 commit intomasterfrom
fix/matching-updates
Closed

Fix matching updates (#26)#27
brainiac-five wants to merge 1 commit intomasterfrom
fix/matching-updates

Conversation

@brainiac-five
Copy link
Copy Markdown
Collaborator

@brainiac-five brainiac-five commented Feb 26, 2026

Explanation of the Fix

An update that is an exact key-and-value match with an existing node should return that existing node rather than nil, because the return value from update() is (across some nesting update functions across class layers) expected to be the new node (that in this special case is an old node) and assigned to pm.n in SwarmPot.Update() in mode.go

and to root in Index.Update() in index.go

root = update

Imperfection of this PR

In hindsight this PR should probably better add tests to index_test.go instead of kvs_test.go but the tests added to kvs_test.go track how the bug was found (and reported in #26 / branch issue/26); and because of where the fix is (in pkg/elements/ops.go), the issue would appear to get tested equally well for regression in either suite.

Regarding Added Tests

Three of the tests that are being added by this PR were failing without the fix. See branch issue/26. But not all of them, because they were made to help pinpoint the exact picture of the bug, including what works although it looks similar to the failing scenarios. See issue #26.

* fix: matching updates (#26)

* add: matching KVS update tests
@lexonian
Copy link
Copy Markdown

Force push fixed test prompts (deleted " - ok" / " - returns error" from t.Run calls).

@brainiac-five
Copy link
Copy Markdown
Collaborator Author

Closing because PR #30 solves it, together with issue #29.

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.

2 participants