Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove extra space #5238

Merged
merged 4 commits into from
Mar 17, 2025
Merged

Remove extra space #5238

merged 4 commits into from
Mar 17, 2025

Conversation

nohwnd
Copy link
Member

@nohwnd nohwnd commented Mar 14, 2025

image

Looks like the code is there since forever, and would always add a single extra space before the first "at".

Fix #5152

@microsoft-github-policy-service microsoft-github-policy-service bot added the Area: MTP Belongs to the Microsoft.Testing.Platform core library label Mar 14, 2025
@nohwnd
Copy link
Member Author

nohwnd commented Mar 14, 2025

image

not sure why the single space ends up being much larger shift in the ansi code output.

this is before removing the append:

image

@nohwnd
Copy link
Member Author

nohwnd commented Mar 14, 2025

also fixes normal terminal (with --no-ansi)

@Youssef1313
Copy link
Member

Could you make a test with VSTest as well?

@nohwnd
Copy link
Member Author

nohwnd commented Mar 14, 2025

It does not fix vstest, but I can see the same problem there with nunit. (while of course nunit is fine in mtp)

@Youssef1313
Copy link
Member

It does not fix vstest

Not sure what do you mean? What is the behavior when using VSTest before and after the change. I was expecting VSTest to probably be having the correct spacing already, and gets broken with the change. That's the guess I have.

@nohwnd
Copy link
Member Author

nohwnd commented Mar 14, 2025

vstest output has incorrect spacing before and after the change, both for mstest and nunit.

@Youssef1313
Copy link
Member

Nice. That was unexpected to me 😄

@nohwnd
Copy link
Member Author

nohwnd commented Mar 14, 2025

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@nohwnd nohwnd marked this pull request as ready for review March 17, 2025 08:31
@nohwnd nohwnd merged commit 33eefd4 into main Mar 17, 2025
7 of 9 checks passed
@nohwnd nohwnd deleted the stacktrace-is-shifted branch March 17, 2025 08:32
@nohwnd
Copy link
Member Author

nohwnd commented Mar 17, 2025

/backport to rel/3.8

Copy link
Contributor

Started backporting to rel/3.8: https://github.com/microsoft/testfx/actions/runs/13894905574

Copy link
Contributor

@nohwnd backporting to rel/3.8 failed, the patch most likely resulted in conflicts:

$ git am --3way --empty=keep --ignore-whitespace --keep-non-patch changes.patch

Applying: Remove extra space
Applying: Remove extra space
Applying: Fix tests
Using index info to reconstruct a base tree...
M	test/UnitTests/MSTestAdapter.UnitTests/Execution/TestAssemblyInfoTests.cs
M	test/UnitTests/MSTestAdapter.UnitTests/Execution/TestClassInfoTests.cs
M	test/UnitTests/MSTestAdapter.UnitTests/Execution/TestMethodInfoTests.cs
M	test/UnitTests/MSTestAdapter.UnitTests/Extensions/ExceptionExtensionsTests.cs
Falling back to patching base and 3-way merge...
Auto-merging test/UnitTests/MSTestAdapter.UnitTests/Extensions/ExceptionExtensionsTests.cs
CONFLICT (content): Merge conflict in test/UnitTests/MSTestAdapter.UnitTests/Extensions/ExceptionExtensionsTests.cs
Auto-merging test/UnitTests/MSTestAdapter.UnitTests/Execution/TestMethodInfoTests.cs
CONFLICT (content): Merge conflict in test/UnitTests/MSTestAdapter.UnitTests/Execution/TestMethodInfoTests.cs
Auto-merging test/UnitTests/MSTestAdapter.UnitTests/Execution/TestClassInfoTests.cs
CONFLICT (content): Merge conflict in test/UnitTests/MSTestAdapter.UnitTests/Execution/TestClassInfoTests.cs
Auto-merging test/UnitTests/MSTestAdapter.UnitTests/Execution/TestAssemblyInfoTests.cs
CONFLICT (content): Merge conflict in test/UnitTests/MSTestAdapter.UnitTests/Execution/TestAssemblyInfoTests.cs
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
hint: When you have resolved this problem, run "git am --continue".
hint: If you prefer to skip this patch, run "git am --skip" instead.
hint: To restore the original branch and stop patching, run "git am --abort".
hint: Disable this message with "git config set advice.mergeConflict false"
Patch failed at 0003 Fix tests
Error: The process '/usr/bin/git' failed with exit code 128

NOTE: A PR will be created, but needs to be revised manually!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: MTP Belongs to the Microsoft.Testing.Platform core library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Exception stack trace has first line incorrectly offset
3 participants