From c29877290f6fbae8bf337bebb10fd6d935d219c0 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 04:11:47 +0000 Subject: [PATCH 1/5] Fix the build: drop SchemaEditor's unused Microsoft.CodeAnalysis.CSharp reference The .NET workflow has been failing on main with three NU1608 errors. SchemaEditor pulls Hexa.NET.ImGui.Widgets.Extras transitively, which pins Microsoft.CodeAnalysis.CSharp.Scripting 4.14.0, and that in turn pins Microsoft.CodeAnalysis.Common and .CSharp to exactly 4.14.0. The project also names Microsoft.CodeAnalysis.CSharp directly, which central package management resolves at 5.9.0, and NuGet rejects the conflict as an error because warnings are errors here. Nothing in SchemaEditor uses Roslyn: the only "CodeAnalysis" in its source is System.Diagnostics.CodeAnalysis, which is in the base class library. The reference is removed rather than downgraded, so the transitive constraint resolves on its own. Schema.Test keeps its own reference at 5.9.0 - it genuinely compiles generated source - and has no ImGui dependency to conflict with. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Jc8o5zF3cmfCGjzWvQdGDE --- SchemaEditor/SchemaEditor.csproj | 1 - 1 file changed, 1 deletion(-) diff --git a/SchemaEditor/SchemaEditor.csproj b/SchemaEditor/SchemaEditor.csproj index 0644d22..ac4d5e4 100644 --- a/SchemaEditor/SchemaEditor.csproj +++ b/SchemaEditor/SchemaEditor.csproj @@ -24,7 +24,6 @@ - From ff3b81c6e360f69ecbf1e19d08ff1e65b1bdbfdf Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 04:12:06 +0000 Subject: [PATCH 2/5] Add a headless test harness for SchemaEditor, and cover its logic (#128) SchemaEditor had never had a test. Every line of it is either an ImGui draw call or logic interleaved with one, so none of it ran without a live ImGui context - which is why #125 landed at 19% coverage of new code and the fix was to exclude the whole application from Sonar's coverage measurement. ktsu.ImGui.App.Testing supplies that context with no window, no display and no GPU: it rasterizes in software and injects input straight into ImGui's event queue. So the editor's real draw code now runs on a headless CI runner, and disturbs nothing on a desktop. Two fixtures wrap it. EditorHarness starts a real editor from Program.CreateConfig - the same configuration the application itself starts with, so a callback renamed or dropped there breaks the tests rather than leaving them passing against a host that no longer exists - after redirecting AppDataStorage to an in-memory file system, so a test never reads or writes the settings of whoever is running it. WidgetHarness draws one widget and nothing else. 46 tests cover what the issue listed: - RecordRecentFile: ordering, deduplication (including a list that already held duplicates), bounding, and an empty path. - EditField: that an editing session is one write however many frames it spans, that leaving a field untouched or abandoning an edit with Escape writes nothing, that an edit spanning frames keeps every character, and that a field not being edited follows the model so an undo is not written back over. - The unsaved-changes guard: that an unmodified document is discarded without asking, a modified one is not, and that Save, Discard and Cancel each do what they say - driven by clicking the prompt in a rendered frame, since the sequencing being tested is what those callbacks do. Also that a save with no path defers to the file browser rather than running the continuation, that a failed save does not run it, and that closing is refused while work would be lost without stacking a prompt per attempt. - Diagnostics: that validation waits for the schema to settle, that each edit restarts the debounce, that it does not re-run unprompted, and that clicking an issue selects the right element for every element kind. TwoRowsSharingALabelDoNotShareABuffer is the one that needed pixels. ImGui keeps its own copy of the text while a widget is being edited, so with the buffers wrongly shared the row being typed into still commits the right value - the row that is wrong is the other one, handed its sibling's text to draw. The test compares that row's pixels before and during the edit; reintroducing the label-keyed buffer fails it, and nothing else in the suite notices. Production changes are confined to what the tests need to reach: Program.CreateConfig is split out of Main, and a handful of members on SchemaEditor widen from private to internal. Both InternalsVisibleTo names are given on each non-test project, since there are two test assemblies now and KTSU0002 is not checking which one reads what. The Sonar coverage exclusion narrows from SchemaEditor/**/*.cs to the panel and tree files, which are still pure draw code that this harness does not reach yet. Docs and the roadmap are updated to match, including Phase 2, which still described #116 as blocked after it landed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Jc8o5zF3cmfCGjzWvQdGDE --- .github/workflows/dotnet.yml | 13 +- CLAUDE.md | 6 +- Directory.Packages.props | 2 + README.md | 4 +- Schema.sln | 14 + Schema/AssemblyInfo.cs | 5 + SchemaEditor.Test/AssemblyInfo.cs | 5 + SchemaEditor.Test/DiagnosticsTests.cs | 268 +++++++++++++++++ SchemaEditor.Test/DocumentGuardTests.cs | 331 +++++++++++++++++++++ SchemaEditor.Test/EditFieldTests.cs | 262 ++++++++++++++++ SchemaEditor.Test/EditorHarness.cs | 98 ++++++ SchemaEditor.Test/HarnessSmokeTests.cs | 32 ++ SchemaEditor.Test/RecentFilesTests.cs | 103 +++++++ SchemaEditor.Test/SchemaEditor.Test.csproj | 37 +++ SchemaEditor.Test/WidgetHarness.cs | 68 +++++ SchemaEditor/AppData.cs | 5 + SchemaEditor/Program.cs | 28 +- SchemaEditor/SchemaEditor.Diagnostics.cs | 6 +- SchemaEditor/SchemaEditor.Files.cs | 10 +- SchemaTool/Program.cs | 5 + docs/README.md | 3 +- docs/ROADMAP.md | 35 ++- docs/development/README.md | 50 +++- 23 files changed, 1348 insertions(+), 42 deletions(-) create mode 100644 SchemaEditor.Test/AssemblyInfo.cs create mode 100644 SchemaEditor.Test/DiagnosticsTests.cs create mode 100644 SchemaEditor.Test/DocumentGuardTests.cs create mode 100644 SchemaEditor.Test/EditFieldTests.cs create mode 100644 SchemaEditor.Test/EditorHarness.cs create mode 100644 SchemaEditor.Test/HarnessSmokeTests.cs create mode 100644 SchemaEditor.Test/RecentFilesTests.cs create mode 100644 SchemaEditor.Test/SchemaEditor.Test.csproj create mode 100644 SchemaEditor.Test/WidgetHarness.cs diff --git a/.github/workflows/dotnet.yml b/.github/workflows/dotnet.yml index 9898d62..dd13017 100644 --- a/.github/workflows/dotnet.yml +++ b/.github/workflows/dotnet.yml @@ -364,12 +364,13 @@ jobs: '/d:sonar.host.url=https://sonarcloud.io' '/d:sonar.projectBaseDir=${{ github.workspace }}' '/d:sonar.cs.vscoveragexml.reportsPaths=coverage/**/coverage.xml' - # SchemaEditor is excluded from COVERAGE only - it is still analysed for bugs, smells - # and security. It is an ImGui application: its code is immediate-mode draw calls that - # need a live UI context to execute, so line coverage there cannot be earned without a - # UI test harness, tracked separately. Measuring it would only ever report the absence - # of a harness. The library, which is what consumers depend on, stays fully measured. - '/d:sonar.coverage.exclusions=**/*Test*.cs,**/*.Tests.cs,**/*.Tests/**/*,**/obj/**/*,**/*.dll,**/NativeExports.cs,SchemaEditor/**/*.cs' + # The editor's panel and tree files are excluded from COVERAGE only - they are still + # analysed for bugs, smells and security. They are pure immediate-mode draw code: + # a Show() method that reads a value, draws a widget and acts on what the widget + # reports, with nothing to assert that is not a pixel. SchemaEditor.Test now drives the + # rest of the editor headlessly, so the rest is measured; this list is what that + # harness does not yet reach, not the whole application. + '/d:sonar.coverage.exclusions=**/*Test*.cs,**/*.Tests.cs,**/*.Tests/**/*,**/obj/**/*,**/*.dll,**/NativeExports.cs,SchemaEditor/ButtonTree.cs,SchemaEditor/ClassGraphView.cs,SchemaEditor/CodeGeneratorPanel.cs,SchemaEditor/SchemaEditor.Panels.cs,SchemaEditor/Tree*.cs' '/d:sonar.cs.vstest.reportsPaths=coverage/**/*.trx' '/d:sonar.exclusions=**/NativeExports.cs' ) diff --git a/CLAUDE.md b/CLAUDE.md index 4b334d5..1aaa7e8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -4,11 +4,12 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co ## Project Overview -Schema is a C# library for defining and managing data structure schemas. It consists of four projects: +Schema is a C# library for defining and managing data structure schemas. It consists of five projects: - **Schema** - Core library providing schema definition types (classes, enums, members, types) - **Schema.Test** - MSTest unit tests for the core library - **SchemaEditor** - ImGui-based visual editor application for creating and editing `.schema.json` files +- **SchemaEditor.Test** - Headless UI tests for the editor, driven through `ktsu.ImGui.App.Testing` - **SchemaTool** - Command line entry point for validating schemas and running their code generators ## Build Commands @@ -80,6 +81,9 @@ Schema elements maintain parent references via `AssociateWith()` methods. After - `Schema/Models/Types/BaseType.cs` - Abstract base with `[JsonDerivedType]` attributes for polymorphic serialization - `Schema/Models/SchemaClass.cs` - Class definitions containing `SchemaMember` collections - `SchemaEditor/SchemaEditor.cs` - Main editor application using `ktsu.ImGui.App` +- `SchemaEditor/Program.cs` - Builds the `ImGuiAppConfig`; `CreateConfig` is what the tests drive too +- `SchemaEditor.Test/EditorHarness.cs` - Runs a real editor headlessly, frames advanced by the test +- `SchemaEditor.Test/WidgetHarness.cs` - A headless frame containing only the widget under test ## Dependencies diff --git a/Directory.Packages.props b/Directory.Packages.props index ca7394f..a024231 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -18,6 +18,7 @@ + @@ -28,5 +29,6 @@ + diff --git a/README.md b/README.md index e5db654..8e6d656 100644 --- a/README.md +++ b/README.md @@ -14,11 +14,13 @@ A C# library for defining, managing, and editing data structure schemas with a r **ktsu.Schema** lets you define structured data models programmatically or visually, then serialize them to `.schema.json` files (documented in the [schema file format reference](docs/schema-format.md)). It provides a foundation for code generation, data validation, and tooling that needs to understand your data structures at a metadata level. -The solution contains three projects: +The solution contains five projects: - **Schema** - Core library with schema definition types, a rich type system, and JSON serialization - **Schema.Test** - Unit tests for the core library - **SchemaEditor** - ImGui-based desktop application for visual schema editing +- **SchemaEditor.Test** - Headless UI tests that drive the editor with no window or display +- **SchemaTool** - Command line entry point for validating schemas and running their code generators ## Installation diff --git a/Schema.sln b/Schema.sln index 5b50b19..0327819 100644 --- a/Schema.sln +++ b/Schema.sln @@ -11,6 +11,8 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "SchemaEditor", "SchemaEdito EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "SchemaTool", "SchemaTool\SchemaTool.csproj", "{1EBD8E07-A2A3-4091-8748-ECFEB1D301F0}" EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "SchemaEditor.Test", "SchemaEditor.Test\SchemaEditor.Test.csproj", "{31DEE39F-3542-4B65-B744-74243D28D878}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -69,6 +71,18 @@ Global {1EBD8E07-A2A3-4091-8748-ECFEB1D301F0}.Release|x64.Build.0 = Release|Any CPU {1EBD8E07-A2A3-4091-8748-ECFEB1D301F0}.Release|x86.ActiveCfg = Release|Any CPU {1EBD8E07-A2A3-4091-8748-ECFEB1D301F0}.Release|x86.Build.0 = Release|Any CPU + {31DEE39F-3542-4B65-B744-74243D28D878}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {31DEE39F-3542-4B65-B744-74243D28D878}.Debug|Any CPU.Build.0 = Debug|Any CPU + {31DEE39F-3542-4B65-B744-74243D28D878}.Debug|x64.ActiveCfg = Debug|Any CPU + {31DEE39F-3542-4B65-B744-74243D28D878}.Debug|x64.Build.0 = Debug|Any CPU + {31DEE39F-3542-4B65-B744-74243D28D878}.Debug|x86.ActiveCfg = Debug|Any CPU + {31DEE39F-3542-4B65-B744-74243D28D878}.Debug|x86.Build.0 = Debug|Any CPU + {31DEE39F-3542-4B65-B744-74243D28D878}.Release|Any CPU.ActiveCfg = Release|Any CPU + {31DEE39F-3542-4B65-B744-74243D28D878}.Release|Any CPU.Build.0 = Release|Any CPU + {31DEE39F-3542-4B65-B744-74243D28D878}.Release|x64.ActiveCfg = Release|Any CPU + {31DEE39F-3542-4B65-B744-74243D28D878}.Release|x64.Build.0 = Release|Any CPU + {31DEE39F-3542-4B65-B744-74243D28D878}.Release|x86.ActiveCfg = Release|Any CPU + {31DEE39F-3542-4B65-B744-74243D28D878}.Release|x86.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE diff --git a/Schema/AssemblyInfo.cs b/Schema/AssemblyInfo.cs index d349950..fc84194 100644 --- a/Schema/AssemblyInfo.cs +++ b/Schema/AssemblyInfo.cs @@ -1,3 +1,8 @@ // Copyright (c) 2023-2026 ktsu-dev contributors +// Both test assemblies are named, rather than only the one that reads this project's internals. +// ktsu.Sdk's KTSU0002 requires a non-test project to expose its internals to the repository's test +// projects, and there are two of them now; which of the two a given project actually needs is not +// what the rule is checking. [assembly: System.Runtime.CompilerServices.InternalsVisibleTo("ktsu.Schema.Test")] +[assembly: System.Runtime.CompilerServices.InternalsVisibleTo("ktsu.SchemaEditor.Test")] diff --git a/SchemaEditor.Test/AssemblyInfo.cs b/SchemaEditor.Test/AssemblyInfo.cs new file mode 100644 index 0000000..1389d87 --- /dev/null +++ b/SchemaEditor.Test/AssemblyInfo.cs @@ -0,0 +1,5 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +// One ImGui context exists at a time, and ImGuiAppHarness refuses to start a second while one is +// running, so these tests cannot run concurrently with each other the way the library's can. +[assembly: DoNotParallelize] diff --git a/SchemaEditor.Test/DiagnosticsTests.cs b/SchemaEditor.Test/DiagnosticsTests.cs new file mode 100644 index 0000000..dccd262 --- /dev/null +++ b/SchemaEditor.Test/DiagnosticsTests.cs @@ -0,0 +1,268 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.SchemaEditor.Test; + +using System.Linq; + +using ktsu.Schema.Models; +using ktsu.Schema.Models.Names; +using ktsu.Semantics.Strings; + +/// +/// The diagnostics panel's logic: when validation runs, and what clicking an issue selects. +/// +[TestClass] +public sealed class DiagnosticsTests +{ + private EditorHarness harness = null!; + + [TestInitialize] + public void StartEditor() => harness = EditorHarness.Start(); + + [TestCleanup] + public void StopEditor() => harness.Dispose(); + + /// + /// Builds a schema whose every element kind has something wrong with it, so one validation run + /// produces an issue pointing at each. + /// + private static Schema BuildSchemaWithAnIssuePerElementKind() + { + Schema schema = new(); + + // A member with no type: a warning naming the member. + SchemaClass user = schema.AddClass("User".As())!; + user.AddMember("Untyped".As()); + + // An empty class name: an error naming the class. + schema.AddClass(new ClassName()); + + // An empty enum name: an error naming the enum. + schema.AddEnum(new EnumName()); + + // A data source with no class and no file: warnings naming the data source. + schema.AddDataSource("Users".As()); + + // A code generator with no output path and no language: warnings naming the generator. + schema.AddCodeGenerator("CSharp".As()); + + return schema; + } + + [TestMethod] + public void ValidationDoesNotRunUntilTheSchemaSettles() + { + harness.Editor.CurrentSchema = BuildSchemaWithAnIssuePerElementKind(); + harness.Editor.RequestValidation(); + + // Just short of the debounce: the edit is still in flight, so nothing has been validated. + harness.Editor.UpdateValidation(SchemaEditor.ValidationDebounceSeconds - 0.01f); + Assert.AreEqual(0, harness.Editor.Diagnostics.Count); + + harness.Editor.UpdateValidation(0.02f); + Assert.IsTrue(harness.Editor.Diagnostics.Count > 0, "The schema should have been validated once the debounce elapsed."); + } + + /// + /// A burst of edits - the case the debounce exists for - must validate once at the end rather + /// than once per edit. + /// + [TestMethod] + public void EachEditRestartsTheDebounce() + { + harness.Editor.CurrentSchema = BuildSchemaWithAnIssuePerElementKind(); + + for (int edit = 0; edit < 5; edit++) + { + harness.Editor.RequestValidation(); + harness.Editor.UpdateValidation(SchemaEditor.ValidationDebounceSeconds - 0.01f); + Assert.AreEqual(0, harness.Editor.Diagnostics.Count, $"Validation ran while edit {edit} was still in flight."); + } + + harness.Editor.UpdateValidation(0.02f); + Assert.IsTrue(harness.Editor.Diagnostics.Count > 0); + } + + [TestMethod] + public void ValidationDoesNotRunAgainUntilSomethingChanges() + { + harness.Editor.CurrentSchema = BuildSchemaWithAnIssuePerElementKind(); + harness.Editor.RequestValidation(); + harness.Editor.UpdateValidation(SchemaEditor.ValidationDebounceSeconds); + + System.Collections.ObjectModel.Collection first = harness.Editor.Diagnostics; + + harness.Editor.UpdateValidation(10f); + + Assert.AreSame(first, harness.Editor.Diagnostics, "Validation re-ran without the schema having changed."); + } + + [TestMethod] + public void ValidatingWithNoSchemaClearsTheIssues() + { + harness.Editor.CurrentSchema = BuildSchemaWithAnIssuePerElementKind(); + harness.Editor.RequestValidation(); + harness.Editor.UpdateValidation(SchemaEditor.ValidationDebounceSeconds); + Assert.IsTrue(harness.Editor.Diagnostics.Count > 0); + + harness.Editor.CurrentSchema = null; + harness.Editor.RequestValidation(); + harness.Editor.UpdateValidation(SchemaEditor.ValidationDebounceSeconds); + + Assert.AreEqual(0, harness.Editor.Diagnostics.Count); + } + + private SchemaValidationIssue IssueFor() => + harness.Editor.Diagnostics.FirstOrDefault(i => i.Element is TElement) + ?? throw new AssertFailedException($"No validation issue was reported against a {typeof(TElement).Name}. Issues: {string.Join("; ", harness.Editor.Diagnostics.Select(i => $"{i.Path}: {i.Message}"))}"); + + private void ValidateOnce() + { + harness.Editor.RequestValidation(); + harness.Editor.UpdateValidation(SchemaEditor.ValidationDebounceSeconds); + } + + [TestMethod] + public void NavigatingToAClassIssueSelectsTheClass() + { + harness.Editor.CurrentSchema = BuildSchemaWithAnIssuePerElementKind(); + ValidateOnce(); + + SchemaValidationIssue issue = IssueFor(); + harness.Editor.NavigateTo(issue); + + Assert.AreSame(issue.Element, harness.Editor.CurrentClass); + } + + /// + /// A member has no panel of its own; its row is drawn in its class's, so that is what must be + /// selected. + /// + [TestMethod] + public void NavigatingToAMemberIssueSelectsItsOwningClass() + { + harness.Editor.CurrentSchema = BuildSchemaWithAnIssuePerElementKind(); + ValidateOnce(); + + SchemaValidationIssue issue = IssueFor(); + harness.Editor.NavigateTo(issue); + + Assert.AreSame(((SchemaMember)issue.Element!).ParentClass, harness.Editor.CurrentClass); + } + + [TestMethod] + public void NavigatingToAnEnumIssueSelectsTheEnum() + { + harness.Editor.CurrentSchema = BuildSchemaWithAnIssuePerElementKind(); + ValidateOnce(); + + SchemaValidationIssue issue = IssueFor(); + harness.Editor.NavigateTo(issue); + + Assert.AreSame(issue.Element, harness.Editor.CurrentEnum); + } + + [TestMethod] + public void NavigatingToADataSourceIssueSelectsTheDataSource() + { + harness.Editor.CurrentSchema = BuildSchemaWithAnIssuePerElementKind(); + ValidateOnce(); + + SchemaValidationIssue issue = IssueFor(); + harness.Editor.NavigateTo(issue); + + Assert.AreSame(issue.Element, harness.Editor.CurrentDataSource); + } + + [TestMethod] + public void NavigatingToACodeGeneratorIssueSelectsTheCodeGenerator() + { + harness.Editor.CurrentSchema = BuildSchemaWithAnIssuePerElementKind(); + ValidateOnce(); + + SchemaValidationIssue issue = IssueFor(); + harness.Editor.NavigateTo(issue); + + Assert.AreSame(issue.Element, harness.Editor.CurrentCodeGenerator); + } + + /// + /// Selecting one element must clear the rest, or two panels would claim to be showing the + /// current selection at once. + /// + [TestMethod] + public void NavigatingClearsThePreviousSelection() + { + harness.Editor.CurrentSchema = BuildSchemaWithAnIssuePerElementKind(); + ValidateOnce(); + + harness.Editor.NavigateTo(IssueFor()); + harness.Editor.NavigateTo(IssueFor()); + + Assert.IsNull(harness.Editor.CurrentClass); + Assert.IsNotNull(harness.Editor.CurrentDataSource); + } + + /// + /// A duplicate-name issue names no single element, so there is nothing to select and nothing + /// should change. + /// + [TestMethod] + public void NavigatingToAnIssueWithNoElementChangesNothing() + { + harness.Editor.CurrentSchema = BuildSchemaWithAnIssuePerElementKind(); + ValidateOnce(); + harness.Editor.NavigateTo(IssueFor()); + SchemaClass? selected = harness.Editor.CurrentClass; + + harness.Editor.NavigateTo(new SchemaValidationIssue + { + Severity = SchemaValidationSeverity.Error, + Path = "User", + Message = "Duplicate class name 'User'.", + }); + + Assert.AreSame(selected, harness.Editor.CurrentClass); + } + + [TestMethod] + public void AnElementWithNoIssueIsNotMarked() + { + Schema schema = new(); + SchemaClass clean = schema.AddClass("Clean".As())!; + harness.Editor.CurrentSchema = schema; + ValidateOnce(); + + Assert.IsNull(harness.Editor.GetIssueFor(clean)); + } + + [TestMethod] + public void AnElementWithNoElementReferenceIsNotMatched() => + Assert.IsNull(harness.Editor.GetIssueFor(null)); + + /// + /// The inline marker has room for one issue, so it must be the most severe one affecting the + /// element rather than whichever validation happened to report first. + /// + [TestMethod] + public void TheMarkerForAnElementIsItsMostSevereIssue() + { + Schema schema = new(); + + // No class and no file: the missing class is a warning, and the missing file is another. + // Pointing it at a class that does not exist makes the first an error instead. + DataSource dataSource = schema.AddDataSource("Users".As())!; + dataSource.ClassName = "Missing".As(); + + harness.Editor.CurrentSchema = schema; + ValidateOnce(); + + SchemaValidationIssue? marked = harness.Editor.GetIssueFor(dataSource); + + Assert.IsNotNull(marked); + Assert.AreEqual(SchemaValidationSeverity.Error, marked.Severity); + Assert.IsTrue( + harness.Editor.Diagnostics.Any(i => ReferenceEquals(i.Element, dataSource) && i.Severity == SchemaValidationSeverity.Warning), + "This test only proves anything while the data source also has a warning to be outranked."); + } +} diff --git a/SchemaEditor.Test/DocumentGuardTests.cs b/SchemaEditor.Test/DocumentGuardTests.cs new file mode 100644 index 0000000..036b94e --- /dev/null +++ b/SchemaEditor.Test/DocumentGuardTests.cs @@ -0,0 +1,331 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.SchemaEditor.Test; + +using System; +using System.IO; + +using ktsu.Schema.Models; +using ktsu.Schema.Models.Names; +using ktsu.Semantics.Paths; +using ktsu.Semantics.Strings; +using ktsu.UndoRedo; + +/// +/// The guard that stands between an action and the open document's unsaved work, and the +/// save-then-continue sequence it starts. +/// +/// +/// These run through the prompt as a user does - the popup is drawn into a real frame and the +/// button is clicked by name - because the sequencing being tested is precisely what the popup's +/// callbacks do, and a test that invoked those callbacks directly would not be testing it. +/// +[TestClass] +public sealed class DocumentGuardTests +{ + private EditorHarness harness = null!; + private string scratchDirectory = null!; + + [TestInitialize] + public void StartEditor() + { + harness = EditorHarness.Start(); + + // A real directory, not the in-memory one: SchemaFile saves through System.IO directly. + scratchDirectory = Path.Combine(Path.GetTempPath(), $"schema-editor-tests-{Guid.NewGuid():N}"); + Directory.CreateDirectory(scratchDirectory); + } + + [TestCleanup] + public void StopEditor() + { + harness.Dispose(); + + try + { + Directory.Delete(scratchDirectory, recursive: true); + } + catch (IOException) + { + // A leftover temporary directory is not worth failing a passing test over. + } + } + + private AbsoluteFilePath ScratchFile(string name) => + Path.Combine(scratchDirectory, name).As(); + + /// + /// Opens a document and makes an edit through the undo service, which is what + /// reads. + /// + private void OpenDirtyDocument() + { + Schema schema = new(); + harness.Editor.CurrentSchema = schema; + harness.Editor.Execute(new DelegateCommand( + "Add Class", + () => schema.TryAddClass("User".As()), + () => schema.RemoveClass("User".As()), + ChangeType.Insert)); + + Assert.IsTrue(harness.Editor.HasUnsavedChanges, "The document should be dirty after an edit."); + } + + /// + /// Advances frames until the prompt has been drawn, then clicks one of its buttons. + /// + /// + /// The extra frames after the button first appears 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 button on its first frame is not where that button ends up. Clicking there hits the + /// background instead, and the prompt sits unanswered. + /// + private void AnswerPrompt(string button) + { + harness.StepUntil(() => harness.App.Probe.Matches(button).Count > 0, $"the '{button}' button appearing"); + harness.App.Step(3); + harness.App.Click(button); + harness.App.Step(2); + } + + [TestMethod] + public void AnUnmodifiedDocumentIsDiscardedWithoutAsking() + { + harness.Editor.CurrentSchema = new Schema(); + bool proceeded = false; + + harness.Editor.WithUnsavedChangesGuard(() => proceeded = true); + + Assert.IsTrue(proceeded, "Nothing was at risk, so the action should have run straight away."); + } + + [TestMethod] + public void AModifiedDocumentIsNotDiscardedWithoutAsking() + { + OpenDirtyDocument(); + bool proceeded = false; + + harness.Editor.WithUnsavedChangesGuard(() => proceeded = true); + harness.App.Step(3); + + Assert.IsFalse(proceeded, "The action ran before the user was asked about unsaved work."); + Assert.IsTrue(harness.App.Probe.Matches("Discard").Count > 0, "The unsaved-changes prompt was not raised."); + } + + [TestMethod] + public void DiscardingRunsTheAction() + { + OpenDirtyDocument(); + bool proceeded = false; + + harness.Editor.WithUnsavedChangesGuard(() => proceeded = true); + AnswerPrompt("Discard"); + + Assert.IsTrue(proceeded); + } + + [TestMethod] + public void CancellingRunsNeitherTheActionNorNothing() + { + OpenDirtyDocument(); + bool proceeded = false; + bool cancelled = false; + + harness.Editor.WithUnsavedChangesGuard(() => proceeded = true, () => cancelled = true); + AnswerPrompt("Cancel"); + + Assert.IsFalse(proceeded); + Assert.IsTrue(cancelled); + } + + [TestMethod] + public void SavingWritesTheDocumentAndThenRunsTheAction() + { + OpenDirtyDocument(); + AbsoluteFilePath path = ScratchFile("saved.schema.json"); + harness.Editor.CurrentSchemaPath = path; + bool proceeded = false; + + harness.Editor.WithUnsavedChangesGuard(() => proceeded = true); + AnswerPrompt("Save"); + + Assert.IsTrue(File.Exists(path), "The document should have been written before the action ran."); + Assert.IsTrue(proceeded); + Assert.IsFalse(harness.Editor.HasUnsavedChanges, "Saving should have cleared the unsaved marker."); + } + + /// + /// A document with no path cannot be saved without asking where to put it, and the file browser + /// answers over later frames. The continuation must wait for that answer rather than running as + /// though the save had happened. + /// + [TestMethod] + public void SavingADocumentWithNoPathDefersTheActionToTheFileBrowser() + { + harness.Editor.CurrentSchema = new Schema(); + Assert.AreEqual(string.Empty, harness.Editor.CurrentSchemaPath.ToString()); + bool proceeded = false; + + harness.Editor.SaveThen(() => proceeded = true); + harness.App.Step(3); + + Assert.IsFalse(proceeded, "The action ran without the document having been saved anywhere."); + } + + [TestMethod] + public void SavingADocumentWithAPathRunsTheActionOnceItIsWritten() + { + harness.Editor.CurrentSchema = new Schema(); + AbsoluteFilePath path = ScratchFile("direct.schema.json"); + harness.Editor.CurrentSchemaPath = path; + bool proceeded = false; + + harness.Editor.SaveThen(() => proceeded = true); + + Assert.IsTrue(File.Exists(path)); + Assert.IsTrue(proceeded); + } + + /// + /// A save that fails must not run the continuation: the work it was guarding is still at risk. + /// + [TestMethod] + public void AFailedSaveDoesNotRunTheAction() + { + harness.Editor.CurrentSchema = new Schema(); + + // A path whose parent is an existing file rather than a directory cannot be created. + AbsoluteFilePath blocker = ScratchFile("blocker"); + File.WriteAllText(blocker, "not a directory"); + harness.Editor.CurrentSchemaPath = Path.Combine(blocker, "nested.schema.json").As(); + + bool proceeded = false; + harness.Editor.SaveThen(() => proceeded = true); + + Assert.IsFalse(proceeded); + } + + [TestMethod] + public void SavingRecordsTheDocumentAsRecentlyUsed() + { + harness.Editor.CurrentSchema = new Schema(); + AbsoluteFilePath path = ScratchFile("recent.schema.json"); + harness.Editor.CurrentSchemaPath = path; + + harness.Editor.SaveThen(null); + + Assert.AreEqual(path, harness.Editor.Options.RecentFiles[0]); + } + + [TestMethod] + public void OpeningADocumentRecordsItAsRecentlyUsedAndSelectsItsFirstClass() + { + Schema source = new(); + source.AddClass("First".As()); + source.AddClass("Second".As()); + AbsoluteFilePath path = ScratchFile("opened.schema.json"); + File.WriteAllText(path, SchemaSerializer.Serialize(source)); + + harness.Editor.LoadFrom(path); + + Assert.IsNotNull(harness.Editor.CurrentSchema); + Assert.AreEqual(path, harness.Editor.CurrentSchemaPath); + Assert.AreEqual("First", harness.Editor.CurrentClass?.Name.ToString()); + Assert.AreEqual(path, harness.Editor.Options.RecentFiles[0]); + Assert.AreEqual("opened.schema.json", harness.Editor.DocumentName); + } + + /// + /// A failed open must leave the document that is already open alone, rather than half-replacing + /// it with nothing. + /// + [TestMethod] + public void AFailedOpenLeavesTheCurrentDocumentAlone() + { + Schema open = new(); + harness.Editor.CurrentSchema = open; + AbsoluteFilePath path = ScratchFile("kept.schema.json"); + harness.Editor.CurrentSchemaPath = path; + + harness.Editor.LoadFrom(ScratchFile("missing.schema.json")); + + Assert.AreSame(open, harness.Editor.CurrentSchema); + Assert.AreEqual(path, harness.Editor.CurrentSchemaPath); + } + + [TestMethod] + public void ANewDocumentReplacesAnUnmodifiedOne() + { + Schema first = new(); + harness.Editor.CurrentSchema = first; + + harness.Editor.New(); + + Assert.IsNotNull(harness.Editor.CurrentSchema); + Assert.AreNotSame(first, harness.Editor.CurrentSchema); + Assert.AreEqual(string.Empty, harness.Editor.CurrentSchemaPath.ToString()); + } + + [TestMethod] + public void ANewDocumentAsksBeforeReplacingAModifiedOne() + { + OpenDirtyDocument(); + Schema dirty = harness.Editor.CurrentSchema!; + + harness.Editor.New(); + harness.App.Step(3); + + Assert.AreSame(dirty, harness.Editor.CurrentSchema); + + AnswerPrompt("Discard"); + + Assert.AreNotSame(dirty, harness.Editor.CurrentSchema); + } + + [TestMethod] + public void ClosingIsAllowedWhenNothingWouldBeLost() + { + harness.Editor.CurrentSchema = new Schema(); + + Assert.IsTrue(harness.Editor.ShouldClose()); + } + + [TestMethod] + public void ClosingIsRefusedWhileWorkWouldBeLost() + { + OpenDirtyDocument(); + + Assert.IsFalse(harness.Editor.ShouldClose()); + } + + /// + /// The refused close has to raise its prompt from the render loop, since the close callback + /// returns before another frame is drawn. + /// + [TestMethod] + public void ARefusedCloseRaisesThePromptOnALaterFrame() + { + OpenDirtyDocument(); + + Assert.IsFalse(harness.Editor.ShouldClose()); + harness.StepUntil(() => harness.App.Probe.Matches("Discard").Count > 0, "the close prompt appearing"); + } + + /// + /// Hitting the close button repeatedly must not stack a prompt per press, which would leave the + /// user dismissing the same question several times. + /// + [TestMethod] + public void RepeatedCloseAttemptsRaiseOnlyOnePrompt() + { + OpenDirtyDocument(); + + Assert.IsFalse(harness.Editor.ShouldClose()); + Assert.IsFalse(harness.Editor.ShouldClose()); + harness.App.Step(3); + harness.Editor.ProcessCloseRequest(); + harness.App.Step(3); + + Assert.IsFalse(harness.App.Probe.IsAmbiguous("Discard"), "More than one unsaved-changes prompt was raised."); + } +} diff --git a/SchemaEditor.Test/EditFieldTests.cs b/SchemaEditor.Test/EditFieldTests.cs new file mode 100644 index 0000000..1ec4e98 --- /dev/null +++ b/SchemaEditor.Test/EditFieldTests.cs @@ -0,0 +1,262 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.SchemaEditor.Test; + +using System; +using System.Numerics; + +using Hexa.NET.ImGui; + +using ktsu.ImGui.App.Testing; + +/// +/// That a text field reports one value per editing session rather than one per frame, and that two +/// fields sharing a label do not share a buffer. +/// +/// +/// This is the widget the editor's every name and description field is built on. ImGui hands back +/// its buffer on every frame the widget exists, so the naive binding writes to the model sixty +/// times a second and, now that edits are undoable, pushes an undo entry each time. +/// +[TestClass] +public sealed class EditFieldTests +{ + private WidgetHarness harness = null!; + + [TestInitialize] + public void StartHarness() + { + harness = WidgetHarness.Start(); + EditField.Reset(); + } + + [TestCleanup] + public void StopHarness() + { + EditField.Reset(); + harness.Dispose(); + } + + /// + /// One text field, with everything a test needs to drive it and to see what it reported. + /// + private sealed class Field + { + internal string Model { get; set; } = string.Empty; + internal string Id { get; init; } = "##Field"; + internal string? Scope { get; init; } + internal int Commits { get; private set; } + internal string? LastCommit { get; private set; } + internal Vector2 Centre { get; private set; } + internal Rectangle Bounds { get; private set; } + + internal void Draw() + { + if (Scope is not null) + { + ImGui.PushID(Scope); + } + + if (EditField.Text(Id, 200f, Model, out string committed)) + { + Commits++; + LastCommit = committed; + Model = committed; + } + + Vector2 min = ImGui.GetItemRectMin(); + Vector2 max = ImGui.GetItemRectMax(); + Centre = (min + max) * 0.5f; + Bounds = new Rectangle((int)min.X, (int)min.Y, (int)MathF.Ceiling(max.X), (int)MathF.Ceiling(max.Y)); + + if (Scope is not null) + { + ImGui.PopID(); + } + } + } + + /// + /// Clicks into a field and replaces its contents, leaving the field still being edited. + /// + private void TypeInto(Field field, string text) + { + harness.App.Mouse.Click(field.Centre.X, field.Centre.Y); + harness.App.Keyboard.Press(ImGuiKey.A, ctrl: true); + harness.App.Keyboard.Type(text); + } + + private void CommitWithEnter() => harness.App.Keyboard.Press(ImGuiKey.Enter); + + [TestMethod] + public void AnUntouchedFieldReportsNothing() + { + Field field = new() { Model = "Name" }; + harness.Draw = field.Draw; + + harness.App.Step(10); + + Assert.AreEqual(0, field.Commits, "A field nobody touched wrote to the model."); + } + + [TestMethod] + public void FinishingAnEditReportsTheNewValue() + { + Field field = new() { Model = "Name" }; + harness.Draw = field.Draw; + harness.App.Step(2); + + TypeInto(field, "User"); + CommitWithEnter(); + + Assert.AreEqual(1, field.Commits); + Assert.AreEqual("User", field.LastCommit); + } + + /// + /// The point of the widget: an editing session is one write, and so one undo entry, however + /// many frames it spans. + /// + [TestMethod] + public void AnEditIsReportedOnceRatherThanPerFrame() + { + Field field = new() { Model = "Name" }; + harness.Draw = field.Draw; + harness.App.Step(2); + + TypeInto(field, "User"); + CommitWithEnter(); + harness.App.Step(30); + + Assert.AreEqual(1, field.Commits, "The field kept writing to the model after the edit finished."); + } + + [TestMethod] + public void LeavingAFieldWithoutChangingItReportsNothing() + { + Field field = new() { Model = "Name" }; + harness.Draw = field.Draw; + harness.App.Step(2); + + harness.App.Mouse.Click(field.Centre.X, field.Centre.Y); + harness.App.Keyboard.Press(ImGuiKey.Enter); + harness.App.Step(3); + + Assert.AreEqual(0, field.Commits); + Assert.AreEqual("Name", field.Model); + } + + /// + /// Escape abandons the edit, so nothing should reach the model - and no undo entry should be + /// pushed for an edit the user took back. + /// + [TestMethod] + public void AbandoningAnEditReportsNothing() + { + Field field = new() { Model = "Name" }; + harness.Draw = field.Draw; + harness.App.Step(2); + + TypeInto(field, "User"); + harness.App.Keyboard.Press(ImGuiKey.Escape); + harness.App.Step(3); + + Assert.AreEqual(0, field.Commits); + Assert.AreEqual("Name", field.Model); + } + + /// + /// An edit spanning several frames must accumulate. The scratch buffer is what carries it, so + /// this fails the moment the buffer is dropped or overwritten between frames. + /// + [TestMethod] + public void AnEditSpanningManyFramesKeepsEveryCharacter() + { + Field field = new() { Model = "Name" }; + harness.Draw = field.Draw; + harness.App.Step(2); + + TypeInto(field, "LongerReplacement"); + CommitWithEnter(); + + Assert.AreEqual("LongerReplacement", field.LastCommit); + } + + /// + /// Every member row in the editor draws its name field as "##Name" and relies on the + /// surrounding PushID to tell the rows apart. Keying the scratch buffer on the label rather + /// than the resolved id lets one row's half-typed text into the next row's field. + /// + /// + /// The leak has to be measured on screen. ImGui keeps its own copy of the text while a widget + /// is being edited, so the row being typed into still commits the right value with the buffers + /// shared - the row that is wrong is the other one, which is handed its sibling's text to draw. + /// So this compares the pixels of the second row against the same pixels before the edit + /// started: nothing about that row changed, so nothing about it should be drawn differently. + /// + [TestMethod] + public void TwoRowsSharingALabelDoNotShareABuffer() + { + Field first = new() { Model = "Alpha", Id = "##Name", Scope = "row0" }; + Field second = new() { Model = "Beta", Id = "##Name", Scope = "row1" }; + harness.Draw = () => + { + first.Draw(); + second.Draw(); + }; + harness.App.Step(3); + + CapturedFrame before = harness.App.Capture(); + Rectangle secondRow = second.Bounds; + + // Long and unlike "Beta", so a leak into the second row is unmistakable on screen. + TypeInto(first, "WWWWWWWWWWWW"); + CapturedFrame during = harness.App.Capture(); + + AssertUnchanged(before, during, secondRow, "The second row was redrawn while its sibling was being edited, so the sibling's text leaked into it."); + + CommitWithEnter(); + + Assert.AreEqual("WWWWWWWWWWWW", first.LastCommit); + Assert.AreEqual(1, first.Commits); + Assert.AreEqual(0, second.Commits, "Editing one row wrote to another row's model."); + Assert.AreEqual("Beta", second.Model); + } + + private static void AssertUnchanged(CapturedFrame before, CapturedFrame after, Rectangle region, string message) + { + for (int y = region.MinY; y < region.MaxY; y++) + { + for (int x = region.MinX; x < region.MaxX; x++) + { + if (before.GetPixel(x, y) != after.GetPixel(x, y)) + { + Assert.Fail($"{message} First difference at ({x}, {y})."); + } + } + } + } + + /// + /// While a field is not being edited the model is the source of truth, so an undo that changes + /// the value shows up the moment the field is redrawn - and does not get written back over. + /// + [TestMethod] + public void AFieldThatIsNotBeingEditedFollowsTheModel() + { + Field field = new() { Model = "Name" }; + harness.Draw = field.Draw; + harness.App.Step(2); + + TypeInto(field, "User"); + CommitWithEnter(); + Assert.AreEqual("User", field.Model); + + // What an undo of that rename does to the model. + field.Model = "Name"; + harness.App.Step(5); + + Assert.AreEqual(1, field.Commits, "The field wrote its own last text back over the undone value."); + Assert.AreEqual("Name", field.Model); + } +} diff --git a/SchemaEditor.Test/EditorHarness.cs b/SchemaEditor.Test/EditorHarness.cs new file mode 100644 index 0000000..08c4ab0 --- /dev/null +++ b/SchemaEditor.Test/EditorHarness.cs @@ -0,0 +1,98 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.SchemaEditor.Test; + +using System; +using System.IO.Abstractions.TestingHelpers; + +using ktsu.ImGui.App.Testing; + +/// +/// An editor running headlessly, with frames advanced by the test rather than by a display. +/// +/// +/// +/// The editor's code is immediate-mode draw calls: a value is read, a widget is drawn, and what the +/// widget reports decides what happens next, all inside one function. None of that executes without +/// a live ImGui context, which is why the editor had no tests at all. +/// supplies that context with no window, no display and no GPU - it rasterizes in software and +/// injects input straight into ImGui's event queue - so the real draw code runs on a headless +/// continuous integration runner. +/// +/// +/// The configuration comes from , the same one the application +/// starts with, so a callback renamed or dropped there breaks these tests instead of leaving them +/// passing against a host that no longer exists. +/// +/// +/// Settings are redirected to an in-memory file system before the editor is constructed, because +/// otherwise reads and writes the real settings of whoever runs the suite - +/// and the editor saves them on exit, so a test could overwrite a developer's open document and +/// recent files. +/// +/// +internal sealed class EditorHarness : IDisposable +{ + /// + /// Gets the editor under test. + /// + internal SchemaEditor Editor { get; } + + /// + /// Gets the harness advancing its frames. + /// + internal ImGuiAppHarness App { get; } + + private bool disposed; + + private EditorHarness(SchemaEditor editor, ImGuiAppHarness app) + { + Editor = editor; + App = app; + } + + /// + /// Starts an editor with empty settings and advances the frames it needs to be drawing. + /// + /// The running harness. Dispose it to release the ImGui context. + internal static EditorHarness Start() + { + // Must precede the constructor: it is the constructor that loads the settings. + ktsu.AppDataStorage.AppData.ConfigureForTesting(() => new MockFileSystem()); + + SchemaEditor editor = new(); + ImGuiAppHarness app = ImGuiAppHarness.Start(Program.CreateConfig(editor), new HarnessOptions()); + + // The first frame builds the font atlas and lays the panels out; nothing is measurable + // before it has run. + app.Step(); + + return new EditorHarness(editor, app); + } + + /// + /// Advances frames until a condition holds, failing the test if it never does. + /// + /// Checked before the first frame and after every frame. + /// What was being waited for, for the failure message. + /// The frame budget. Frames rather than time, so a loaded runner is slower without being flakier. + internal void StepUntil(Func condition, string description, int maxFrames = 120) + { + if (!App.StepUntil(condition, maxFrames)) + { + Assert.Fail($"{description} did not happen within {maxFrames} frames."); + } + } + + public void Dispose() + { + if (disposed) + { + return; + } + + disposed = true; + App.Dispose(); + ktsu.AppDataStorage.AppData.ResetFileSystem(); + } +} diff --git a/SchemaEditor.Test/HarnessSmokeTests.cs b/SchemaEditor.Test/HarnessSmokeTests.cs new file mode 100644 index 0000000..7627086 --- /dev/null +++ b/SchemaEditor.Test/HarnessSmokeTests.cs @@ -0,0 +1,32 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.SchemaEditor.Test; + +/// +/// That the harness itself works: without these, a failure anywhere else is ambiguous between the +/// editor being wrong and the harness never having drawn a frame. +/// +[TestClass] +public sealed class HarnessSmokeTests +{ + [TestMethod] + public void EditorDrawsFramesHeadlessly() + { + using EditorHarness harness = EditorHarness.Start(); + + int before = harness.App.FrameCount; + harness.App.Step(3); + + Assert.AreEqual(before + 3, harness.App.FrameCount); + } + + [TestMethod] + public void EditorStartsWithNoDocument() + { + using EditorHarness harness = EditorHarness.Start(); + + Assert.IsNull(harness.Editor.CurrentSchema); + Assert.IsFalse(harness.Editor.HasUnsavedChanges); + Assert.AreEqual("Untitled schema", harness.Editor.DocumentName); + } +} diff --git a/SchemaEditor.Test/RecentFilesTests.cs b/SchemaEditor.Test/RecentFilesTests.cs new file mode 100644 index 0000000..f8980fc --- /dev/null +++ b/SchemaEditor.Test/RecentFilesTests.cs @@ -0,0 +1,103 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.SchemaEditor.Test; + +using System.Linq; + +using ktsu.Semantics.Paths; +using ktsu.Semantics.Strings; + +/// +/// The recent-files list. Pure logic - no ImGui context needed - which is why it is the first +/// thing here. +/// +[TestClass] +public sealed class RecentFilesTests +{ + // AppData is disposable and saves on dispose if a save is outstanding, so even these + // filesystem-free tests are pointed at an in-memory store rather than the real settings of + // whoever runs the suite. + [TestInitialize] + public void RedirectSettings() => + ktsu.AppDataStorage.AppData.ConfigureForTesting(() => new System.IO.Abstractions.TestingHelpers.MockFileSystem()); + + [TestCleanup] + public void RestoreSettings() => ktsu.AppDataStorage.AppData.ResetFileSystem(); + + private static AbsoluteFilePath Path(string name) => + $"{(OperatingSystem.IsWindows() ? "C:\\schemas\\" : "/schemas/")}{name}".As(); + + [TestMethod] + public void RecordingAFilePutsItFirst() + { + using AppData options = new(); + + options.RecordRecentFile(Path("a.schema.json")); + options.RecordRecentFile(Path("b.schema.json")); + + Assert.AreEqual(2, options.RecentFiles.Count); + Assert.AreEqual(Path("b.schema.json"), options.RecentFiles[0]); + Assert.AreEqual(Path("a.schema.json"), options.RecentFiles[1]); + } + + [TestMethod] + public void RecordingAFileAgainMovesItRatherThanDuplicatingIt() + { + using AppData options = new(); + + options.RecordRecentFile(Path("a.schema.json")); + options.RecordRecentFile(Path("b.schema.json")); + options.RecordRecentFile(Path("a.schema.json")); + + Assert.AreEqual(2, options.RecentFiles.Count); + Assert.AreEqual(Path("a.schema.json"), options.RecentFiles[0]); + Assert.AreEqual(Path("b.schema.json"), options.RecentFiles[1]); + } + + [TestMethod] + public void RecordingIsBoundedAndDropsTheOldest() + { + using AppData options = new(); + + for (int index = 0; index < AppData.MaxRecentFiles + 5; index++) + { + options.RecordRecentFile(Path($"file{index}.schema.json")); + } + + Assert.AreEqual(AppData.MaxRecentFiles, options.RecentFiles.Count); + + // Newest first, so the most recently recorded file leads and the oldest survivor is the + // one recorded MaxRecentFiles ago. + Assert.AreEqual(Path($"file{AppData.MaxRecentFiles + 4}.schema.json"), options.RecentFiles[0]); + Assert.AreEqual(Path("file5.schema.json"), options.RecentFiles[^1]); + Assert.IsFalse(options.RecentFiles.Contains(Path("file0.schema.json"))); + } + + [TestMethod] + public void RecordingAnEmptyPathIsIgnored() + { + using AppData options = new(); + + options.RecordRecentFile(new AbsoluteFilePath()); + + Assert.AreEqual(0, options.RecentFiles.Count); + } + + /// + /// A list that already holds duplicates - written by an older build, or hand-edited - must not + /// keep one of them behind when the file is recorded again. + /// + [TestMethod] + public void RecordingRemovesEveryEarlierCopy() + { + using AppData options = new(); + options.RecentFiles.Add(Path("a.schema.json")); + options.RecentFiles.Add(Path("b.schema.json")); + options.RecentFiles.Add(Path("a.schema.json")); + + options.RecordRecentFile(Path("a.schema.json")); + + Assert.AreEqual(2, options.RecentFiles.Count); + Assert.AreEqual(1, options.RecentFiles.Count(p => p == Path("a.schema.json"))); + } +} diff --git a/SchemaEditor.Test/SchemaEditor.Test.csproj b/SchemaEditor.Test/SchemaEditor.Test.csproj new file mode 100644 index 0000000..e6a5226 --- /dev/null +++ b/SchemaEditor.Test/SchemaEditor.Test.csproj @@ -0,0 +1,37 @@ + + + + + + true + + + + + + + + + + + + + + + + 18.8.0 + + + + + + + + net10.0 + + + diff --git a/SchemaEditor.Test/WidgetHarness.cs b/SchemaEditor.Test/WidgetHarness.cs new file mode 100644 index 0000000..2d65c3d --- /dev/null +++ b/SchemaEditor.Test/WidgetHarness.cs @@ -0,0 +1,68 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.SchemaEditor.Test; + +using System; + +using ktsu.ImGui.App; +using ktsu.ImGui.App.Testing; + +/// +/// A headless ImGui frame with nothing in it but the widget under test. +/// +/// +/// Separate from , which drives the whole editor through the real +/// application configuration. A widget like is a unit below that: what is +/// being tested is how it behaves across frames as ImGui reports the widget activating, +/// being edited and deactivating, and putting the whole editor on screen to reach it would only +/// add ways for the test to fail for reasons that are not the widget's. +/// +internal sealed class WidgetHarness : IDisposable +{ + /// + /// Gets the harness advancing the frames. + /// + internal ImGuiAppHarness App { get; } + + /// + /// Gets or sets what to draw each frame. Called from inside a live frame, so it may call ImGui + /// freely. + /// + internal Action Draw { get; set; } = () => { }; + + private bool disposed; + + private WidgetHarness(ImGuiAppHarness app) => App = app; + + /// + /// Starts a harness and advances the first frame, which builds the font atlas. + /// + /// The running harness. Dispose it to release the ImGui context. + internal static WidgetHarness Start() + { + WidgetHarness? harness = null; + + ImGuiAppConfig config = new() + { + Title = nameof(WidgetHarness), + OnRender = _ => harness?.Draw(), + }; + + ImGuiAppHarness app = ImGuiAppHarness.Start(config, new HarnessOptions()); + harness = new WidgetHarness(app); + app.Step(); + + return harness; + } + + public void Dispose() + { + if (disposed) + { + return; + } + + disposed = true; + App.Dispose(); + } +} diff --git a/SchemaEditor/AppData.cs b/SchemaEditor/AppData.cs index 6ec5004..e3de098 100644 --- a/SchemaEditor/AppData.cs +++ b/SchemaEditor/AppData.cs @@ -1,5 +1,10 @@ // Copyright (c) 2023-2026 ktsu-dev contributors +// Both test assemblies are named, rather than only the one that reads this project's internals. +// ktsu.Sdk's KTSU0002 requires a non-test project to expose its internals to the repository's test +// projects, and there are two of them now; which of the two a given project actually needs is not +// what the rule is checking. +[assembly: System.Runtime.CompilerServices.InternalsVisibleTo("ktsu.SchemaEditor.Test")] [assembly: System.Runtime.CompilerServices.InternalsVisibleTo("ktsu.Schema.Test")] namespace ktsu.SchemaEditor; diff --git a/SchemaEditor/Program.cs b/SchemaEditor/Program.cs index de6a2ad..375052c 100644 --- a/SchemaEditor/Program.cs +++ b/SchemaEditor/Program.cs @@ -15,18 +15,32 @@ namespace ktsu.SchemaEditor; /// internal static class Program { - private static void Main(string[] _) => - ImGuiApp.Start(new() + /// + /// Builds the host configuration for an editor instance. + /// + /// + /// Split out from , and taking the editor rather than reaching for + /// , so the headless test harness can drive frames through + /// the same configuration the real application runs. A test that assembled its own callbacks + /// would be exercising a parallel host rather than this one, and would keep passing after a + /// callback here was renamed or dropped. + /// + /// The editor the callbacks are bound to. + /// The configuration to hand to . + internal static ImGuiAppConfig CreateConfig(SchemaEditor editor) => + new() { // The startup title only; SchemaEditor keeps it current from there, showing the open // document and whether it has unsaved changes. Title = nameof(SchemaEditor), OnStart = SchemaEditor.OnStart, - OnUpdate = SchemaEditor.Instance.OnTick, - OnRender = SchemaEditor.Instance.OnRender, - OnAppMenu = SchemaEditor.Instance.OnMenu, + OnUpdate = editor.OnTick, + OnRender = editor.OnRender, + OnAppMenu = editor.OnMenu, // Refuses a close that would discard unsaved work, so the editor can ask first. - OnClosing = SchemaEditor.Instance.ShouldClose, - }); + OnClosing = editor.ShouldClose, + }; + + private static void Main(string[] _) => ImGuiApp.Start(CreateConfig(SchemaEditor.Instance)); } diff --git a/SchemaEditor/SchemaEditor.Diagnostics.cs b/SchemaEditor/SchemaEditor.Diagnostics.cs index 91b2f23..64ef597 100644 --- a/SchemaEditor/SchemaEditor.Diagnostics.cs +++ b/SchemaEditor/SchemaEditor.Diagnostics.cs @@ -25,7 +25,7 @@ public partial class SchemaEditor /// frame budget on a schema that has not changed. Debouncing also means a burst of edits - /// deleting a class, then its data source - validates once rather than once per edit. /// - private const float ValidationDebounceSeconds = 0.35f; + internal const float ValidationDebounceSeconds = 0.35f; private bool validationPending; private float timeSinceValidationRequested; @@ -48,7 +48,7 @@ internal void RequestValidation() timeSinceValidationRequested = 0f; } - private void UpdateValidation(float dt) + internal void UpdateValidation(float dt) { if (!validationPending) { @@ -149,7 +149,7 @@ private void ShowDiagnostic(SchemaValidationIssue issue) /// unambiguously split when a name contains a dot. A member selects its owning class, because /// that is the panel its row is drawn in. /// - private void NavigateTo(SchemaValidationIssue issue) + internal void NavigateTo(SchemaValidationIssue issue) { switch (issue.Element) { diff --git a/SchemaEditor/SchemaEditor.Files.cs b/SchemaEditor/SchemaEditor.Files.cs index c62b2ed..bf9c1c7 100644 --- a/SchemaEditor/SchemaEditor.Files.cs +++ b/SchemaEditor/SchemaEditor.Files.cs @@ -103,7 +103,7 @@ private void ShowRecentFilesMenu() /// Run if the user backs out. Only the close path needs this, to release the latch that stops a /// second prompt stacking on the first; New and Open simply do nothing. /// - private void WithUnsavedChangesGuard(Action proceed, Action? onCancel = null) + internal void WithUnsavedChangesGuard(Action proceed, Action? onCancel = null) { if (!HasUnsavedChanges) { @@ -122,7 +122,7 @@ private void WithUnsavedChangesGuard(Action proceed, Action? onCancel = null) }); } - private void New() => WithUnsavedChangesGuard(NewInternal); + internal void New() => WithUnsavedChangesGuard(NewInternal); private void NewInternal() { @@ -137,7 +137,7 @@ private void NewInternal() private void OpenInternal() => Popups.OpenBrowserFileOpen("Open Schema", LoadFrom, "*.schema.json"); - private void LoadFrom(AbsoluteFilePath filePath) + internal void LoadFrom(AbsoluteFilePath filePath) { SchemaLoadResult result = SchemaFile.Load(filePath); @@ -185,7 +185,7 @@ private void Reset() /// appeared has to wait for that answer rather than running immediately. /// /// What to do after a successful save, if anything. - private void SaveThen(Action? continuation) + internal void SaveThen(Action? continuation) { if (string.IsNullOrEmpty(CurrentSchemaPath)) { @@ -267,7 +267,7 @@ internal bool ShouldClose() /// /// Raises the unsaved-changes prompt for a close that refused. /// - private void ProcessCloseRequest() + internal void ProcessCloseRequest() { if (!closeRequested || closePromptShowing) { diff --git a/SchemaTool/Program.cs b/SchemaTool/Program.cs index 06fcc24..e84c333 100644 --- a/SchemaTool/Program.cs +++ b/SchemaTool/Program.cs @@ -1,6 +1,11 @@ // Copyright (c) 2023-2026 ktsu-dev contributors +// Both test assemblies are named, rather than only the one that reads this project's internals. +// ktsu.Sdk's KTSU0002 requires a non-test project to expose its internals to the repository's test +// projects, and there are two of them now; which of the two a given project actually needs is not +// what the rule is checking. [assembly: System.Runtime.CompilerServices.InternalsVisibleTo("ktsu.Schema.Test")] +[assembly: System.Runtime.CompilerServices.InternalsVisibleTo("ktsu.SchemaEditor.Test")] namespace ktsu.SchemaTool; diff --git a/docs/README.md b/docs/README.md index 34d3fcb..7259695 100644 --- a/docs/README.md +++ b/docs/README.md @@ -63,7 +63,8 @@ if (SchemaSerializer.TryDeserialize(File.ReadAllText("user.schema.json"), out Sc - **[Schema](api/schema-core.md)** - Core library containing the schema definition system - **[SchemaEditor](features/schema-editor.md)** - Visual editor application -- **Schema.Test** - MSTest suite for the library (see the [development guide](development/README.md)) +- **SchemaTool** - Command line validator and code generator runner +- **Schema.Test** / **SchemaEditor.Test** - MSTest suites for the library and the editor (see the [development guide](development/README.md)) ## Documentation diff --git a/docs/ROADMAP.md b/docs/ROADMAP.md index 34fc443..5e349cb 100644 --- a/docs/ROADMAP.md +++ b/docs/ROADMAP.md @@ -38,6 +38,9 @@ time. For what the library actually does, the tests in recent-files list. - **CLI** — `SchemaTool` validates a schema or runs its code generators, exiting non-zero on errors so it can gate a build. +- **Editor tests** — `SchemaEditor.Test` drives the editor headlessly through + `ktsu.ImGui.App.Testing`, which rasterizes in software and injects input straight into ImGui, so + the editor's real draw code runs on a continuous integration runner with no window or display. - **CI/CD** — GitHub Actions with build, multi-framework test, SonarCloud analysis, CodeQL, NuGet publishing and winget manifest updates; Dependabot with auto-merge. @@ -49,10 +52,8 @@ substantial items: | Issue | Work | Why it is not done | | --- | --- | --- | | [#110](https://github.com/ktsu-dev/Schema/issues/110) | Implement or delete the unused `Schema.Contracts` API | Needs a decision from the project owner; deleting is a breaking change | -| [#116](https://github.com/ktsu-dev/Schema/issues/116) | Editor window title and close prompt | Blocked by `ktsu.ImGui.App`: `Title` is init-only and there is no cancellable close hook | | [#126](https://github.com/ktsu-dev/Schema/issues/126) | Generated data editors | Builds on the generator architecture | | [#127](https://github.com/ktsu-dev/Schema/issues/127) | Generated data migrations | Needs a schema diff, which does not exist yet | -| [#128](https://github.com/ktsu-dev/Schema/issues/128) | A test harness for the editor | The editor has never had tests; its code needs a live ImGui context to run | ## Phase status @@ -66,16 +67,17 @@ build blocker that made the repository unbuildable on a current SDK ([#123](https://github.com/ktsu-dev/Schema/issues/123)) is fixed, and the test suite now runs against every published target framework rather than just the newest. -### Phase 2 — Editor completeness — **done except two window behaviours** +### Phase 2 — Editor completeness — **done** Renaming, descriptions, the code generator panel, member reordering, validation surfacing, undo coverage, Save As, dirty tracking and recent files have landed. Cross-platform "Open Externally" is fixed. -Outstanding: the window title cannot reflect the open document and the window's close button -cannot prompt to save, because `ktsu.ImGui.App` exposes neither a settable title nor a cancellable -close hook. The document name, dirty marker and a guarded Exit item live in the menu bar instead. -Both would be small upstream changes to that package. +The window title and the close prompt ([#116](https://github.com/ktsu-dev/Schema/issues/116)) were +blocked on `ktsu.ImGui.App` exposing neither a settable title nor a cancellable close hook; both +landed upstream, and the editor now keeps its title current and refuses a close that would discard +unsaved work. The document name and dirty marker are still drawn in the menu bar as well, because a +maximised title bar is easy to overlook and a tiling window manager may not draw one at all. ### Phase 3 — Code generation — **done, less the deferred targets** @@ -98,16 +100,25 @@ The `.schema.json` format is documented and versioned with a stated compatibilit Outstanding: editor packaging via winget, and cutting the v2.0 milestone. +### Phase 6 — Test coverage — **the editor is now testable** + +Not one of the original phases; added when the editor grew large enough to need one. + +`SchemaEditor.Test` ([#128](https://github.com/ktsu-dev/Schema/issues/128)) drives the editor +headlessly and covers the recent-files list, the commit-once text field, the unsaved-changes guard +and the save-then-continue sequence, and validation debouncing and click-to-navigate. The +SonarCloud coverage exclusion has narrowed from the whole application to the panel and tree files, +which are still pure draw code. + ## What to do next | Order | Work item | Effort | Rationale | | ----- | --- | --- | --- | | 1 | Decide [#110](https://github.com/ktsu-dev/Schema/issues/110): implement or delete `Schema.Contracts` | S | A decision, not a build. It is public API on a published package that nothing implements, and `docs/examples/dependency-injection.md` documents it as though it works | -| 2 | [#128](https://github.com/ktsu-dev/Schema/issues/128): a test harness for the editor | M | The editor is now the largest untested surface, and it grew a lot recently | -| 3 | [#126](https://github.com/ktsu-dev/Schema/issues/126): generated data editors | L | The first thing the data source binding was for | -| 4 | Upstream the two `ktsu.ImGui.App` changes, then finish [#116](https://github.com/ktsu-dev/Schema/issues/116) | S | Small change in another repository, then a small change here | -| 5 | [#127](https://github.com/ktsu-dev/Schema/issues/127): generated migrations | L | Needs a schema diff first; the largest remaining design problem | -| 6 | Editor packaging and the v2.0 milestone | M | Ship it | +| 2 | [#126](https://github.com/ktsu-dev/Schema/issues/126): generated data editors | L | The first thing the data source binding was for | +| 3 | Extend `SchemaEditor.Test` to the panel and tree files | M | The harness exists; those files are what it does not reach yet, and they are the ones still excluded from coverage | +| 4 | [#127](https://github.com/ktsu-dev/Schema/issues/127): generated migrations | L | Needs a schema diff first; the largest remaining design problem | +| 5 | Editor packaging and the v2.0 milestone | M | Ship it | ## Decisions diff --git a/docs/development/README.md b/docs/development/README.md index e789ef1..fa104f7 100644 --- a/docs/development/README.md +++ b/docs/development/README.md @@ -43,11 +43,13 @@ dotnet run --project SchemaEditor | Directory | Purpose | | --------------- | ------------------------------ | -| `Schema/` | Core schema definition library | -| `Schema.Test/` | MSTest unit tests | -| `SchemaEditor/` | ImGui-based visual editor | -| `docs/` | Markdown documentation | -| `scripts/` | Build automation (PSBuild) | +| `Schema/` | Core schema definition library | +| `Schema.Test/` | MSTest unit tests for the library | +| `SchemaEditor/` | ImGui-based visual editor | +| `SchemaEditor.Test/` | Headless UI tests for the editor | +| `SchemaTool/` | Command line validator and generator | +| `docs/` | Markdown documentation | +| `scripts/` | Build automation (PSBuild) | Within the core library: @@ -58,7 +60,43 @@ Within the core library: ## Testing -Tests live in `Schema.Test` and use MSTest with the Microsoft.Testing.Platform runner. Run them with `dotnet test`. All new features should include tests; the existing suites (`SchemaTests`, `SchemaClassTests`, `SchemaEnumTests`, `TypeSystemTests`, `SchemaSerializerTests`, `AddClassFromTypeTests`) show the conventions in use. +Both suites use MSTest with the Microsoft.Testing.Platform runner, and `dotnet test` runs them +together. All new features should include tests. + +### The library — `Schema.Test` + +Plain unit tests over the core library, run against every framework it publishes. The existing +suites (`SchemaTests`, `SchemaClassTests`, `SchemaEnumTests`, `TypeSystemTests`, +`SchemaSerializerTests`, `AddClassFromTypeTests`) show the conventions in use. + +### The editor — `SchemaEditor.Test` + +The editor's code is immediate-mode draw calls, so none of it executes without a live ImGui +context. `ktsu.ImGui.App.Testing` supplies one with no window, no display and no GPU: it rasterizes +in software and injects input straight into ImGui's event queue, so these tests run unchanged on a +headless continuous integration runner and disturb nothing on a desktop. + +Two fixtures wrap it: + +- **`EditorHarness`** starts a real editor from `Program.CreateConfig`, the same configuration + the application itself starts with, so a callback renamed or dropped there breaks the tests + rather than leaving them passing against a host that no longer exists. It redirects + `AppDataStorage` to an in-memory file system first, so a test never reads or writes the + settings of whoever is running it. +- **`WidgetHarness`** draws one widget and nothing else, for widget-level behaviour such as + `EditField`. + +Frames are advanced by the test, never by wall-clock time — `Step(n)` for a fixed number and +`StepUntil(condition, budget)` for a wait — so a loaded runner is slower rather than flakier. +Widgets are addressed by name through `App.Probe` and `App.Click(name)` where the widget library +records them, and by measured rectangle otherwise. Where a regression is only visible on screen, +`App.Capture()` gives the rendered pixels: `TwoRowsSharingALabelDoNotShareABuffer` compares one +row's pixels before and during an edit of its sibling, which is the only place the shared-buffer +bug it guards against is observable at all. + +Note that a modal sizes itself on the frame it appears and is centred on the next, so a button's +rectangle is not final until a few frames after it is first recorded. Step past that before +clicking. ## Code Style From 7c3afcbc861cb9a8d1dc9fcf35fdf358736e3a7a Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 04:16:38 +0000 Subject: [PATCH 3/5] Make the test scratch paths provably leaf-relative Three findings from the code quality bot, all the same rule: Path.Combine silently discards everything before a rooted segment, so a name that turns out not to be a leaf puts the file somewhere other than the scratch directory - and so outside what the cleanup deletes. Every name involved is a literal or a hex GUID today, so none of the three can actually be rooted. Reducing each to its leaf first is what says so in the code rather than in the caller's head, and costs nothing. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Jc8o5zF3cmfCGjzWvQdGDE --- SchemaEditor.Test/DocumentGuardTests.cs | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/SchemaEditor.Test/DocumentGuardTests.cs b/SchemaEditor.Test/DocumentGuardTests.cs index 036b94e..0ab4b7a 100644 --- a/SchemaEditor.Test/DocumentGuardTests.cs +++ b/SchemaEditor.Test/DocumentGuardTests.cs @@ -32,7 +32,8 @@ public void StartEditor() harness = EditorHarness.Start(); // A real directory, not the in-memory one: SchemaFile saves through System.IO directly. - scratchDirectory = Path.Combine(Path.GetTempPath(), $"schema-editor-tests-{Guid.NewGuid():N}"); + string scratchDirectoryName = $"schema-editor-tests-{Guid.NewGuid():N}"; + scratchDirectory = Path.Combine(Path.GetTempPath(), Path.GetFileName(scratchDirectoryName)); Directory.CreateDirectory(scratchDirectory); } @@ -51,8 +52,18 @@ public void StopEditor() } } + /// + /// A path inside this test's scratch directory. + /// + /// + /// The name is reduced to its leaf first. discards + /// everything before a rooted segment, so combining an unreduced name would put the file + /// somewhere other than the scratch directory - and so outside what + /// deletes. + /// + /// The file name, which must be a leaf. private AbsoluteFilePath ScratchFile(string name) => - Path.Combine(scratchDirectory, name).As(); + Path.Combine(scratchDirectory, Path.GetFileName(name)).As(); /// /// Opens a document and makes an edit through the undo service, which is what @@ -197,7 +208,7 @@ public void AFailedSaveDoesNotRunTheAction() // A path whose parent is an existing file rather than a directory cannot be created. AbsoluteFilePath blocker = ScratchFile("blocker"); File.WriteAllText(blocker, "not a directory"); - harness.Editor.CurrentSchemaPath = Path.Combine(blocker, "nested.schema.json").As(); + harness.Editor.CurrentSchemaPath = Path.Combine(blocker.ToString(), "nested.schema.json").As(); bool proceeded = false; harness.Editor.SaveThen(() => proceeded = true); From 99e1ee714347ed359959d060470242a2a565f058 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 04:20:27 +0000 Subject: [PATCH 4/5] Compose the test scratch paths with the typed path API Reducing each name to its leaf did not satisfy the rule - the bot flagged all three sites again - because Path.Combine cannot be proven safe from the shape of its arguments at the call site. So there is no Path.Combine in the file now. ktsu.Semantics.Paths composes with '/' over FileName and DirectoryName, neither of which can be rooted, so the scratch directory cannot be dropped by construction rather than by the caller passing the right thing. scratchDirectory becomes an AbsoluteDirectoryPath, which is what it always was. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Jc8o5zF3cmfCGjzWvQdGDE --- SchemaEditor.Test/DocumentGuardTests.cs | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/SchemaEditor.Test/DocumentGuardTests.cs b/SchemaEditor.Test/DocumentGuardTests.cs index 0ab4b7a..cf80a84 100644 --- a/SchemaEditor.Test/DocumentGuardTests.cs +++ b/SchemaEditor.Test/DocumentGuardTests.cs @@ -24,7 +24,7 @@ namespace ktsu.SchemaEditor.Test; public sealed class DocumentGuardTests { private EditorHarness harness = null!; - private string scratchDirectory = null!; + private AbsoluteDirectoryPath scratchDirectory = null!; [TestInitialize] public void StartEditor() @@ -32,8 +32,7 @@ public void StartEditor() harness = EditorHarness.Start(); // A real directory, not the in-memory one: SchemaFile saves through System.IO directly. - string scratchDirectoryName = $"schema-editor-tests-{Guid.NewGuid():N}"; - scratchDirectory = Path.Combine(Path.GetTempPath(), Path.GetFileName(scratchDirectoryName)); + scratchDirectory = Path.GetTempPath().As() / $"schema-editor-tests-{Guid.NewGuid():N}".As(); Directory.CreateDirectory(scratchDirectory); } @@ -56,14 +55,14 @@ public void StopEditor() /// A path inside this test's scratch directory. /// /// - /// The name is reduced to its leaf first. discards - /// everything before a rooted segment, so combining an unreduced name would put the file - /// somewhere other than the scratch directory - and so outside what - /// deletes. + /// Composed with the '/' operator from ktsu.Semantics.Paths rather than + /// Path.Combine, which silently discards everything before a rooted segment - so a + /// name that was not a leaf would put the file somewhere other than the scratch directory, + /// and so outside what deletes. A cannot be + /// rooted, which is what makes the composition safe rather than merely intended to be. /// - /// The file name, which must be a leaf. - private AbsoluteFilePath ScratchFile(string name) => - Path.Combine(scratchDirectory, Path.GetFileName(name)).As(); + /// The file name. + private AbsoluteFilePath ScratchFile(string name) => scratchDirectory / name.As(); /// /// Opens a document and makes an edit through the undo service, which is what @@ -206,9 +205,11 @@ public void AFailedSaveDoesNotRunTheAction() harness.Editor.CurrentSchema = new Schema(); // A path whose parent is an existing file rather than a directory cannot be created. - AbsoluteFilePath blocker = ScratchFile("blocker"); + const string blockerName = "blocker"; + AbsoluteFilePath blocker = ScratchFile(blockerName); File.WriteAllText(blocker, "not a directory"); - harness.Editor.CurrentSchemaPath = Path.Combine(blocker.ToString(), "nested.schema.json").As(); + harness.Editor.CurrentSchemaPath = + scratchDirectory / blockerName.As() / "nested.schema.json".As(); bool proceeded = false; harness.Editor.SaveThen(() => proceeded = true); From b7c1bcafaa6fabee836c8f4a801a911e1f6e0cf7 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 04:37:06 +0000 Subject: [PATCH 5/5] Split the entry point from the host configuration it builds SonarCloud's gate failed at 66.7% coverage on new code. The whole of that shortfall was one statement: Main, which calls ImGuiApp.Start, opens a window and does not return. It cannot be executed by a test, and Sonar's exclusions are per file, so it was dragging CreateConfig - which the harness does drive - down with it. So CreateConfig moves to EditorHost.cs, and Program.cs holds nothing but Main. The coverage exclusion names Program.cs, which now makes an honest claim: not "nobody has tested this yet", but "this cannot run outside a real window". The configuration itself stays measured, and is covered. That is the same separation Program.cs already argued for one level up - the host's contract with ImGuiApp does not belong in the editor class - applied again: describing the host is not the same as starting it. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Jc8o5zF3cmfCGjzWvQdGDE --- .github/workflows/dotnet.yml | 5 +++- CLAUDE.md | 3 +- SchemaEditor.Test/EditorHarness.cs | 4 +-- SchemaEditor/EditorHost.cs | 47 ++++++++++++++++++++++++++++++ SchemaEditor/Program.cs | 37 ++++------------------- docs/development/README.md | 2 +- 6 files changed, 61 insertions(+), 37 deletions(-) create mode 100644 SchemaEditor/EditorHost.cs diff --git a/.github/workflows/dotnet.yml b/.github/workflows/dotnet.yml index dd13017..1fc0574 100644 --- a/.github/workflows/dotnet.yml +++ b/.github/workflows/dotnet.yml @@ -370,7 +370,10 @@ jobs: # reports, with nothing to assert that is not a pixel. SchemaEditor.Test now drives the # rest of the editor headlessly, so the rest is measured; this list is what that # harness does not yet reach, not the whole application. - '/d:sonar.coverage.exclusions=**/*Test*.cs,**/*.Tests.cs,**/*.Tests/**/*,**/obj/**/*,**/*.dll,**/NativeExports.cs,SchemaEditor/ButtonTree.cs,SchemaEditor/ClassGraphView.cs,SchemaEditor/CodeGeneratorPanel.cs,SchemaEditor/SchemaEditor.Panels.cs,SchemaEditor/Tree*.cs' + # SchemaEditor/Program.cs is the one file here excluded because it cannot be executed + # rather than because nobody has yet: it holds only Main, which opens a window and does + # not return. What the host is configured with lives in EditorHost.cs, which is tested. + '/d:sonar.coverage.exclusions=**/*Test*.cs,**/*.Tests.cs,**/*.Tests/**/*,**/obj/**/*,**/*.dll,**/NativeExports.cs,SchemaEditor/Program.cs,SchemaEditor/ButtonTree.cs,SchemaEditor/ClassGraphView.cs,SchemaEditor/CodeGeneratorPanel.cs,SchemaEditor/SchemaEditor.Panels.cs,SchemaEditor/Tree*.cs' '/d:sonar.cs.vstest.reportsPaths=coverage/**/*.trx' '/d:sonar.exclusions=**/NativeExports.cs' ) diff --git a/CLAUDE.md b/CLAUDE.md index 1aaa7e8..8133b52 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -81,7 +81,8 @@ Schema elements maintain parent references via `AssociateWith()` methods. After - `Schema/Models/Types/BaseType.cs` - Abstract base with `[JsonDerivedType]` attributes for polymorphic serialization - `Schema/Models/SchemaClass.cs` - Class definitions containing `SchemaMember` collections - `SchemaEditor/SchemaEditor.cs` - Main editor application using `ktsu.ImGui.App` -- `SchemaEditor/Program.cs` - Builds the `ImGuiAppConfig`; `CreateConfig` is what the tests drive too +- `SchemaEditor/EditorHost.cs` - Builds the `ImGuiAppConfig`; `CreateConfig` is what the tests drive too +- `SchemaEditor/Program.cs` - The entry point, and the only file excluded from coverage measurement - `SchemaEditor.Test/EditorHarness.cs` - Runs a real editor headlessly, frames advanced by the test - `SchemaEditor.Test/WidgetHarness.cs` - A headless frame containing only the widget under test diff --git a/SchemaEditor.Test/EditorHarness.cs b/SchemaEditor.Test/EditorHarness.cs index 08c4ab0..62037ad 100644 --- a/SchemaEditor.Test/EditorHarness.cs +++ b/SchemaEditor.Test/EditorHarness.cs @@ -20,7 +20,7 @@ namespace ktsu.SchemaEditor.Test; /// continuous integration runner. /// /// -/// The configuration comes from , the same one the application +/// The configuration comes from , the same one the application /// starts with, so a callback renamed or dropped there breaks these tests instead of leaving them /// passing against a host that no longer exists. /// @@ -61,7 +61,7 @@ internal static EditorHarness Start() ktsu.AppDataStorage.AppData.ConfigureForTesting(() => new MockFileSystem()); SchemaEditor editor = new(); - ImGuiAppHarness app = ImGuiAppHarness.Start(Program.CreateConfig(editor), new HarnessOptions()); + ImGuiAppHarness app = ImGuiAppHarness.Start(EditorHost.CreateConfig(editor), new HarnessOptions()); // The first frame builds the font atlas and lays the panels out; nothing is measurable // before it has run. diff --git a/SchemaEditor/EditorHost.cs b/SchemaEditor/EditorHost.cs new file mode 100644 index 0000000..242fd62 --- /dev/null +++ b/SchemaEditor/EditorHost.cs @@ -0,0 +1,47 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.SchemaEditor; + +using ktsu.ImGui.App; + +/// +/// The host configuration an editor runs under. +/// +/// +/// Separate from because describing the host is not part of editing a +/// schema: the configuration names a windowing framework and a delegate type for each callback, +/// and holding all of that in the editor class counts against its coupling budget without earning +/// anything. It also keeps the whole of the host's contract with ImGuiApp readable in one place. +/// +/// Separate from for the same reason one level down: describing the host is +/// not the same as starting it. Describing it is testable and tested; starting it opens a window +/// and does not return. +/// +internal static class EditorHost +{ + /// + /// Builds the host configuration for an editor instance. + /// + /// + /// Takes the editor rather than reaching for so the + /// headless test harness can drive frames through the same configuration the real application + /// runs. A test that assembled its own callbacks would be exercising a parallel host rather + /// than this one, and would keep passing after a callback here was renamed or dropped. + /// + /// The editor the callbacks are bound to. + /// The configuration to hand to . + internal static ImGuiAppConfig CreateConfig(SchemaEditor editor) => + new() + { + // The startup title only; SchemaEditor keeps it current from there, showing the open + // document and whether it has unsaved changes. + Title = nameof(SchemaEditor), + OnStart = SchemaEditor.OnStart, + OnUpdate = editor.OnTick, + OnRender = editor.OnRender, + OnAppMenu = editor.OnMenu, + + // Refuses a close that would discard unsaved work, so the editor can ask first. + OnClosing = editor.ShouldClose, + }; +} diff --git a/SchemaEditor/Program.cs b/SchemaEditor/Program.cs index 375052c..3c63793 100644 --- a/SchemaEditor/Program.cs +++ b/SchemaEditor/Program.cs @@ -8,39 +8,12 @@ namespace ktsu.SchemaEditor; /// The application entry point. /// /// -/// Separate from because starting the application is not part of editing -/// a schema: the configuration names a windowing framework and a delegate type for each callback, -/// and holding all of that in the editor class counts against its coupling budget without earning -/// anything. It also keeps the whole of the host's contract with ImGuiApp readable in one place. +/// This file holds the one statement that cannot be executed by a test: it opens a window and does +/// not return until the application closes. That is why it is a file of its own, and the only part +/// of the editor excluded from coverage measurement - what the host is configured with lives in +/// , which the tests drive. /// internal static class Program { - /// - /// Builds the host configuration for an editor instance. - /// - /// - /// Split out from , and taking the editor rather than reaching for - /// , so the headless test harness can drive frames through - /// the same configuration the real application runs. A test that assembled its own callbacks - /// would be exercising a parallel host rather than this one, and would keep passing after a - /// callback here was renamed or dropped. - /// - /// The editor the callbacks are bound to. - /// The configuration to hand to . - internal static ImGuiAppConfig CreateConfig(SchemaEditor editor) => - new() - { - // The startup title only; SchemaEditor keeps it current from there, showing the open - // document and whether it has unsaved changes. - Title = nameof(SchemaEditor), - OnStart = SchemaEditor.OnStart, - OnUpdate = editor.OnTick, - OnRender = editor.OnRender, - OnAppMenu = editor.OnMenu, - - // Refuses a close that would discard unsaved work, so the editor can ask first. - OnClosing = editor.ShouldClose, - }; - - private static void Main(string[] _) => ImGuiApp.Start(CreateConfig(SchemaEditor.Instance)); + private static void Main(string[] _) => ImGuiApp.Start(EditorHost.CreateConfig(SchemaEditor.Instance)); } diff --git a/docs/development/README.md b/docs/development/README.md index fa104f7..864822d 100644 --- a/docs/development/README.md +++ b/docs/development/README.md @@ -78,7 +78,7 @@ headless continuous integration runner and disturb nothing on a desktop. Two fixtures wrap it: -- **`EditorHarness`** starts a real editor from `Program.CreateConfig`, the same configuration +- **`EditorHarness`** starts a real editor from `EditorHost.CreateConfig`, the same configuration the application itself starts with, so a callback renamed or dropped there breaks the tests rather than leaving them passing against a host that no longer exists. It redirects `AppDataStorage` to an in-memory file system first, so a test never reads or writes the