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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Aug 13, 2021
Merged

Disable System.Drawing.Common tests for mobile workloads and skip a few other tests #57114

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
faa0a45
Disable System.Drawing.Common tests for mobile workloads
Aug 10, 2021
f3cfb1d
Shorter condition
Aug 10, 2021
33d157a
Remove condition that prevents the GenerateRuntimeConfig target from …
Aug 10, 2021
409a979
Properly skip some tests that were failing on CI
Aug 10, 2021
a792aac
Go with santi's suggestion.
Aug 10, 2021
ce7e54b
Skip platform specific test
Aug 11, 2021
09fea57
Apply suggestions from code review
akoeplinger Aug 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion eng/testing/tests.mobile.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@
AssemblyFile="$(RuntimeConfigParserTasksAssemblyPath)"
Condition="'$(RuntimeConfigParserTasksAssemblyPath)' != ''" />

<Target Name="GenerateRuntimeConfig" Condition="'$(OutputType)' != 'library' and '$(TargetOS)' != 'Browser'">
Copy link
Member

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?

Copy link
Member Author

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

Copy link
Member

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Good question. @fanyang-mono ?

Copy link
Member

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'.

<Target Name="GenerateRuntimeConfig" Condition="'$(TargetOS)' != 'Browser'">
<PropertyGroup>
<RuntimeConfigFilePath>$(PublishDir)$(AssemblyName).runtimeconfig.json</RuntimeConfigFilePath>
<ParsedRuntimeConfigFilePath>$(PublishDir)runtimeconfig.bin</ParsedRuntimeConfigFilePath>
Expand Down
2 changes: 1 addition & 1 deletion src/libraries/System.Drawing.Common/tests/System.Drawing.Common.Tests.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
<AllowUnsafeBlocks>true</AllowUnsafeBlocks>
<IncludeRemoteExecutor>true</IncludeRemoteExecutor>
<TargetFrameworks>$(NetCoreAppCurrent)-windows;$(NetCoreAppCurrent)-Unix;net48</TargetFrameworks>
<IgnoreForCI Condition="'$(TargetOS)' == 'Browser'">true</IgnoreForCI>
<IgnoreForCI Condition="'$(TargetsMobile)' == 'true'">true</IgnoreForCI>
steveisok marked this conversation as resolved.
Show resolved Hide resolved
</PropertyGroup>
<ItemGroup>
<Compile Include="AssemblyInfo.cs" />
Expand Down