test: Extended Command Help and Command Build tests #31
Conversation
samtrion
added
state:ready for merge
WalkthroughThe pull request adds Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes The changes follow consistent patterns: a single option registration, straightforward test helper updates with conditional logic, and numerous homogeneous snapshot file updates (predominantly adding the same error field). However, note:
Poem
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #31 +/- ##
==========================================
+ Coverage 82.75% 85.05% +2.29%
==========================================
Files 4 4
Lines 87 87
Branches 2 2
==========================================
+ Hits 72 74 +2
+ Misses 15 13 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/NetEvolve.ForgingBlazor.Tests.Integration/Predefined.cs (1)
12-20: Addusing static VerifyTests.Verifier;or qualify the call asVerifier.DerivePathInfo(...).The unqualified
DerivePathInfo(...)call on line 12 will not resolve without a static import. The current file has onlyusing VerifyTests;, which does not bringDerivePathInfointo scope. Either addusing static VerifyTests.Verifier;at the top of the file, or qualify the call asVerifier.DerivePathInfo(...). For module-level configuration,VerifierSettings.DerivePathInfo(...)is also a valid global alternative.
🧹 Nitpick comments (1)
tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet10_0/Help_Theory_Expected.CommandHelpTests_cf823c7fea89c954.verified.txt (1)
5-5: Emptyerrorfield inclusion is consistent; watch for snapshot brittleness
Iferror: ,varies across frameworks/line-endings, consider normalizing empty error to""/nullin the test data before verification.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (16)
src/NetEvolve.ForgingBlazor/Commands/CommandBuild.cs(1 hunks)tests/NetEvolve.ForgingBlazor.Tests.Integration/Commands/CommandBuildTests.cs(1 hunks)tests/NetEvolve.ForgingBlazor.Tests.Integration/Commands/CommandHelpTests.cs(1 hunks)tests/NetEvolve.ForgingBlazor.Tests.Integration/Predefined.cs(2 hunks)tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet10_0/Help_Theory_Expected.CommandHelpTests_25a03f3b48369ce7.verified.txt(1 hunks)tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet10_0/Help_Theory_Expected.CommandHelpTests_3cb5690aa64d0539.verified.txt(2 hunks)tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet10_0/Help_Theory_Expected.CommandHelpTests_7482efceccfc847a.verified.txt(1 hunks)tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet10_0/Help_Theory_Expected.CommandHelpTests_a46afb280312df84.verified.txt(1 hunks)tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet10_0/Help_Theory_Expected.CommandHelpTests_cf823c7fea89c954.verified.txt(1 hunks)tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet10_0/Help_Theory_Expected.CommandHelpTests_d2a59b5932a503ab.verified.txt(1 hunks)tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet9_0/Help_Theory_Expected.CommandHelpTests_25a03f3b48369ce7.verified.txt(1 hunks)tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet9_0/Help_Theory_Expected.CommandHelpTests_3cb5690aa64d0539.verified.txt(2 hunks)tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet9_0/Help_Theory_Expected.CommandHelpTests_7482efceccfc847a.verified.txt(1 hunks)tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet9_0/Help_Theory_Expected.CommandHelpTests_a46afb280312df84.verified.txt(1 hunks)tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet9_0/Help_Theory_Expected.CommandHelpTests_cf823c7fea89c954.verified.txt(1 hunks)tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet9_0/Help_Theory_Expected.CommandHelpTests_d2a59b5932a503ab.verified.txt(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/NetEvolve.ForgingBlazor/Commands/CommandBuild.cs (1)
src/NetEvolve.ForgingBlazor.Extensibility/Commands/CommandOptions.cs (1)
CommandOptions(13-173)
tests/NetEvolve.ForgingBlazor.Tests.Integration/Commands/CommandHelpTests.cs (1)
src/NetEvolve.ForgingBlazor/Application.cs (2)
Application(16-89)Application(52-56)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Build & Tests / Run Tests (ubuntu-latest) / Testing .NET solution
- GitHub Check: Build & Tests / Run Tests (windows-latest) / Testing .NET solution
🔇 Additional comments (10)
tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet10_0/Help_Theory_Expected.CommandHelpTests_a46afb280312df84.verified.txt (1)
3-6: Snapshot correctly asserts stderr for missing command scenario
Capturing and verifying the error text here is valuable to prevent regressions in CLI diagnostics.tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet10_0/Help_Theory_Expected.CommandHelpTests_7482efceccfc847a.verified.txt (1)
6-6: Consistent snapshot shape update (errorcaptured for help output)
No concerns with addingerroralongsideargsandoutput.tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet9_0/Help_Theory_Expected.CommandHelpTests_7482efceccfc847a.verified.txt (1)
6-6: DotNet9_0 snapshot aligned with DotNet10_0 (errorfield added)
Keeps cross-target verification consistent.tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet9_0/Help_Theory_Expected.CommandHelpTests_25a03f3b48369ce7.verified.txt (1)
6-6: Snapshot update looks correct (empty stderr captured)tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet9_0/Help_Theory_Expected.CommandHelpTests_cf823c7fea89c954.verified.txt (1)
5-5: Global help snapshot now includes stderr channel (empty)tests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet10_0/Help_Theory_Expected.CommandHelpTests_d2a59b5932a503ab.verified.txt (1)
6-6: Consistenterrorfield addition forexample --helpsnapshottests/NetEvolve.ForgingBlazor.Tests.Integration/_snapshots/DotNet10_0/Help_Theory_Expected.CommandHelpTests_25a03f3b48369ce7.verified.txt (1)
6-6: DotNet10_0create --helpsnapshot updated to include stderr (empty)tests/NetEvolve.ForgingBlazor.Tests.Integration/Predefined.cs (1)
21-25: Global sorting settings: confirm this is intended for all snapshots.
SortJsonObjects()+SortPropertiesAlphabetically()can reduce churn, but they also change baseline snapshot semantics globally (including for any tests that relied on insertion order).tests/NetEvolve.ForgingBlazor.Tests.Integration/Commands/CommandBuildTests.cs (1)
11-21: Verify default-args branch is still exercising the intended output-path behavior.
In theelsebranch you no longer pass an output path arg value (while the other branch effectively does by appendingdirectory.Path). This may be totally fine ifHelper.VerifyStaticContent(directory.Path, args)controls output path out-of-band—just worth confirming to avoid a silent behavior change in coverage.src/NetEvolve.ForgingBlazor/Commands/CommandBuild.cs (1)
32-45: Option registration looks correct (--content-pathshows up in help and parse results).
...on/_snapshots/DotNet10_0/Help_Theory_Expected.CommandHelpTests_3cb5690aa64d0539.verified.txt
Show resolved
Hide resolved
| build, | ||
| --help | ||
| ], | ||
| error: , |
There was a problem hiding this comment.
Same note as DotNet10 snapshot: error: , suggests the verified value isn’t a clean string.
Consider snapshotting the error text (error.ToString()) to avoid formatter artifacts.
...ion/_snapshots/DotNet9_0/Help_Theory_Expected.CommandHelpTests_a46afb280312df84.verified.txt
Show resolved
Hide resolved
...ion/_snapshots/DotNet9_0/Help_Theory_Expected.CommandHelpTests_a46afb280312df84.verified.txt
Show resolved
Hide resolved
| if (args is not null && args.Length != 0) | ||
| { | ||
| args = [.. args, directory.Path]; | ||
| args = [.. args, directory.Path, "--content-path", "_setup/content"]; | ||
| } | ||
| else | ||
| { | ||
| args = ["build"]; | ||
| args = ["build", "--content-path", "_setup/Content"]; | ||
| } | ||
|
|
||
| await Helper.VerifyStaticContent(directory.Path, args).ConfigureAwait(false); |
There was a problem hiding this comment.
Fix _setup/content vs _setup/Content casing (will fail on case-sensitive filesystems).
Line 13 uses _setup/content, Line 17 uses _setup/Content. Pick one canonical casing that matches the repo folder.
- args = ["build", "--content-path", "_setup/Content"];
+ args = ["build", "--content-path", "_setup/content"];🤖 Prompt for AI Agents
In tests/NetEvolve.ForgingBlazor.Tests.Integration/Commands/CommandBuildTests.cs
around lines 11–20 there is inconsistent casing for the content path
(_setup/content vs _setup/Content) which will break on case-sensitive
filesystems; pick the canonical folder casing used in the repository (e.g.,
"_setup/Content") and update both occurrences to that exact casing so both
branches pass the same path to VerifyStaticContent.
Summary by CodeRabbit
New Features
--content-pathcommand option to specify custom content directory paths.Tests
✏️ Tip: You can customize this high-level summary in your review settings.