-
Notifications
You must be signed in to change notification settings - Fork 266
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
Update InvokeTestingPlatformTask to prefer running Exe over dotnet exec dll
#5094
Conversation
UseAppHost="$(UseAppHost)" | ||
IsExecutable="$(_IsExecutable)" | ||
TargetDir="$(TargetDir)" | ||
AssemblyName="$(AssemblyName)" | ||
NativeExecutableExtension="$(_NativeExecutableExtension)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@baronfel Could you please confirm if it's okay to use _NativeExecutableExtension
and _IsExecutable
? We want to avoid using things that may potentially break in future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both are set by the .NET SDK in props files and so should be available. They are _-prefixed properties, which means we don't like to encourage their direct use, but I don't think we make this data available in any other way at this time. cc @dsplaisted for heads-up that more folks are taking data dependencies on SDK-private members.
src/Platform/Microsoft.Testing.Platform.MSBuild/Tasks/InvokeTestingPlatformTask.cs
Outdated
Show resolved
Hide resolved
src/Platform/Microsoft.Testing.Platform.MSBuild/Tasks/InvokeTestingPlatformTask.cs
Show resolved
Hide resolved
/azp run |
Azure Pipelines successfully started running 1 pipeline(s). |
/backport to rel/3.8 |
Started backporting to rel/3.8: https://github.com/microsoft/testfx/actions/runs/13527928833 |
@Youssef1313 backporting to rel/3.8 failed, the patch most likely resulted in conflicts: $ git am --continue
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"
Applying: Propagate DOTNET_ROOT
No changes - did you forget to use 'git add'?
If there is nothing left to stage, chances are that something else
already introduced the same changes; you might want to skip this patch.
Error: The process '/usr/bin/git' failed with exit code 128 NOTE: A PR will be created, but needs to be revised manually! |
/backport to rel/3.8 |
Started backporting to rel/3.8: https://github.com/microsoft/testfx/actions/runs/13530362018 |
Manually verified this works as expected. For microsoft/PowerToys#37001, tests run successfully when running the Exe directly but not when using
dotnet exec dll
. I wasn't able to set up projects in a similar way as PowerToys to repro the issue and have a test. But I think it makes sense to run directly, and potentially even faster. We probably have enough tests to know if this change is problematic for a main stream scenario.