Uh oh!
There was an error while loading. Please reload this page.
[Test Improver] Add unit tests for PasteArguments.AppendArgument - #7888
Conversation
Covers argument escaping edge cases: simple args, empty string, spaces, tabs, quotes, backslash-at-end (doubling), backslash-before-quote (2N+1 rule), multiple args with separator, and path-with-spaces. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds targeted unit test coverage for Microsoft.Testing.Platform.Helpers.PasteArguments.AppendArgument, a Windows command-line argument escaping utility used when building process command lines.
Changes:
- Introduces a new
PasteArgumentsTeststest class with scenarios covering quoting/escaping rules for whitespace, quotes, and backslashes. - Adds multi-argument composition tests to validate separator insertion and end-to-end command line output.
Show a summary per file
| File | Description |
|---|---|
| test/UnitTests/Microsoft.Testing.Platform.UnitTests/Helpers/PasteArgumentsTests.cs | Adds unit tests covering key escaping/quoting edge cases for PasteArguments.AppendArgument. |
Copilot's findings
- Files reviewed: 1/1 changed files
- Comments generated: 0
There was a problem hiding this comment.
Summary
Workflow: Test Expert Reviewer 🧪
Date: 2026-04-28
Repository: microsoft/testfx
Key Findings
- [Coverage – Medium]
char.IsWhiteSpacecovers'\n','\r','\v','\f'in addition to space and tab, but only space (' ') and tab ('\t') are tested. A regression replacing thechar.IsWhiteSpacecheck with an explicit set would go undetected. - [Coverage – Low] The separator-space logic is only tested with two simple unquoted arguments; second-call behaviour with empty-string or space-containing arguments is untested.
- [DataDriven – Minor] The 15 individual
[TestMethod]tests could be consolidated into a smaller set of parameterised tests using[DataRow], improving discoverability of coverage gaps.
What's Good
- Every test uses a fresh local
StringBuilder— no shared mutable state, no isolation risk. - Assertions use
Assert.AreEqualthroughout, giving clear actual-vs-expected failure output. - All significant backslash-escaping rules (end-of-arg doubling, 2N+1 rule before quote) are covered with concrete examples.
- No timing dependencies, no I/O, no environment sensitivity.
Recommendations
- Add
[DataRow]/[TestMethod]entries for'\n'and'\r'in the "wraps in quotes" tests. - Add a test verifying the separator appears before complex (empty or space-containing) second arguments.
- Consider consolidating similar tests with
[DataRow]to make coverage gaps more visible.
Generated by Test Expert Reviewer 🧪
🧪 Test quality reviewed by Test Expert Reviewer 🧪
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Amaury Levé (Evangelink)
commented
Apr 28, 2026
Copilot please use the use cases and structure of the tests in the given link. |
…net/runtime reference Agent-Logs-Url: https://github.com/microsoft/testfx/sessions/4f3cd68a-ca2f-4d93-8488-e84abbb91a1b Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
Done in 60c5a5d. Replaced the individual |
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Covers argument escaping edge cases: simple args, empty string, spaces, tabs, quotes, backslash-at-end (doubling), backslash-before-quote (2N+1 rule), multiple args with separator, and path-with-spaces.
Fixes#7879