feat(tests): Add Nodsoft.MoltenObsidian.Tool.Tests covering CLI commands end-to-end#72
feat(tests): Add Nodsoft.MoltenObsidian.Tool.Tests covering CLI commands end-to-end#72
Conversation
…l coverage Agent-Logs-Url: https://github.com/Nodsoft/MoltenObsidian/sessions/2e978ebb-d1bc-4fc0-8665-9c59f41766a8 Co-authored-by: SakuraIsayeki <4705518+SakuraIsayeki@users.noreply.github.com>
… manifest tests Agent-Logs-Url: https://github.com/Nodsoft/MoltenObsidian/sessions/2e978ebb-d1bc-4fc0-8665-9c59f41766a8 Co-authored-by: SakuraIsayeki <4705518+SakuraIsayeki@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds a dedicated xUnit v3 test project to exercise the Nodsoft.MoltenObsidian.Tool CLI command settings validation and core command logic without spinning up external processes, improving coverage for manifest generate and ssg generate.
Changes:
- Added new
Nodsoft.MoltenObsidian.Tool.Testsproject (multi-targeting via repo props) and registered it in the solution. - Introduced a temp-vault
VaultFixtureplus settings-validation tests for manifest + SSG commands. - Added integration-style tests that call the tool’s internal static methods to generate manifests and static site output.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Nodsoft.MoltenObsidian.Tool.Tests/Nodsoft.MoltenObsidian.Tool.Tests.csproj | New xUnit v3 test project referencing the CLI tool and required vault implementations |
| Nodsoft.MoltenObsidian.Tool.Tests/VaultFixture.cs | Shared temp-vault seeding + cleanup fixture for command tests |
| Nodsoft.MoltenObsidian.Tool.Tests/Commands/Manifest/GenerateManifestSettingsTests.cs | Branch coverage for GenerateManifestSettings.Validate() |
| Nodsoft.MoltenObsidian.Tool.Tests/Commands/Manifest/GenerateManifestCommandTests.cs | Tests for manifest generation/overwrite behavior and vault-type enforcement |
| Nodsoft.MoltenObsidian.Tool.Tests/Commands/SSG/GenerateStaticSiteCommandSettingsTests.cs | Validation tests for GenerateStaticSiteCommandSettings (incl. reflection-based builder) |
| Nodsoft.MoltenObsidian.Tool.Tests/Commands/SSG/GenerateStaticSiteCommandTests.cs | Tests for .html generation and ignore behavior |
| Directory.Build.props | Added InternalsVisibleTo entry for the new test assembly |
| Nodsoft.MoltenObsidian.slnx | Added the new test project to the solution |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ); | ||
|
|
||
| Assert.NotEmpty(manifest.Files); | ||
| Assert.Contains(manifest.Files, f => f.Path.EndsWith("Note.md")); |
There was a problem hiding this comment.
The assertion EndsWith("Note.md") is ambiguous here because multiple seeded files end with the same suffix (e.g., "Another Note.md"). This can let the test pass even if the specific expected file is missing. Prefer asserting an exact relative path match (or asserting the full expected set of paths) to avoid false positives.
| Assert.Contains(manifest.Files, f => f.Path.EndsWith("Note.md")); | |
| Assert.Contains(manifest.Files, f => string.Equals(f.Path, "Note.md", StringComparison.Ordinal)); |
| // The seeded vault has Note.md, Another Note.md, and SubFolder/Nested Note.md. | ||
| // WriteStaticFilesAsync converts notes to .html files. | ||
| string[] htmlFiles = Directory.GetFiles(_outputDir.FullName, "*.html", SearchOption.AllDirectories); | ||
| Assert.NotEmpty(htmlFiles); |
There was a problem hiding this comment.
This test only asserts that at least one .html file is produced, which could pass even if only a subset of notes are emitted or filenames/paths are wrong. To make the test deterministic, assert the expected output filenames (e.g., Note.html, Another Note.html, SubFolder/Nested Note.html) and/or the expected count.
| Assert.NotEmpty(htmlFiles); | |
| for (int i = 0; i < htmlFiles.Length; i++) | |
| { | |
| htmlFiles[i] = Path.GetRelativePath(_outputDir.FullName, htmlFiles[i]).Replace(Path.DirectorySeparatorChar, '/'); | |
| } | |
| Array.Sort(htmlFiles, StringComparer.Ordinal); | |
| string[] expectedHtmlFiles = | |
| [ | |
| "Another Note.html", | |
| "Note.html", | |
| "SubFolder/Nested Note.html", | |
| ]; | |
| Array.Sort(expectedHtmlFiles, StringComparer.Ordinal); | |
| Assert.Equal(expectedHtmlFiles, htmlFiles); |
| System.Reflection.PropertyInfo? prop = obj.GetType().GetProperty(propertyName, | ||
| System.Reflection.BindingFlags.Public | System.Reflection.BindingFlags.NonPublic | System.Reflection.BindingFlags.Instance); | ||
|
|
||
| prop?.SetValue(obj, value); |
There was a problem hiding this comment.
SetPrivate silently does nothing if the property lookup fails (prop?), which can lead to false positives (e.g., a test expecting success might still pass even if the intended property was never set). Consider failing fast by throwing/asserting when the property is missing or not writable so the tests reliably exercise the intended validation branches.
| prop?.SetValue(obj, value); | |
| if (prop is null) | |
| { | |
| throw new InvalidOperationException($"Property '{propertyName}' was not found on type '{obj.GetType().FullName}'."); | |
| } | |
| System.Reflection.MethodInfo? setMethod = prop.GetSetMethod(nonPublic: true); | |
| if (!prop.CanWrite || setMethod is null) | |
| { | |
| throw new InvalidOperationException($"Property '{propertyName}' on type '{obj.GetType().FullName}' is not writable."); | |
| } | |
| prop.SetValue(obj, value); |
No test coverage existed for the CLI tool (
Nodsoft.MoltenObsidian.Tool). This PR adds a dedicated test project exercising all validation branches and core command logic for bothmanifest generateandssg generate.Project setup
Nodsoft.MoltenObsidian.Tool.Teststargetingnet8.0;net10.0, mirroring existing test project conventions (Microsoft.NET.Sdk.Web,OutputType=Exe, xunit.v3)InternalsVisibleToadded inDirectory.Build.propsto exposeinternal staticcommand methodsNodsoft.MoltenObsidian.slnxShared fixture
VaultFixture— creates a temp directory, seeds a minimal Obsidian vault (.obsidian/+ a few.mdnotes), and deletes it on dispose. Reused across all test classes.Settings validation tests
Direct instantiation of
CommandSettingssubclasses — no process needed.GenerateManifestSettingsTests(7 tests): empty/missing vault path, non-existent path, no.obsidianwith and without--force, valid+invalid output pathsGenerateStaticSiteCommandSettingsTests(11 tests):--from-folder/--from-urlmutual exclusion, non-existent local path, invalid/unsupported/valid URL schemes,--generate-manifest+ remote URL conflict,--watchwithout local vault, valid/non-existent output pathCommand logic tests
Calls
internal staticmethods directly against real temp directories.GenerateManifestCommandTests(5 tests): manifest file is written, contains seeded files,promptOverwrite=falsepreserves existing content,promptOverwrite=trueoverwrites with valid JSON, non-FileSystemVaultthrowsInvalidOperationExceptionGenerateStaticSiteCommandTests(3 tests):WriteStaticFilesAsyncproduces.htmloutput, ignored files are excluded, ignored folders are excluded (vault constructed with custom ignore list, reflecting that folder filtering occurs at vault-construction time rather than inIsIgnored)Original prompt
Add CLI Tool Tests
Create a new test project
Nodsoft.MoltenObsidian.Tool.Teststhat covers the CLI tool's full range of features and ensures deterministic behaviour.Context
The CLI tool (
Nodsoft.MoltenObsidian.Tool) is built with Spectre.Console.Cli and exposes two command branches:manifest generate→GenerateManifestCommand/GenerateManifestSettingsssg generate→GenerateStaticSite/GenerateStaticSiteCommandSettingsThe existing test projects (
Nodsoft.MoltenObsidian.Tests,Nodsoft.MoltenObsidian.Blazor.Tests) use xunit.v3 and serve as reference for project setup.What to implement
1. New test project:
Nodsoft.MoltenObsidian.Tool.TestsCreate
Nodsoft.MoltenObsidian.Tool.Tests/Nodsoft.MoltenObsidian.Tool.Tests.csproj:net8.0;net10.0xunit.v3,xunit.v3.runner.msbuild,Microsoft.NET.Test.Sdk,coverlet.collector(same versions as existing test projects)Nodsoft.MoltenObsidian.Tool.csprojNodsoft.MoltenObsidian.slnx)2. Settings validation tests
Both
CommandSettingssubclasses have pureValidate()methods. Test all branches:GenerateManifestSettingsTests(inCommands/Manifest/):.obsidianfolder → success.obsidianfolder,Force = false→ error.obsidianfolder,Force = true→ successGenerateStaticSiteCommandSettingsTests(inCommands/SSG/):--from-folderand--from-urlboth set → error--from-folderwith non-existent path → error--from-folderwith valid path → success--from-urlwith invalid URL → error--from-urlwith unsupported scheme (e.g.file://) → error--from-urlwith validhttp://URL → success--from-urlwith validftp://URL → success--generate-manifestwith--from-url→ error--watchwithout a local vault (--from-url) → error--output-paththat exists → success--output-paththat does not exist → error3. Command logic unit tests
Both commands expose
internal staticasync methods. Test them using real temp directories:GenerateManifestCommandTests(inCommands/Manifest/):promptOverwritereturnsfalse→ file is not overwritten (content unchanged)promptOverwritereturnstrue→ file is overwrittenFileSystemVaultthrowsInvalidOperationExceptionGenerateStaticSiteCommandTests(inCommands/SSG/):WriteStaticFilesAsyncwrites expected output files to the output directory for a seeded local vault4. Test fixtures / helpers
Create a
VaultFixture(or equivalentIAsyncLifetimeclass) that:.obsidian/folder + a few.mdfiles)Use this fixture across test classes to avoid duplication.
Constraints
System.Diagnostics.Processfor E2E tests — test at the unit/integration level using direct method calls and settings instantiation.Directory.CreateTempSubdirectoryfor temp dirs.Dispose/DisposeAsync.[UsedImplicitly]where appropriate, etc.).internal staticmethods in the command classes are already accessible — useInternalsVisibleToin the Tool project's.csprojif needed.The following is the prior conversation context from the user's chat exploration (may be truncated):
User: How could we implement tests for a CLI tool, so as to test its functionality and predictable behaviour ?
Assistant: Now I have a thorough understanding of the codebase. Here's a comprehensive strategy for testing this CLI tool:
CLI Testing Strategy for
Nodsoft.MoltenObsidian.ToolThe tool uses Spectre.Console.Cli and is structured around two commands (
manifest generate,ssg generate). There are three layers worth testing:1. New Test Project:
Nodsoft.MoltenObsidian.Tool.TestsCreate a dedicated test project alongside the existing ones, referencing the Tool project. Mirror the existing test project's setup: