From: Dan Moseley Date: Thu, 12 Nov 2020 01:58:23 +0000 (-0800) Subject: Fix StartInfo test (#44392) X-Git-Tag: submit/tizen/20210909.063632~4663 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=a207c7f5df922560aac9028a2d7416116d38fce6;p=platform%2Fupstream%2Fdotnet%2Fruntime.git Fix StartInfo test (#44392) * Stabilize StartInfo_NotepadWithContent_withArgumentList * Add state in failed SetApartmentState exception * Same for other tests * Extract common code * Modify * Extract common code * Pass down throwOnError * Apply suggestions from code review Co-authored-by: Jan Kotas * feedback * Add process for disposal * Make slow test outer loop Co-authored-by: Jan Kotas --- diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Threading/Thread.CoreCLR.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Threading/Thread.CoreCLR.cs index dc8a82c..e096e96 100644 --- a/src/coreclr/src/System.Private.CoreLib/src/System/Threading/Thread.CoreCLR.cs +++ b/src/coreclr/src/System.Private.CoreLib/src/System/Threading/Thread.CoreCLR.cs @@ -418,7 +418,7 @@ namespace System.Threading /// single-threaded or multi-threaded apartment. /// #if FEATURE_COMINTEROP_APARTMENT_SUPPORT - private bool TrySetApartmentStateUnchecked(ApartmentState state) + private bool SetApartmentStateUnchecked(ApartmentState state, bool throwOnError) { ApartmentState retState = (ApartmentState)SetApartmentStateNative((int)state); @@ -433,6 +433,12 @@ namespace System.Threading if (retState != state) { + if (throwOnError) + { + string msg = SR.Format(SR.Thread_ApartmentState_ChangeFailed, retState); + throw new InvalidOperationException(msg); + } + return false; } @@ -445,9 +451,19 @@ namespace System.Threading [MethodImpl(MethodImplOptions.InternalCall)] internal extern int SetApartmentStateNative(int state); #else // FEATURE_COMINTEROP_APARTMENT_SUPPORT - private static bool TrySetApartmentStateUnchecked(ApartmentState state) + private static bool SetApartmentStateUnchecked(ApartmentState state, bool throwOnError) { - return state == ApartmentState.Unknown; + if (state != ApartmentState.Unknown) + { + if (throwOnError) + { + throw new PlatformNotSupportedException(SR.PlatformNotSupported_ComInterop); + } + + return false; + } + + return true; } #endif // FEATURE_COMINTEROP_APARTMENT_SUPPORT diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs index 3d946ad..e1c3c55 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs @@ -947,11 +947,7 @@ namespace System.Diagnostics.Tests try { - process.WaitForInputIdle(); // Give the file a chance to load - Assert.Equal("notepad", process.ProcessName); - - // On some Windows versions, the file extension is not included in the title - Assert.StartsWith(Path.GetFileNameWithoutExtension(tempFile), process.MainWindowTitle); + VerifyNotepadMainWindowTitle(process, tempFile); } finally { @@ -985,18 +981,7 @@ namespace System.Diagnostics.Tests try { - process.WaitForInputIdle(); // Give the file a chance to load - Assert.Equal("notepad", process.ProcessName); - - if (PlatformDetection.IsInAppContainer) - { - Assert.Throws(() => process.MainWindowTitle); - } - else - { - // On some Windows versions, the file extension is not included in the title - Assert.StartsWith(Path.GetFileNameWithoutExtension(tempFile), process.MainWindowTitle); - } + VerifyNotepadMainWindowTitle(process, tempFile); } finally { @@ -1172,11 +1157,7 @@ namespace System.Diagnostics.Tests try { - process.WaitForInputIdle(); // Give the file a chance to load - Assert.Equal("notepad", process.ProcessName); - - // On some Windows versions, the file extension is not included in the title - Assert.StartsWith(Path.GetFileNameWithoutExtension(tempFile), process.MainWindowTitle); + VerifyNotepadMainWindowTitle(process, tempFile); } finally { @@ -1184,5 +1165,30 @@ namespace System.Diagnostics.Tests } } } + + private void VerifyNotepadMainWindowTitle(Process process, string filename) + { + // On some Windows versions, the file extension is not included in the title + string expected = Path.GetFileNameWithoutExtension(filename); + + process.WaitForInputIdle(); // Give the file a chance to load + Assert.Equal("notepad", process.ProcessName); + + // Notepad calls CreateWindowEx with pWindowName of empty string, then calls SetWindowTextW + // with "Untitled - Notepad" then finally if you're opening a file, calls SetWindowTextW + // with something similar to "myfilename - Notepad". So there's a race between input idle + // and the expected MainWindowTitle because of how Notepad is implemented. + string title = process.MainWindowTitle; + int count = 0; + while (!title.StartsWith(expected) && count < 500) + { + Thread.Sleep(10); + process.Refresh(); + title = process.MainWindowTitle; + count++; + } + + Assert.StartsWith(expected, title); + } } } diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessTestBase.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessTestBase.cs index 6b1d00e..efd8422 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessTestBase.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessTestBase.cs @@ -19,8 +19,12 @@ namespace System.Diagnostics.Tests protected Process CreateDefaultProcess() { + if (_process != null) + throw new InvalidOperationException(); + _process = CreateProcessLong(); _process.Start(); + AddProcessForDispose(_process); return _process; } diff --git a/src/libraries/System.Diagnostics.Process/tests/ProcessWaitingTests.cs b/src/libraries/System.Diagnostics.Process/tests/ProcessWaitingTests.cs index bb0edda..7348b70 100644 --- a/src/libraries/System.Diagnostics.Process/tests/ProcessWaitingTests.cs +++ b/src/libraries/System.Diagnostics.Process/tests/ProcessWaitingTests.cs @@ -637,6 +637,7 @@ namespace System.Diagnostics.Tests } [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] + [OuterLoop("As written, takes 30 seconds")] public async Task WaitAsyncForProcess() { Process p = CreateSleepProcess(WaitInMS); diff --git a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx index cd4d6a2..575a5fd 100644 --- a/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx +++ b/src/libraries/System.Private.CoreLib/src/Resources/Strings.resx @@ -3440,7 +3440,7 @@ An attempt was made to transition a task to a final state when it had already completed. - Failed to set the specified COM apartment state. + Failed to set the specified COM apartment state. Current apartment state '{0}'. Use CompressedStack.(Capture/Run) instead. diff --git a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems index 1597214..5ceff52 100644 --- a/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems +++ b/src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems @@ -1623,7 +1623,6 @@ - @@ -1829,7 +1828,6 @@ - diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/Thread.Unix.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/Thread.Unix.cs deleted file mode 100644 index c68be85..0000000 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/Thread.Unix.cs +++ /dev/null @@ -1,10 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -namespace System.Threading -{ - public sealed partial class Thread - { - private static Exception GetApartmentStateChangeFailedException() => new PlatformNotSupportedException(SR.PlatformNotSupported_ComInterop); - } -} diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/Thread.Windows.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/Thread.Windows.cs deleted file mode 100644 index 6bc647f..0000000 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/Thread.Windows.cs +++ /dev/null @@ -1,11 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -namespace System.Threading -{ - public sealed partial class Thread - { - private static Exception GetApartmentStateChangeFailedException() => - new InvalidOperationException(SR.Thread_ApartmentState_ChangeFailed); - } -} diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/Thread.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/Thread.cs index 0bfcc94..e0b4978 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/Thread.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/Thread.cs @@ -295,14 +295,16 @@ namespace System.Threading [SupportedOSPlatform("windows")] public void SetApartmentState(ApartmentState state) { - if (!TrySetApartmentState(state)) - { - throw GetApartmentStateChangeFailedException(); - } + SetApartmentState(state, throwOnError:true); } public bool TrySetApartmentState(ApartmentState state) { + return SetApartmentState(state, throwOnError:false); + } + + private bool SetApartmentState(ApartmentState state, bool throwOnError) + { switch (state) { case ApartmentState.STA: @@ -314,7 +316,7 @@ namespace System.Threading throw new ArgumentOutOfRangeException(nameof(state), SR.ArgumentOutOfRange_Enum); } - return TrySetApartmentStateUnchecked(state); + return SetApartmentStateUnchecked(state, throwOnError); } [Obsolete("Thread.GetCompressedStack is no longer supported. Please use the System.Threading.CompressedStack class")] diff --git a/src/libraries/System.Threading.Thread/tests/ThreadTests.cs b/src/libraries/System.Threading.Thread/tests/ThreadTests.cs index 17c08a5..877c2cb 100644 --- a/src/libraries/System.Threading.Thread/tests/ThreadTests.cs +++ b/src/libraries/System.Threading.Thread/tests/ThreadTests.cs @@ -190,7 +190,7 @@ namespace System.Threading.Threads.Tests RemoteExecutor.Invoke(() => { Assert.Equal(ApartmentState.MTA, Thread.CurrentThread.GetApartmentState()); - Assert.Throws(() => Thread.CurrentThread.SetApartmentState(ApartmentState.STA)); + AssertExtensions.ThrowsContains(() => Thread.CurrentThread.SetApartmentState(ApartmentState.STA), "MTA"); Thread.CurrentThread.SetApartmentState(ApartmentState.MTA); }).Dispose(); } diff --git a/src/mono/netcore/System.Private.CoreLib/src/System/Threading/Thread.Mono.cs b/src/mono/netcore/System.Private.CoreLib/src/System/Threading/Thread.Mono.cs index 0cc8310..2ca14d0 100644 --- a/src/mono/netcore/System.Private.CoreLib/src/System/Threading/Thread.Mono.cs +++ b/src/mono/netcore/System.Private.CoreLib/src/System/Threading/Thread.Mono.cs @@ -309,7 +309,20 @@ namespace System.Threading return YieldInternal(); } - private static bool TrySetApartmentStateUnchecked(ApartmentState state) => state == ApartmentState.Unknown; + private static bool SetApartmentStateUnchecked(ApartmentState state, bool throwOnError) + { + if (state != ApartmentState.Unknown) + { + if (throwOnError) + { + throw new PlatformNotSupportedException(SR.PlatformNotSupported_ComInterop); + } + + return false; + } + + return true; + } private ThreadState ValidateThreadState() {