Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -540,6 +540,36 @@ records its tabs nor takes a selection from outside. A panel behind it - the cla
diagnostics list - is tested by drawing it directly in a `WidgetHarness`, which is what the tab's
own delegate does.

### What a Linux-only suite stops seeing

The frame-driven suite runs on Linux alone, and the two tests that first failed elsewhere were
failing for reasons that had nothing to do with the operating system - which is exactly what a
one-platform suite is bad at telling you.

`FileBrowserTests` expected the path it made its scratch directory under. macOS reaches the
temporary directory through `/var`, a symbolic link to `/private/var`, and the working directory
resolves the link, so the browser answered with the other spelling of the same directory. The test
now reads the working directory back after setting it, which is a fact about symbolic links rather
than about macOS.

`MenuTests` clicked a recent file whose menu label was its whole path. A menu is as wide as its
widest label, and a submenu that will not fit beside the menu that opened it is placed **on top of
that menu** - which puts the item that opened it under the submenu rather than under the pointer,
so ImGui takes the pointer to have left and closes the submenu on the frame after it opens. The
temporary directory being 44 characters deeper on macOS is all that picked which platform noticed:
any schema saved somewhere deep enough did the same everywhere, and Open Recent was unusable for
whoever owned it. `SchemaEditor.ElideRecentFileLabel` bounds the label and the tooltip keeps the
path. Both halves are pinned - the mechanism in `MenuTests.ARecentFileTooDeepToLabelWholeStillOpens`,
and the elision itself in `Schema.Editor.Test/RecentFileLabelTests.cs`, which needs no frame and so
runs on every platform. That is the answer to promoting the suite: what was platform-sensitive here
was a string, and a string can be tested where the rasterizer cannot go.

`EditorHarness.Click` asked whether the probe had **ever** recorded a name, not whether the item was
on screen now, so an item drawn once and then gone still satisfied the wait and the click failed
later on a stale rectangle. It now asks `IsOnScreen` on both sides of the settle frames. That is a
better message, not a fix: with the wrong question it failed inside the click, with the right one it
fails at the wait, and only the elision makes it pass.

## Dependencies

- **ktsu.Semantics.Strings/Paths** - Type-safe string and path wrappers
Expand Down
68 changes: 68 additions & 0 deletions Schema.Editor.Test/RecentFileLabelTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
// Copyright (c) 2023-2026 ktsu-dev contributors

namespace ktsu.Schema.Editor.Test;

/// <summary>
/// The label a recent file is offered under in the File menu.
/// </summary>
/// <remarks>
/// <para>
/// Offering the whole path was not a display preference that happened to be verbose. A menu is as
/// wide as its widest label, and a submenu that will not fit beside the menu that opened it is
/// placed on top of that menu instead - which puts the item that opened it underneath the submenu
/// rather than under the pointer, so ImGui takes the pointer to have left and closes the submenu
/// on the frame after it opened. Open Recent was therefore unusable for anyone whose schemas lived
/// deep enough, and how deep that was depended on the window's width.
/// </para>
/// <para>
/// Frameless, so it runs on every platform. That matters here more than usual: this was found by
/// two tests failing on macOS alone, for no reason macOS was responsible for - its temporary
/// directory is simply 44 characters deeper than <c>/tmp</c>.
/// </para>
/// </remarks>
[TestClass]
public sealed class RecentFileLabelTests
{
[TestMethod]
public void APathThatFitsIsOfferedWhole()
{
string path = new('x', SchemaEditor.MaxRecentFileLabelLength);

Assert.AreEqual(path, SchemaEditor.ElideRecentFileLabel(path));
}

[TestMethod]
public void ALongerPathIsCutToTheSameWidth()
{
string label = SchemaEditor.ElideRecentFileLabel(new string('x', SchemaEditor.MaxRecentFileLabelLength * 4));

Assert.AreEqual(SchemaEditor.MaxRecentFileLabelLength, label.Length, "A label past the budget is what makes the menu wider than the window.");
}

/// <summary>
/// The end rather than the beginning: what tells two recent files apart is the file name and
/// the directory holding it, and the root they share is the part worth losing.
/// </summary>
[TestMethod]
public void WhatIsKeptIsTheEndOfThePath()
{
string path = $"/a/very/long/root/that/nobody/needs/to/read/again/and/again/and/again/{new string('d', 20)}/schema.json";

string label = SchemaEditor.ElideRecentFileLabel(path);

StringAssert.EndsWith(label, $"{new string('d', 20)}/schema.json", StringComparison.Ordinal);

Check warning on line 53 in Schema.Editor.Test/RecentFileLabelTests.cs

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use 'Assert.EndsWith' instead of 'StringAssert.EndsWith'

See more on https://sonarcloud.io/project/issues?id=ktsu-dev_Schema&issues=AaCf2hsEjLkBUbkKwd0t&open=AaCf2hsEjLkBUbkKwd0t&pullRequest=195
StringAssert.StartsWith(label, "…", StringComparison.Ordinal);

Check warning on line 54 in Schema.Editor.Test/RecentFileLabelTests.cs

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use 'Assert.StartsWith' instead of 'StringAssert.StartsWith'

See more on https://sonarcloud.io/project/issues?id=ktsu-dev_Schema&issues=AaCf2hsEjLkBUbkKwd0u&open=AaCf2hsEjLkBUbkKwd0u&pullRequest=195
}

/// <summary>
/// A file name longer than the whole budget still leaves a label of the right width, rather
/// than a negative slice or a label that is once again as wide as the path.
/// </summary>
[TestMethod]
public void AFileNameLongerThanTheBudgetIsCutToo()
{
string path = $"/schemas/{new string('n', SchemaEditor.MaxRecentFileLabelLength * 2)}.schema.json";

Assert.AreEqual(SchemaEditor.MaxRecentFileLabelLength, SchemaEditor.ElideRecentFileLabel(path).Length);
}
}
49 changes: 48 additions & 1 deletion Schema.Editor/SchemaEditor.Files.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,42 @@ public partial class SchemaEditor
? "Untitled schema"
: Path.GetFileName(CurrentSchemaPath);

/// <summary>
/// The longest label a recent file is offered under, in characters.
/// </summary>
/// <remarks>
/// <para>
/// A menu is as wide as its widest label, and Open Recent's labels are absolute paths, so its
/// width was whatever depth the user happened to have saved at. That is not merely untidy: a
/// submenu that does not fit beside the menu that opened it is placed on top of it, and the
/// item that opened it is then underneath the submenu rather than under the pointer, which
/// ImGui reads as the pointer having left - so the submenu closes on the frame after it opens
/// and the menu cannot be used at all.
/// </para>
/// <para>
/// Forty is measured rather than chosen for looks: the submenu opens a little past the File
/// menu's own width and grows by about eight pixels a character, so forty leaves it inside the
/// 700-pixel window the narrowest of these tests drives, where sixty does not. The whole path
/// is still a hover away.
/// </para>
/// </remarks>
internal const int MaxRecentFileLabelLength = 40;

/// <summary>
/// Shortens a path to <see cref="MaxRecentFileLabelLength"/> for display, keeping its end.
/// </summary>
/// <remarks>
/// The end rather than the beginning, because what tells two recent files apart is the file
/// name and the directory holding it. What is dropped is the root they are most likely to
/// share.
/// </remarks>
/// <param name="path">The path to label.</param>
/// <returns>The path itself if it fits, or its last characters behind an ellipsis.</returns>
internal static string ElideRecentFileLabel(string path) =>
path.Length <= MaxRecentFileLabelLength
? path
: $"…{path[^(MaxRecentFileLabelLength - 1)..]}";

private void ShowRecentFilesMenu()
{
IReadOnlyList<AbsoluteFilePath> recent = [.. Options.RecentFiles];
Expand All @@ -58,12 +94,23 @@ private void ShowRecentFilesMenu()
}

anyShown = true;
bool clicked = ImGui.MenuItem(path);

// The whole path is the id and the elided path is the label, so two files with the
// same name stay separate items however alike they read.
ImGui.PushID(path);
bool clicked = ImGui.MenuItem(ElideRecentFileLabel(path));

// Recorded under the file name rather than the whole path, which a probe name would
// otherwise read as a chain of scopes because both are separated by slashes.
ImGuiProbes.MarkItem("recent", Path.GetFileName(path));

if (ImGui.IsItemHovered())
{
ImGui.SetTooltip(path);
}

ImGui.PopID();

if (clicked)
{
AbsoluteFilePath captured = path;
Expand Down
13 changes: 12 additions & 1 deletion tests/Schema.Editor.UITests/EditorHarness.cs
Original file line number Diff line number Diff line change
Expand Up @@ -100,16 +100,27 @@ internal void StepUntil(Func<bool> condition, string description, int maxFrames
/// Waits for a marked item to be drawn, then clicks it.
/// </summary>
/// <remarks>
/// <para>
/// The frames between the item first appearing and the click are not padding. A modal sizes
/// itself from its contents on the frame it appears and is centred on the next, so the
/// rectangle recorded for a control on its first frame is not where that control ends up;
/// clicking there hits the background instead.
/// </para>
/// <para>
/// Both waits ask <see cref="IsOnScreen(string)"/> rather than whether the probe has ever
/// recorded the name, and the second one is why there are two: an item can be drawn once and
/// then go away again while those frames run, and the name it left behind is enough to satisfy
/// a wait that only asks whether the probe has heard of it. Asked this way, a test that clicks
/// something no longer there says so, instead of failing inside the click on a rectangle that
/// has since been taken by whatever moved into it.
/// </para>
/// </remarks>
/// <param name="item">A marked name, or the trailing part of one.</param>
internal void Click(string item)
{
StepUntil(() => App.Probe.Matches(item).Count > 0, $"'{item}' appearing");
StepUntil(() => IsOnScreen(item), $"'{item}' appearing");
App.Step(3);
StepUntil(() => IsOnScreen(item), $"'{item}' still being drawn once it had settled");
App.Click(item);
App.Step(2);
}
Expand Down
6 changes: 6 additions & 0 deletions tests/Schema.Editor.UITests/FileBrowserTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,12 @@ public void StartEditor()
previousWorkingDirectory = Directory.GetCurrentDirectory();
Directory.SetCurrentDirectory(scratchDirectory);

// Read the directory back rather than keeping the name it was created under. On macOS the
// temporary directory is reached through /var, which is a symbolic link to /private/var,
// and the working directory resolves the link - so the browser, which opens on the working
// directory, answers with the spelling that the name it was created under is not.
scratchDirectory = Directory.GetCurrentDirectory().As<AbsoluteDirectoryPath>();

harness = EditorHarness.Start();
}

Expand Down
38 changes: 38 additions & 0 deletions tests/Schema.Editor.UITests/MenuTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,44 @@ public void OpeningARecentFileLoadsIt()
Assert.AreEqual("Recalled", harness.Editor.CurrentClass?.Name.ToString());
}

/// <summary>
/// A schema saved somewhere deep is still offered and still opens.
/// </summary>
/// <remarks>
/// Labelled with its whole path, such a file made Open Recent wider than the room beside the
/// File menu, and a submenu that does not fit beside the menu that opened it is placed on top
/// of it - so the item that opened it sat under the submenu rather than under the pointer,
/// ImGui took the pointer to have left, and the submenu closed on the frame after it opened.
/// This is the case that found it, in the only form that says what it was: the two tests it
/// broke failed on macOS alone, whose temporary directory is 44 characters deeper than
/// <c>/tmp</c> and which had nothing else to do with it.
/// </remarks>
[TestMethod]
public void ARecentFileTooDeepToLabelWholeStillOpens()
{
AbsoluteDirectoryPath deep = scratchDirectory;
for (int level = 0; level < 3; level++)
{
deep /= $"a-directory-named-at-length-{level}".As<DirectoryName>();
}

Directory.CreateDirectory(deep);

Schema source = new();
source.AddClass("Deep".As<ClassName>());
AbsoluteFilePath path = deep / "deep.schema.json".As<FileName>();
File.WriteAllText(path, SchemaSerializer.Serialize(source));
harness.Editor.Options.RecordRecentFile(path);

Assert.IsGreaterThan(SchemaEditor.MaxRecentFileLabelLength, path.ToString().Length, "The path was not long enough to be the case this pins.");

harness.ChooseMenuItem("File", "Open Recent");
harness.Click("recent/deep.schema.json");

Assert.AreEqual(path, harness.Editor.CurrentSchemaPath);
Assert.AreEqual("Deep", harness.Editor.CurrentClass?.Name.ToString());
}

[TestMethod]
public void UndoFromTheEditMenuRevertsTheLastEdit()
{
Expand Down