Skip to content

DiffViewer: add test coverage for JumpToHunk during in-flight reload (stale bar click) #33

@geevensingh

Description

@geevensingh

Problem

DiffPaneViewModel.JumpToHunk(int index) (DiffViewer/ViewModels/DiffPaneViewModel.cs:647-659) is the entry point used by the hunk overview bar when the user clicks a marker:

public void JumpToHunk(int index)
{
    if (index < 0 || index >= _currentHunks.Count) return;
    CurrentHunkIndex = index;
    RaiseHunkNav();
}

The clamp is documented (XML doc lines 647-653):

Out-of-range indices are clamped to a no-op rather than throwing — the overview bar's hit-test math can briefly disagree with the VM's hunk list mid-reload.

This is the right behaviour: during a load, the bar's geometry was computed against the previous hunk list, and a click can resolve to an index that no longer exists. But there is no test for this scenario, and:

  1. If someone removes the bounds check thinking "the bar only emits valid indices," any user click during a reload will throw IndexOutOfRangeException and crash the app.
  2. If someone changes the no-op to instead clamp index to [0, _currentHunks.Count - 1], the user will be silently scrolled to a different hunk than they clicked — a subtle UX bug.

Suggested tests

Add to DiffPaneViewModelTests.cs:

[Fact]
public void JumpToHunk_OutOfRange_NoOps()
{
    var repo = new FakeRepoForKeyboard();
    repo.SetChanges(ModifiedChange("a.cs"));
    repo.LeftText  = "alpha\n";
    repo.RightText = "ALPHA\n";

    var diff = new DiffService();
    var vm = new DiffPaneViewModel(repo, diff);
    vm.LoadAsync(EntryFor("a.cs")).GetAwaiter().GetResult();
    var nHunks = vm.CurrentHunks.Count;

    var raised = 0;
    vm.HunkNavigationRequested += (_, _) => raised++;

    vm.JumpToHunk(-1);            // negative
    vm.JumpToHunk(nHunks);        // one past end
    vm.JumpToHunk(nHunks + 100);  // way past end

    raised.Should().Be(0, "out-of-range JumpToHunk should be a no-op, not throw or clamp");
    vm.CurrentHunkIndex.Should().Be(-1, "CurrentHunkIndex should not be modified");
}

[Fact]
public async Task JumpToHunk_DuringReload_DropsStaleClick()
{
    // Simulates the documented race: bar's hit-test resolved to an index
    // valid for the OLD hunk list, but ApplyResult has since installed
    // a smaller list (or empty list during placeholder transition).
    var repo = new FakeRepoForKeyboard();
    repo.SetChanges(ModifiedChange("a.cs"));
    repo.LeftText  = "a\nb\nc\nd\ne\nf\n";       // 6 hunks worth of variation
    repo.RightText = "A\nb\nC\nd\nE\nf\n";

    var diff = new DiffService();
    var vm = new DiffPaneViewModel(repo, diff);
    await vm.LoadAsync(EntryFor("a.cs"));
    var staleMaxIndex = vm.CurrentHunks.Count - 1;

    // Now load an empty diff (e.g., user toggled IgnoreWhitespace and the
    // file becomes whitespace-only). CurrentHunks goes empty.
    repo.LeftText = repo.RightText = "same\n";
    await vm.LoadAsync(EntryFor("a.cs"));

    Action click = () => vm.JumpToHunk(staleMaxIndex);
    click.Should().NotThrow("a stale-bar click during reload must not crash");
    vm.CurrentHunkIndex.Should().Be(-1);
}

Citations

  • DiffViewer/ViewModels/DiffPaneViewModel.cs:647-659JumpToHunk and its docs
  • DiffViewer/Rendering/HunkOverviewBar.cs — the producer of JumpToHunk(...) calls
  • DiffViewer/ViewModels/DiffPaneViewModel.cs:483-513ApplyResult (where _currentHunks is replaced; the race window)

Severity

Low (no known bug today). Test gap protecting against crash on user click during reload — unobservable in normal use, but not tested anywhere.

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions