Skip to content

refactor(ui): move the test-only chart reference helpers out of ChartWidgets.h (#3116) - #3134

Merged
mgradwohl merged 1 commit into
mainfrom
refactor/3116-chart-reference-helpers
Oct 27, 2026
Merged

mgradwohl merged 1 commit into
mainfrom
refactor/3116-chart-reference-helpers

Conversation

@mgradwohl

Copy link
Copy Markdown
Owner

Description

src/UI/ChartWidgets.h still carried four value-buffer helpers that nothing under src/, benchmarks/ or the fuzz targets calls. Only tests used them. This moves them, and the writeReducedPoint helper that only the first two used, into a test-support header. Their code is unchanged.

What moved where

  • reduceSeriesKeepingGaps, reduceSeriesMinMax, reduceAlignedSeries, holdLastValuesToNow and writeReducedPoint now live in tests/UI/ChartReferenceHelpers.h, namespace UI::Widgets::TestReference. The header's file comment says what they are and that nothing under src/ may include it.
  • Production keeps the shared primitives: forEachStrideReducedPoint, forEachMinMaxReducedPoint, forEachAlignedReducedPoint, reduceSeriesMinMaxPoints, reduceAlignedPoints, holdLastValueToNow, holdTimeToNow, appendHeldValues and the gather functions.

Tests

  • tests/UI/test_ChartWidgets.cpp includes the new header and pulls in the four helpers with using TestReference::.... Every existing test is still there and its assertions are unchanged. That includes perf(ui): chart sections keep their per-publication series and window maxima between samples, so a frame between samples rebuilds only x (#843) #2937's equivalence test (HeldValuesAndAHeldTimeDrawWhatHoldLastValuesToNowDoes), which still checks appendHeldValues + holdTimeToNow against holdLastValuesToNow, along with the stale, empty, mismatched-length and NaN hold cases and the peak, gap, end-point, budget and aligned-series tests.
  • tests/App/test_ProcessDetailsHistory.cpp: two tests held a series to now with holdLastValuesToNow. They now use the charts' own appendHeldValues + holdTimeToNow, with the same assertions. This is the path Process Details actually draws with, so the App test doesn't need a UI test header.
  • tests/UI/test_ChartWidgetsEdgeCases.cpp already used only reduceSeriesMinMaxPoints, so it is unchanged.

Docs and comments

  • The descriptions that were on the removed wrappers now sit on the production primitives:
  • Every other comment in ChartWidgets.h, ProcessDetailsCharts.cpp and SystemMetricsPanel.cpp that named a removed helper now names a production one.
  • Includes: none of ChartWidgets.h's includes was needed only by the moved helpers (<initializer_list>, <limits>, <algorithm>, <array> and <type_traits> all still have production users), so none was removed.

git grep finds no remaining reference to the moved names outside tests/UI/. Production behaviour does not change: only unused inline templates and functions left the header, plus comment edits.

Fixes #3116

PR Size

Small: about 230 lines added and 185 removed. Most of it is the moved code: about 170 lines out of ChartWidgets.h, about 157 into the new header.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Build/CI improvement

Refactor, no runtime change: test-only code moved out of a production header, plus comment updates.

Checklist

  • I have read the CONTRIBUTING guidelines
  • My code follows the project's coding standards
  • I have run clang-tidy and addressed any warnings (no local build: see Testing)
  • I have run clang-format on my changes
  • I have run pre-commit run --all-files or installed pre-commit hooks (pre-commit run --files on the touched files: all passed)
  • I have added tests that prove my fix/feature works (existing tests kept and moved to the reference header; no new behaviour to test)
  • All new and existing tests pass (CI is the first build)
  • I have updated documentation as needed

Testing

Not built locally. Free RAM stayed near 5.5 GB, under this machine's 8 GB build threshold, for the whole 15-minute wait, so CI is the first build and test run of this change. The suites to watch are ChartWidgetsTest, ChartWidgetsStackTest, ChartWidgetsReduceTest and the other suites in test_ChartWidgets.cpp and test_ChartWidgetsEdgeCases.cpp, plus ProcessDetailsHistoryTest.

git grep -n -E "reduceSeriesKeepingGaps|reduceSeriesMinMax\b|reduceAlignedSeries|holdLastValuesToNow|writeReducedPoint"   # only tests/UI/
clang-format -i <touched files>
pre-commit run --files <touched files>
cmake --workflow --preset win-dev   # left to CI

Additional Notes

  • tests/UI/ChartReferenceHelpers.h is reached as "UI/ChartReferenceHelpers.h" through the tests directory that is already on TaskSmackTests' include path, the same way "Core/WindowSizeTestUtils.h" is. It is header-only, so tests/CMakeLists.txt doesn't change.
  • Labels: agent:Windows, refactoring, ui, testing. Milestone v0.10.0, so no backport label.

🤖 Generated with Claude Code

…Widgets.h (#3116)

reduceSeriesKeepingGaps, reduceSeriesMinMax, reduceAlignedSeries and
holdLastValuesToNow, and writeReducedPoint behind the first two, had no
caller under src/, benchmarks/ or the fuzzers: only tests used them. They
move unchanged to tests/UI/ChartReferenceHelpers.h, in
UI::Widgets::TestReference, so test_ChartWidgets.cpp keeps its peak, gap,
alignment and hold-to-now tests and #2937's check of appendHeldValues +
holdTimeToNow against holdLastValuesToNow.

The two test_ProcessDetailsHistory.cpp tests that held a series to now
now use the charts' own appendHeldValues + holdTimeToNow, with the same
assertions. The descriptions the removed wrappers carried move to the
production primitives (forEachStrideReducedPoint,
forEachAlignedReducedPoint, holdTimeToNow, appendHeldValues), and the
comments that pointed at the removed names point at those. No include
of ChartWidgets.h was used only by the moved helpers. No production
behaviour changes.

Fixes #3116

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mgradwohl mgradwohl added this to the v0.10.0 milestone Oct 27, 2026
@mgradwohl mgradwohl added testing refactoring ui UI / rendering issues agent:Windows Owned by the Windows Claude session (bug-fix conveyor) labels Oct 27, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@mgradwohl
mgradwohl enabled auto-merge (squash) October 27, 2026 06:15
@mgradwohl
mgradwohl merged commit 0ab078f into main Oct 27, 2026
33 checks passed
@mgradwohl
mgradwohl deleted the refactor/3116-chart-reference-helpers branch October 27, 2026 06:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agent:Windows Owned by the Windows Claude session (bug-fix conveyor) refactoring testing ui UI / rendering issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Refactor] Move test-only chart reference helpers out of ChartWidgets.h without losing regression coverage

1 participant