Skip to content

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

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

Merged
steveisok 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 all 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'">

@mdh1418 mdh1418 Aug 10, 2021

Copy link
Copy Markdown
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?

@steveisok steveisok Aug 10, 2021

Copy link
Copy Markdown
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

@lewing lewing Aug 11, 2021

Copy link
Copy Markdown
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

@steveisok steveisok Aug 11, 2021

Copy link
Copy Markdown
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 ?

@fanyang-mono fanyang-mono Aug 12, 2021

Copy link
Copy Markdown
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
6 changes: 5 additions & 1 deletion src/libraries/Common/tests/TestUtilities/System/PlatformDetection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,11 @@ public static bool IsDrawingSupported
#if NETCOREAPP
if (!IsWindows)
{
if (IsOSX)
if (IsMobile)
{
return false;
}
else if (IsOSX)
{
return NativeLibrary.TryLoad("libgdiplus.dylib", out _);
}
Expand Down
4 changes: 3 additions & 1 deletion src/libraries/Microsoft.Extensions.Hosting/tests/UnitTests/ConsoleLifetimeExitTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,8 @@ protected override async Task ExecuteAsync(CancellationToken stoppingToken)
/// </summary>
[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
// SIGTERM is only handled on net6.0+, so the workaround to "clobber" the exit code is still in place on NetFramework
[SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)]
[SkipOnTargetFramework(TargetFrameworkMonikers.NetFramework)]
[ActiveIssue("https://github.com/dotnet/runtime/issues/34582", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)]
public void EnsureEnvironmentExitCode()
{
using var remoteHandle = RemoteExecutor.Invoke(async () =>
Expand Down Expand Up @@ -126,6 +127,7 @@ await Task.Run(() =>
/// Tests that calling Environment.Exit from the "main" thread doesn't hang the process forever.
/// </summary>
[ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))]
[ActiveIssue("https://github.com/dotnet/runtime/issues/34582", TestPlatforms.Windows, TargetFrameworkMonikers.Netcoreapp, TestRuntimes.Mono)]
public void EnsureEnvironmentExitDoesntHang()
{
using var remoteHandle = RemoteExecutor.Invoke(async () =>
Expand Down
13 changes: 7 additions & 6 deletions src/libraries/System.Diagnostics.Process/tests/ProcessTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,29 +171,28 @@ public void TestExited_SynchronizingObject(bool invokeRequired)
}

[Fact]
[SkipOnPlatform(TestPlatforms.iOS | TestPlatforms.tvOS, "Not supported on iOS, or tvOS.")]
[SkipOnPlatform(TestPlatforms.iOS | TestPlatforms.tvOS | TestPlatforms.MacCatalyst, "Not supported on iOS, tvOS, or MacCatalyst.")]
public void ProcessStart_TryExitCommandAsFileName_ThrowsWin32Exception()
{
Assert.Throws<Win32Exception>(() => Process.Start(new ProcessStartInfo { UseShellExecute = false, FileName = "exit", Arguments = "42" }));
}

[Fact]
[SkipOnPlatform(TestPlatforms.iOS | TestPlatforms.tvOS, "Not supported on iOS, or tvOS.")]
[SkipOnPlatform(TestPlatforms.iOS | TestPlatforms.tvOS | TestPlatforms.MacCatalyst, "Not supported on iOS, tvOS, or MacCatalyst.")]
public void ProcessStart_UseShellExecuteFalse_FilenameIsUrl_ThrowsWin32Exception()
{
Assert.Throws<Win32Exception>(() => Process.Start(new ProcessStartInfo { UseShellExecute = false, FileName = "https://www.github.com/corefx" }));
}

[Fact]
[SkipOnPlatform(TestPlatforms.iOS | TestPlatforms.tvOS, "Not supported on iOS, or tvOS.")]
[SkipOnPlatform(TestPlatforms.iOS | TestPlatforms.tvOS | TestPlatforms.MacCatalyst, "Not supported on iOS, tvOS, or MacCatalyst.")]
public void ProcessStart_TryOpenFolder_UseShellExecuteIsFalse_ThrowsWin32Exception()
{
Assert.Throws<Win32Exception>(() => Process.Start(new ProcessStartInfo { UseShellExecute = false, FileName = Path.GetTempPath() }));
}

[Fact]
[SkipOnPlatform(TestPlatforms.OSX, "OSX doesn't support throwing on Process.Start")]
[ActiveIssue("https://github.com/dotnet/runtime/issues/52882", TestPlatforms.MacCatalyst)]
[SkipOnPlatform(TestPlatforms.iOS | TestPlatforms.tvOS | TestPlatforms.MacCatalyst, "Not supported on iOS, tvOS, or MacCatalyst.")]
public void TestStartWithBadWorkingDirectory()
{
string program;
Expand Down Expand Up @@ -231,6 +230,7 @@ public void TestStartWithBadWorkingDirectory()

[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.HasWindowsShell))]
[OuterLoop("Launches File Explorer")]
[SkipOnPlatform(TestPlatforms.iOS | TestPlatforms.tvOS | TestPlatforms.MacCatalyst, "Not supported on iOS, tvOS, or MacCatalyst.")]
public void ProcessStart_UseShellExecute_OnWindows_OpenMissingFile_Throws()
{
string fileToOpen = Path.Combine(Environment.CurrentDirectory, "_no_such_file.TXT");
Expand All @@ -243,6 +243,7 @@ public void ProcessStart_UseShellExecute_OnWindows_OpenMissingFile_Throws()
[InlineData(true)]
[InlineData(false)]
[OuterLoop("Launches File Explorer")]
[SkipOnPlatform(TestPlatforms.iOS | TestPlatforms.tvOS | TestPlatforms.MacCatalyst, "Not supported on iOS, tvOS, or MacCatalyst.")]
public void ProcessStart_UseShellExecute_OnWindows_DoesNotThrow(bool isFolder)
{
string fileToOpen;
Expand Down Expand Up @@ -500,7 +501,7 @@ public void HasExited_GetNotStarted_ThrowsInvalidOperationException()
}

[Fact]
[SkipOnPlatform(TestPlatforms.iOS | TestPlatforms.tvOS, "Not supported on iOS, or tvOS.")]
[SkipOnPlatform(TestPlatforms.iOS | TestPlatforms.tvOS | TestPlatforms.MacCatalyst, "Not supported on iOS, tvOS, or MacCatalyst.")]
public void Kill_NotStarted_ThrowsInvalidOperationException()
{
var process = new Process();
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>
Comment thread
steveisok marked this conversation as resolved.
</PropertyGroup>
<ItemGroup>
<Compile Include="AssemblyInfo.cs" />
Expand Down
8 changes: 6 additions & 2 deletions src/libraries/System.Runtime/tests/System/CharTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1089,8 +1089,12 @@ public static IEnumerable<object[]> UpperLowerCasing_TestData()

if (PlatformDetection.IsNotInvariantGlobalization)
{
yield return new object[] { '\u0131', 'I', "tr-TR" };
yield return new object[] { 'i', '\u0130', "tr-TR" };
// Android has issues w/ tr-TR, see https://github.com/dotnet/runtime/issues/37069
if (!PlatformDetection.IsAndroid)
{
yield return new object[] { '\u0131', 'I', "tr-TR" };
yield return new object[] { 'i', '\u0130', "tr-TR" };
}
yield return new object[] { '\u0660', '\u0660', "en-US" };
}
}
Expand Down