Skip to content

Fix answer file splitting#15306

Merged
nohwnd merged 1 commit intomicrosoft:mainfrom
nohwnd:fix-answer-file-parsing
Nov 10, 2025
Merged

Fix answer file splitting#15306
nohwnd merged 1 commit intomicrosoft:mainfrom
nohwnd:fix-answer-file-parsing

Conversation

@nohwnd
Copy link
Member

@nohwnd nohwnd commented Oct 31, 2025

Description

The code was last touched here #1196 fixing some indeterminate issues with the parsing. Seeing the examples in tests it handles quotes differently and backslashes.

However the code runs out of bounds (not a problem but throws index out of range unnecessarilly).

And fails to recognize \" that is in quoted string. It detects the \", but the index is not advanced past that ", so in the next steps is incorrectly stops being a quoted string and spaces will split the command into more tokens.

This is problematic for providing /Tests: which provides the whole test name including spaces etc, in format like this:

/Tests:"UpdateTest.CheckUpdate("C:\\Data\\Config.xml","C:\\Data\\Config","C:\\Data\\User"),UpdateTest.MyTest(\"Lorem Ipsum\")"
mytests.dll

The previous logic would split the parameters to be:

/Tests:UpdateTest.CheckUpdate("C:\\Data\\Config.xml","C:\\Data\\Config","C:\\Data\\User"),UpdateTest.MyTest(\"Lorem
Ipsum\")
mytests.dll

And would fail with Ipsum\") test source not found.

New logic correctly splits it to:

/Tests:UpdateTest.CheckUpdate("C:\\Data\\Config.xml","C:\\Data\\Config","C:\\Data\\User"),UpdateTest.MyTest(\"Lorem Ipsum\")
mytests.dll

I have a much larger file from the issue, but it contains customer data, and the spaces are the only edge case that triggers this problem.

Related issue

https://developercommunity.visualstudio.com/t/vstest-task-fails-with-Unhandled-Except/10527048

@nohwnd nohwnd requested a review from Copilot October 31, 2025 16:35
@nohwnd
Copy link
Member Author

nohwnd commented Oct 31, 2025

@copilot anything that the previous code handled and that I don't test for in the new code?

// If the char after '\' is a '"', output '"' and skip over to the next char.
if (index <= args.Length && args[index] == '"')
{
currentArg.Append('"');
Copy link
Member Author

@nohwnd nohwnd Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Main bug was here imho. We handled the quote, but we don't advance to the next character because the logic is trying to be too clever with the \ being or not being doubled.

The " is then processed in the next loop, and stops the string from being quoted, even though the quote was escaped.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@nohwnd
Copy link
Member Author

nohwnd commented Nov 3, 2025

@copilot anything that the previous code handled and that I don't test for in the new code?

@nohwnd nohwnd requested a review from Copilot November 5, 2025 14:15
@nohwnd nohwnd merged commit bff6edc into microsoft:main Nov 10, 2025
7 checks passed
intellitect-bot pushed a commit to IntelliTect/EssentialCSharp.Web that referenced this pull request Feb 24, 2026
Updated [Microsoft.NET.Test.Sdk](https://github.com/microsoft/vstest)
from 18.0.1 to 18.3.0.

<details>
<summary>Release notes</summary>

_Sourced from [Microsoft.NET.Test.Sdk's
releases](https://github.com/microsoft/vstest/releases)._

## 18.3.0

## What's Changed

* Fix answer file splitting by @​nohwnd in
microsoft/vstest#15306

## Internal fixes and updates

* Bump branding to 18.1 by @​nohwnd in
microsoft/vstest#15286
* Remove stale copy of S.ComponentModel.Composition from testplatform
packages by @​ViktorHofer in
microsoft/vstest#15287
* Update codeflow metadata to fix backflow by @​premun in
microsoft/vstest#15291
* [main] Update dependencies from devdiv/DevDiv/vs-code-coverage by
@​dotnet-maestro[bot] in microsoft/vstest#15283
* Update Microsoft.Build.Utilities.Core by @​Youssef1313 in
microsoft/vstest#15300
* Disable DynamicNative instrumentation by default by @​nohwnd in
microsoft/vstest#15299
* [main] Source code updates from dotnet/dotnet by @​dotnet-maestro[bot]
in microsoft/vstest#15293
* [main] Source code updates from dotnet/dotnet by @​dotnet-maestro[bot]
in microsoft/vstest#15302
* [main] Source code updates from dotnet/dotnet by @​dotnet-maestro[bot]
in microsoft/vstest#15314
* Delete sha1 custom implementation we are not using for a long time by
@​nohwnd in microsoft/vstest#15313
* [main] Source code updates from dotnet/dotnet by @​dotnet-maestro[bot]
in microsoft/vstest#15315
* Update branding to 18.3.0 by @​nohwnd in
microsoft/vstest#15321
* [main] Update dependencies from devdiv/DevDiv/vs-code-coverage by
@​dotnet-maestro[bot] in microsoft/vstest#15325
* [main] Update dependencies from dotnet/arcade by @​dotnet-maestro[bot]
in microsoft/vstest#15264
* Revert adding dotnet_host_path workaround by @​nohwnd in
microsoft/vstest#15328
* [main] Update dependencies from dotnet/arcade by @​dotnet-maestro[bot]
in microsoft/vstest#15338
* [main] Source code updates from dotnet/dotnet by @​dotnet-maestro[bot]
in microsoft/vstest#15322
* [main] Update dependencies from dotnet/arcade by @​dotnet-maestro[bot]
in microsoft/vstest#15343
* Change PreReleaseVersionLabel from 'preview' to 'release' by @​nohwnd
in microsoft/vstest#15352
* [rel/18.3] Update dependencies from devdiv/DevDiv/vs-code-coverage by
@​dotnet-maestro[bot] in microsoft/vstest#15354
* [rel/18.3] Update dependencies from dotnet/arcade by
@​dotnet-maestro[bot] in microsoft/vstest#15389
* [rel/18.3] Update dependencies from dotnet/arcade by
@​dotnet-maestro[bot] in microsoft/vstest#15400
* Update build tools to 17.11.48 to be source buildable by @​nohwnd in
microsoft/vstest#15310
* Disable publishing on RTM by @​nohwnd in
microsoft/vstest#15296
* Don't access nuget.org for package feeds by @​nohwnd in
microsoft/vstest#15316
* No nuget access fix tests by @​nohwnd in
microsoft/vstest#15317
* Disable Dependabot updates in dependabot.yml by @​mmitche in
microsoft/vstest#15324

## New Contributors
* @​premun made their first contribution in
microsoft/vstest#15291

Commits viewable in [compare
view](microsoft/vstest@v18.0.1...v18.3.0).
</details>

[![Dependabot compatibility
score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=Microsoft.NET.Test.Sdk&package-manager=nuget&previous-version=18.0.1&new-version=18.3.0)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores)

Dependabot will resolve any conflicts with this PR as long as you don't
alter it yourself. You can also trigger a rebase manually by commenting
`@dependabot rebase`.

[//]: # (dependabot-automerge-start)
[//]: # (dependabot-automerge-end)

---

<details>
<summary>Dependabot commands and options</summary>
<br />

You can trigger Dependabot actions by commenting on this PR:
- `@dependabot rebase` will rebase this PR
- `@dependabot recreate` will recreate this PR, overwriting any edits
that have been made to it
- `@dependabot show <dependency name> ignore conditions` will show all
of the ignore conditions of the specified dependency
- `@dependabot ignore this major version` will close this PR and stop
Dependabot creating any more for this major version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this minor version` will close this PR and stop
Dependabot creating any more for this minor version (unless you reopen
the PR or upgrade to it yourself)
- `@dependabot ignore this dependency` will close this PR and stop
Dependabot creating any more for this dependency (unless you reopen the
PR or upgrade to it yourself)


</details>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
This was referenced Feb 25, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants