From 61524a9bcd96e6ee842d79c08157635925e751d4 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 05:03:19 +0000 Subject: [PATCH] [patch] Fix NotImplementedException on the default render path TextElement and BorderElement declared explicit switch arms that threw NotImplementedException for enum members whose behaviour the `_` fallback immediately below already implemented. An explicit arm is matched before `_`, so the throw won. In every case the throwing member was the property's default value, so the failure was reached by the most ordinary use of the library: - TextElement.HorizontalAlignment defaults to Left, which threw. - TextElement.VerticalAlignment defaults to Top, which threw. - BorderElement.TitleAlignment defaults to Left, so any titled border threw. - BorderStyle.None threw instead of drawing no border. CalculateHorizontalPosition and CalculateVerticalPosition are called unconditionally from TextElement.OnRender, so this was the main render path rather than a rare branch. Rendering a TextElement with nothing but its text set threw. The three alignment arms now return what the fallback returned. BorderStyle.None gets a real implementation rather than the fallback's single-line border: OnRender returns before drawing, so no border and no title are drawn, while the base class still renders children and the element stays a usable container. Tests: the suite was green throughout because nothing called Render. There was no TextElementTests at all, and BorderElementTests covered only property round-tripping and invalidation. Adds RecordingConsoleProvider, an IConsoleProvider double that records WriteAt calls so tests can assert what was drawn and where, plus TextElementTests and BorderElementRenderTests covering the full alignment and border-style matrix via [DynamicData] over Enum.GetValues. Verified by mutation: reintroducing the three throws fails 21 of the 113 tests. Fixes #102 Fixes #103 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01DTHNXgSNEHUSQ5KMLgivno --- CLAUDE.md | 32 +++ TUI.Core/Elements/Primitives/BorderElement.cs | 10 +- TUI.Core/Elements/Primitives/TextElement.cs | 4 +- TUI.Test/BorderElementRenderTests.cs | 209 ++++++++++++++ TUI.Test/RecordingConsoleProvider.cs | 80 ++++++ TUI.Test/TextElementTests.cs | 255 ++++++++++++++++++ 6 files changed, 586 insertions(+), 4 deletions(-) create mode 100644 TUI.Test/BorderElementRenderTests.cs create mode 100644 TUI.Test/RecordingConsoleProvider.cs create mode 100644 TUI.Test/TextElementTests.cs diff --git a/CLAUDE.md b/CLAUDE.md index 02502d8..4fd8961 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -83,6 +83,38 @@ var mockChild = new Mock(); mockChild.Setup(c => c.IsVisible).Returns(true); ``` +### Render tests + +Property round-tripping is not enough on its own. Every alignment combination of `TextElement`, +every titled `BorderElement` with the default `TitleAlignment`, and every `BorderStyle.None` +element once threw `NotImplementedException` from the render path, and the suite stayed green +throughout because no test called `Render`. + +`RecordingConsoleProvider` (in `TUI.Test`) is the test double for this. It implements +`IConsoleProvider` and records every `WriteAt` call rather than drawing, so a test can assert +*what* was drawn and *where*: + +```csharp +RecordingConsoleProvider provider = new(); +element.Render(provider); +Assert.AreEqual(0, provider.WritesOf("abc").Single().Position.X); +``` + +Rendering to the real `SpectreConsoleProvider` in a test writes to the runner's console and +leaves the output unobservable, so use the recorder instead. + +Any new element needs render coverage across the full matrix of whatever enum drives its +layout — that is exactly where the bug above lived. `TextElementTests` and +`BorderElementRenderTests` use `[DynamicData]` over `Enum.GetValues()` so a newly added +enum member is covered automatically rather than silently skipped. + +Two sizing traps when writing these: + +- `Dimensions.WithoutPadding` floors at zero, and an element with an empty content area returns + from `OnRender` before drawing. An element must be larger than its own padding or it draws + nothing and the assertion fails for a reason unrelated to what is under test. +- `BorderElement` only draws its title when `Width > 4`, and draws no border at all below 2x2. + ## File Headers All source files require this copyright header: diff --git a/TUI.Core/Elements/Primitives/BorderElement.cs b/TUI.Core/Elements/Primitives/BorderElement.cs index e91a21d..8931703 100644 --- a/TUI.Core/Elements/Primitives/BorderElement.cs +++ b/TUI.Core/Elements/Primitives/BorderElement.cs @@ -116,6 +116,13 @@ protected override void OnRender(IConsoleProvider provider) return; } + // BorderStyle.None means draw no border and no title. Children are still rendered by + // the base class after this method returns, so the element remains a usable container. + if (BorderStyle == BorderStyle.None) + { + return; + } + BorderCharacters chars = GetBorderCharacters(BorderStyle); // Draw corners @@ -150,7 +157,7 @@ protected override void OnRender(IConsoleProvider provider) { HorizontalAlignment.Center => position.X + Math.Max(1, (dimensions.Width - titleWithPadding.Length) / 2), HorizontalAlignment.Right => position.X + Math.Max(1, dimensions.Width - titleWithPadding.Length - 1), - HorizontalAlignment.Left => throw new NotImplementedException(), + HorizontalAlignment.Left => position.X + 1, _ => position.X + 1 }; @@ -197,7 +204,6 @@ private static BorderCharacters GetBorderCharacters(BorderStyle style) BorderStyle.Thick => new BorderCharacters('┏', '┓', '┗', '┛', '━', '┃'), BorderStyle.Ascii => new BorderCharacters('+', '+', '+', '+', '-', '|'), BorderStyle.SingleLine => new BorderCharacters('┌', '┐', '└', '┘', '─', '│'), - BorderStyle.None => throw new NotImplementedException(), _ => new BorderCharacters('┌', '┐', '└', '┘', '─', '│') // SingleLine }; } diff --git a/TUI.Core/Elements/Primitives/TextElement.cs b/TUI.Core/Elements/Primitives/TextElement.cs index 0e519b1..b2a8ae4 100644 --- a/TUI.Core/Elements/Primitives/TextElement.cs +++ b/TUI.Core/Elements/Primitives/TextElement.cs @@ -142,7 +142,7 @@ private int CalculateHorizontalPosition(string line, int availableWidth, int bas { HorizontalAlignment.Center => baseX + Math.Max(0, (availableWidth - line.Length) / 2), HorizontalAlignment.Right => baseX + Math.Max(0, availableWidth - line.Length), - HorizontalAlignment.Left => throw new NotImplementedException(), + HorizontalAlignment.Left => baseX, _ => baseX }; } @@ -153,7 +153,7 @@ private int CalculateVerticalPosition(int totalLines, int availableHeight, int b { VerticalAlignment.Center => baseY + Math.Max(0, (availableHeight - totalLines) / 2), VerticalAlignment.Bottom => baseY + Math.Max(0, availableHeight - totalLines), - VerticalAlignment.Top => throw new NotImplementedException(), + VerticalAlignment.Top => baseY, _ => baseY }; } diff --git a/TUI.Test/BorderElementRenderTests.cs b/TUI.Test/BorderElementRenderTests.cs new file mode 100644 index 0000000..bd91f53 --- /dev/null +++ b/TUI.Test/BorderElementRenderTests.cs @@ -0,0 +1,209 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.TUI.Test; + +using ktsu.TUI.Core.Elements.Primitives; +using ktsu.TUI.Core.Models; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +/// +/// Render-path tests for . +/// +/// +/// covers property round-tripping and invalidation. This class +/// covers what the element actually draws, which is where ktsu-dev/TUI#102 lived: a titled +/// border with the default , and every +/// element, threw . +/// +[TestClass] +public sealed class BorderElementRenderTests +{ + private static BorderElement CreateElement(int width = 30, int height = 5) + { + BorderElement element = []; + element.Position = Position.Origin; + element.Dimensions = new Dimensions(width, height); + return element; + } + + /// + /// A titled border with default alignment must render. This is the case that used to throw. + /// + [TestMethod] + public void RenderTitledBorderWithDefaultAlignmentDrawsTheTitle() + { + // Arrange + BorderElement element = CreateElement(); + element.Title = "Title"; + RecordingConsoleProvider provider = new(); + + // Act + element.Render(provider); + + // Assert + Assert.AreEqual(HorizontalAlignment.Left, element.TitleAlignment, "Left must remain the default"); + Assert.ContainsSingle(provider.WritesOf(" Title ")); + } + + /// + /// Every border style and title alignment pairing must render rather than throw. + /// + /// The border style under test. + /// The title alignment under test. + [TestMethod] + [DynamicData(nameof(StyleAlignmentCombinations))] + public void RenderSucceedsForEveryStyleAndTitleAlignment(BorderStyle style, HorizontalAlignment alignment) + { + // Arrange + BorderElement element = CreateElement(); + element.BorderStyle = style; + element.Title = "Title"; + element.TitleAlignment = alignment; + RecordingConsoleProvider provider = new(); + + // Act + element.Render(provider); + + // Assert + int expectedTitles = style == BorderStyle.None ? 0 : 1; + Assert.HasCount(expectedTitles, provider.WritesOf(" Title ").ToList()); + } + + /// + /// Gets every border style and title alignment pairing. + /// + public static IEnumerable StyleAlignmentCombinations => + from style in Enum.GetValues() + from alignment in Enum.GetValues() + select new object[] { style, alignment }; + + /// + /// must draw nothing rather than throwing or drawing a + /// single-line border, which is what the fallback arm would otherwise have produced. + /// + [TestMethod] + public void BorderStyleNoneDrawsNothing() + { + // Arrange + BorderElement element = CreateElement(); + element.BorderStyle = BorderStyle.None; + element.Title = "Title"; + RecordingConsoleProvider provider = new(); + + // Act + element.Render(provider); + + // Assert + Assert.IsEmpty(provider.Writes); + } + + /// + /// must still render children, so it remains a usable container. + /// + [TestMethod] + public void BorderStyleNoneStillRendersChildren() + { + // Arrange + BorderElement element = CreateElement(); + element.BorderStyle = BorderStyle.None; + element.Child = new TextElement("child") + { + Position = Position.Origin, + Dimensions = new Dimensions(20, 1), + }; + RecordingConsoleProvider provider = new(); + + // Act + element.Render(provider); + + // Assert + Assert.ContainsSingle(provider.WritesOf("child")); + } + + /// + /// Each visible border style must draw its own corner glyph. + /// + /// The border style under test. + /// The corner glyph that style is expected to draw. + [TestMethod] + [DataRow(BorderStyle.SingleLine, "┌")] + [DataRow(BorderStyle.DoubleLine, "╔")] + [DataRow(BorderStyle.Rounded, "╭")] + [DataRow(BorderStyle.Thick, "┏")] + [DataRow(BorderStyle.Ascii, "+")] + public void EachStyleDrawsItsOwnCornerGlyph(BorderStyle style, string expectedTopLeft) + { + // Arrange + BorderElement element = CreateElement(); + element.BorderStyle = style; + RecordingConsoleProvider provider = new(); + + // Act + element.Render(provider); + + // Assert + Assert.AreEqual(expectedTopLeft, provider.Writes[0].Text); + Assert.AreEqual(Position.Origin, provider.Writes[0].Position); + } + + /// + /// Left title alignment must sit left of centre alignment, which must sit left of right. + /// + [TestMethod] + public void TitleAlignmentOrdersLeftThenCenterThenRight() + { + // Arrange & Act + static int Render(HorizontalAlignment alignment) + { + BorderElement element = CreateElement(width: 40); + element.Title = "Title"; + element.TitleAlignment = alignment; + RecordingConsoleProvider provider = new(); + element.Render(provider); + return provider.WritesOf(" Title ").Single().Position.X; + } + + int left = Render(HorizontalAlignment.Left); + int center = Render(HorizontalAlignment.Center); + int right = Render(HorizontalAlignment.Right); + + // Assert + Assert.IsGreaterThan(left, center); + Assert.IsGreaterThan(center, right); + } + + /// + /// A border smaller than 2x2 has no room to draw and must draw nothing. + /// + [TestMethod] + public void BorderSmallerThanTwoByTwoDrawsNothing() + { + // Arrange + BorderElement element = CreateElement(width: 1, height: 1); + RecordingConsoleProvider provider = new(); + + // Act + element.Render(provider); + + // Assert + Assert.IsEmpty(provider.Writes); + } + + /// + /// The title is only drawn once the border is wide enough to hold it. + /// + [TestMethod] + public void TitleIsSuppressedWhenTheBorderIsTooNarrow() + { + // Arrange + BorderElement element = CreateElement(width: 4, height: 3); + element.Title = "Title"; + RecordingConsoleProvider provider = new(); + + // Act + element.Render(provider); + + // Assert + Assert.IsEmpty(provider.WritesOf(" Title ")); + } +} diff --git a/TUI.Test/RecordingConsoleProvider.cs b/TUI.Test/RecordingConsoleProvider.cs new file mode 100644 index 0000000..71b2cd9 --- /dev/null +++ b/TUI.Test/RecordingConsoleProvider.cs @@ -0,0 +1,80 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.TUI.Test; + +using System.Collections.ObjectModel; + +using ktsu.TUI.Core.Contracts; +using ktsu.TUI.Core.Models; + +/// +/// An test double that records every call +/// instead of drawing, so a test can assert what an element drew and where it drew it. +/// +/// +/// Rendering to the real SpectreConsoleProvider would write to the test runner's console +/// and would make the drawn output unobservable. Recording the calls keeps render tests +/// headless and lets them assert on positions rather than only on "did not throw". +/// +internal sealed class RecordingConsoleProvider : IConsoleProvider +{ + /// + /// One recorded call. + /// + /// The text that was written. + /// The position it was written at. + /// The style it was written with, if any. + internal sealed record Write(string Text, Position Position, TextStyle? Style); + + private readonly Collection writes = []; + + /// + /// Gets every call recorded so far, in call order. + /// + internal IReadOnlyList Writes => writes; + + /// + /// Gets the number of times was called. + /// + internal int ClearCount { get; private set; } + + /// + public Dimensions Dimensions { get; set; } = new(80, 24); + + /// + public void Clear() => ClearCount++; + + /// + public void Render(IUIElement element, Position position) => element?.Render(this); + + /// + public void WriteAt(string text, Position position, TextStyle? style = null) => + writes.Add(new Write(text, position, style)); + + /// + public Task ReadInputAsync() => Task.FromResult(new InputResult()); + + /// + public void SetCursorVisibility(bool visible) => CursorVisible = visible; + + /// + public void SetCursorPosition(Position position) => CursorPosition = position; + + /// + /// Gets the last cursor visibility set through . + /// + internal bool CursorVisible { get; private set; } = true; + + /// + /// Gets the last cursor position set through . + /// + internal Position CursorPosition { get; private set; } + + /// + /// Returns every recorded write whose text is exactly . + /// + /// The text to match. + /// The matching writes, in call order. + internal IEnumerable WritesOf(string text) => + writes.Where(w => string.Equals(w.Text, text, StringComparison.Ordinal)); +} diff --git a/TUI.Test/TextElementTests.cs b/TUI.Test/TextElementTests.cs new file mode 100644 index 0000000..a04cbae --- /dev/null +++ b/TUI.Test/TextElementTests.cs @@ -0,0 +1,255 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.TUI.Test; + +using ktsu.TUI.Core.Elements.Primitives; +using ktsu.TUI.Core.Models; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +/// +/// Tests for , covering the render path as well as property behaviour. +/// +/// +/// The render cases exist because every alignment combination once threw +/// from the default configuration +/// (ktsu-dev/TUI#102), and the suite was green throughout because nothing called +/// Render. +/// +[TestClass] +public sealed class TextElementTests +{ + private static TextElement CreateElement(string text, int width = 40, int height = 5) + { + return new TextElement + { + Text = text, + Position = Position.Origin, + Dimensions = new Dimensions(width, height), + }; + } + + /// + /// The default configuration must render. This is the exact case that used to throw. + /// + [TestMethod] + public void RenderWithDefaultAlignmentDrawsTheText() + { + // Arrange + TextElement element = CreateElement("hello world"); + RecordingConsoleProvider provider = new(); + + // Act + element.Render(provider); + + // Assert + Assert.AreEqual(HorizontalAlignment.Left, element.HorizontalAlignment, "Left must remain the default"); + Assert.AreEqual(VerticalAlignment.Top, element.VerticalAlignment, "Top must remain the default"); + Assert.ContainsSingle(provider.WritesOf("hello world")); + } + + /// + /// Every alignment combination must render rather than throw. + /// + /// The horizontal alignment under test. + /// The vertical alignment under test. + [TestMethod] + [DynamicData(nameof(AlignmentCombinations))] + public void RenderSucceedsForEveryAlignmentCombination(HorizontalAlignment horizontal, VerticalAlignment vertical) + { + // Arrange + TextElement element = CreateElement("x", width: 20, height: 4); + element.HorizontalAlignment = horizontal; + element.VerticalAlignment = vertical; + RecordingConsoleProvider provider = new(); + + // Act + element.Render(provider); + + // Assert + Assert.ContainsSingle(provider.WritesOf("x")); + } + + /// + /// Gets every horizontal/vertical alignment pairing. + /// + public static IEnumerable AlignmentCombinations => + from horizontal in Enum.GetValues() + from vertical in Enum.GetValues() + select new object[] { horizontal, vertical }; + + /// + /// Left alignment must place text at the content origin, not merely avoid throwing. + /// + [TestMethod] + public void LeftAlignmentPlacesTextAtTheContentOrigin() + { + // Arrange + TextElement element = CreateElement("abc", width: 20, height: 1); + element.HorizontalAlignment = HorizontalAlignment.Left; + RecordingConsoleProvider provider = new(); + + // Act + element.Render(provider); + + // Assert + Assert.AreEqual(0, provider.WritesOf("abc").Single().Position.X); + } + + /// + /// Top alignment must place the first line at the content origin. + /// + [TestMethod] + public void TopAlignmentPlacesTextAtTheContentOrigin() + { + // Arrange + TextElement element = CreateElement("abc", width: 20, height: 5); + element.VerticalAlignment = VerticalAlignment.Top; + RecordingConsoleProvider provider = new(); + + // Act + element.Render(provider); + + // Assert + Assert.AreEqual(0, provider.WritesOf("abc").Single().Position.Y); + } + + /// + /// Centre alignment must shift the text right of the left-aligned position. + /// + [TestMethod] + public void CenterAlignmentShiftsTextRightOfLeftAlignment() + { + // Arrange + RecordingConsoleProvider left = new(); + RecordingConsoleProvider center = new(); + + TextElement leftElement = CreateElement("abc", width: 21, height: 1); + leftElement.HorizontalAlignment = HorizontalAlignment.Left; + + TextElement centerElement = CreateElement("abc", width: 21, height: 1); + centerElement.HorizontalAlignment = HorizontalAlignment.Center; + + // Act + leftElement.Render(left); + centerElement.Render(center); + + // Assert + Assert.IsGreaterThan( + left.WritesOf("abc").Single().Position.X, + center.WritesOf("abc").Single().Position.X); + } + + /// + /// Right alignment must place the text flush against the right edge of the content area. + /// + [TestMethod] + public void RightAlignmentPlacesTextAgainstTheRightEdge() + { + // Arrange + TextElement element = CreateElement("abc", width: 20, height: 1); + element.HorizontalAlignment = HorizontalAlignment.Right; + RecordingConsoleProvider provider = new(); + + // Act + element.Render(provider); + + // Assert + Assert.AreEqual(20 - 3, provider.WritesOf("abc").Single().Position.X); + } + + /// + /// Empty text must draw nothing at all. + /// + [TestMethod] + public void RenderWithEmptyTextDrawsNothing() + { + // Arrange + TextElement element = CreateElement(string.Empty); + RecordingConsoleProvider provider = new(); + + // Act + element.Render(provider); + + // Assert + Assert.IsEmpty(provider.Writes); + } + + /// + /// Padding must inset the drawn text on both axes. + /// + [TestMethod] + public void PaddingInsetsTheDrawnText() + { + // Arrange + // Height must exceed the 6 rows of vertical padding, or the content area is empty + // and TextElement returns before drawing anything. + TextElement element = CreateElement("abc", width: 20, height: 10); + element.Padding = new Padding(2, 3, 2, 3); + RecordingConsoleProvider provider = new(); + + // Act + element.Render(provider); + + // Assert + RecordingConsoleProvider.Write write = provider.WritesOf("abc").Single(); + Assert.AreEqual(2, write.Position.X); + Assert.AreEqual(3, write.Position.Y); + } + + /// + /// Word wrapping must split long text across several lines. + /// + [TestMethod] + public void WordWrapSplitsTextAcrossLines() + { + // Arrange + TextElement element = CreateElement("aaa bbb ccc ddd", width: 7, height: 5); + element.WordWrap = true; + RecordingConsoleProvider provider = new(); + + // Act + element.Render(provider); + + // Assert + Assert.IsGreaterThan(1, provider.Writes.Count, "Wrapped text should produce more than one line"); + Assert.IsTrue( + provider.Writes.All(w => w.Text.Length <= 7), + "No wrapped line should exceed the content width"); + } + + /// + /// An element that is not visible must draw nothing. + /// + [TestMethod] + public void InvisibleElementDrawsNothing() + { + // Arrange + TextElement element = CreateElement("hello"); + element.IsVisible = false; + RecordingConsoleProvider provider = new(); + + // Act + element.Render(provider); + + // Assert + Assert.IsEmpty(provider.Writes); + } + + /// + /// Text must not be drawn beyond the available height. + /// + [TestMethod] + public void RenderDoesNotDrawMoreLinesThanTheContentHeight() + { + // Arrange + TextElement element = CreateElement("aaa bbb ccc ddd eee fff", width: 5, height: 2); + element.WordWrap = true; + RecordingConsoleProvider provider = new(); + + // Act + element.Render(provider); + + // Assert + Assert.IsLessThanOrEqualTo(2, provider.Writes.Count); + } +}