Skip to content

test(heap): cover heapify up when removing an item - #2224

Open
renanmpimentel wants to merge 1 commit into
trekhleb:masterfrom
renanmpimentel:test/heap-remove-heapify-up
Open

renanmpimentel wants to merge 1 commit into
trekhleb:masterfrom
renanmpimentel:test/heap-remove-heapify-up

Conversation

@renanmpimentel

@renanmpimentel renanmpimentel commented Oct 6, 2026 •

Copy link
Copy Markdown

Problem

The heapifyUp branch of Heap.remove is not exercised by any test. Replacing

} else {
  this.heapifyUp(indexToRemove);
}

with nothing keeps all heap, priority-queue and heap-sort tests green (41/41), even though the heap property is then violated after such a removal.

Change

Test-only. Add a MinHeap case where the last item moved into the removed slot is smaller than its new parent, and assert the heap layout right after remove():

1,5,2,6,7,3,4  --remove(6)-->  1,4,2,5,7,3

With the heapifyUp branch removed the new test fails (Received: "1,5,2,4,7,3"); on master it passes. Full suite: 588 tests pass, lint clean.


Found with supertest: mutation testing plus statement-removal analysis, with every finding confirmed by running the regression against the unmodified test.

The heapifyUp branch of Heap.remove could be deleted without any test
failing: no existing case moves a smaller item into the removed slot
below a larger parent. Add one that observes the heap right after the
removal.
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.

1 participant