-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Disable System.Drawing.Common tests for mobile workloads and skip a few other tests #57114
Conversation
Since the library is PNSE by default, make sure the tests do not run on mobile.
Tagging subscribers to this area: @safern, @tarekgh Issue DetailsSince the library is PNSE by default, make sure the tests do not run on mobile.
|
Should we have some shorthand property for |
…running for mobile tests
@@ -54,7 +54,7 @@ | |||
AssemblyFile="$(RuntimeConfigParserTasksAssemblyPath)" | |||
Condition="'$(RuntimeConfigParserTasksAssemblyPath)' != ''" /> | |||
|
|||
<Target Name="GenerateRuntimeConfig" Condition="'$(OutputType)' != 'library' and '$(TargetOS)' != 'Browser'"> |
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.
Out of curiosity, why do we no longer need this, or why was it introduced in the first place?
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.
I think it was introduced by mistake from #56486
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.
why are we skipping it on browser
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.
Good question. @fanyang-mono ?
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.
Wasm has its own target of generating runtime config which is part of the source code. (https://github.com/dotnet/runtime/blob/main/src/mono/wasm/build/WasmApp.targets#L146) This target is for mobile testing only. But for wasm testing, the targets defined in this file will be visible to wasm tests as well. To make things clear, I added '$(TargetOS)' != 'Browser'
.
browser failed with
which is something I have not seen before |
monointerpreter failed with
Are we ever going to fix this? #11063 |
It would be nice. |
src/libraries/System.Drawing.Common/tests/System.Drawing.Common.Tests.csproj
Show resolved
Hide resolved
src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/ConsoleLifetimeExitTests.cs
Outdated
Show resolved
Hide resolved
src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/ConsoleLifetimeExitTests.cs
Outdated
Show resolved
Hide resolved
This will resolve #57184. We should close that issue when this is merged. Thanks for picking it up @steveisok. |
Co-authored-by: Eric Erhardt <eric.erhardt@microsoft.com>
Since the library is PNSE by default, make sure the tests do not run on mobile.
Also skips a few other tests.
Closes #57184