Change GUI apphost tests to not rely on window searches (dotnet/core-setup#6823)

* Disable failures when no window popup is detected. I kept the window popup detection code active as a "Fast" way to detect the process is done (it would timeout otherwise)
* Add new trace to note that we are trying to popup a window
* Add some test helpers to remove code duplication and better logging
* Validate product behavior based on traces

Commit migrated from https://github.com/dotnet/core-setup/commit/74e62c50dbf559ba0dc8e2b1d5893309c2f058a4

diff --git a/src/installer/corehost/corehost.cpp b/src/installer/corehost/corehost.cpp
index 56a5950..1584266 100644 (file)
--- a/src/installer/corehost/corehost.cpp
+++ b/src/installer/corehost/corehost.cpp
@@ -317,8 +317,7 @@ int main(const int argc, const pal::char_t* argv[])
 
     int exit_code = exe_start(argc, argv);
 
-    // Flush traces before exit - just to be sure, and also if we're showing a popup below the error should show up in traces
-    // by the time the popup is displayed.
+    // Flush traces before exit - just to be sure
     trace::flush();
 
 #if defined(_WIN32) && defined(FEATURE_APPHOST)
@@ -332,6 +331,11 @@ int main(const int argc, const pal::char_t* argv[])
             executable_name = get_filename(executable_name);
         }
 
+        trace::verbose(_X("Creating a GUI message box with title: '%s' and message: '%s;."), executable_name.c_str(), g_buffered_errors.c_str());
+
+        // Flush here so that when the dialog is up, the traces are up to date (specifically when they are redirected to a file).
+        trace::flush();
+
         ::MessageBoxW(NULL, g_buffered_errors.c_str(), executable_name.c_str(), MB_OK);
     }
 #endif

diff --git a/src/installer/test/HostActivationTests/CommandExtensions.cs b/src/installer/test/HostActivationTests/CommandExtensions.cs
index 8121825..267d185 100644 (file)
--- a/src/installer/test/HostActivationTests/CommandExtensions.cs
+++ b/src/installer/test/HostActivationTests/CommandExtensions.cs
@@ -3,6 +3,8 @@
 // See the LICENSE file in the project root for more information.
 
 using Microsoft.DotNet.Cli.Build.Framework;
+using System;
+using System.IO;
 
 namespace Microsoft.DotNet.CoreSetup.Test.HostActivation
 {
@@ -13,6 +15,19 @@ namespace Microsoft.DotNet.CoreSetup.Test.HostActivation
             return command.EnvironmentVariable(Constants.HostTracing.TraceLevelEnvironmentVariable, "1");
         }
 
+        public static Command EnableHostTracingToFile(this Command command, out string filePath)
+        {
+            filePath = Path.Combine(TestArtifact.TestArtifactsPath, "trace" + Guid.NewGuid().ToString() + ".log");
+            if (File.Exists(filePath))
+            {
+                File.Delete(filePath);
+            }
+
+            return command
+                .EnableHostTracing()
+                .EnvironmentVariable(Constants.HostTracing.TraceFileEnvironmentVariable, filePath);
+        }
+
         public static Command EnableTracingAndCaptureOutputs(this Command command)
         {
             return command

diff --git a/src/installer/test/HostActivationTests/StandaloneAppActivation.cs b/src/installer/test/HostActivationTests/StandaloneAppActivation.cs
index 1dd4e88..763dc7f 100644 (file)
--- a/src/installer/test/HostActivationTests/StandaloneAppActivation.cs
+++ b/src/installer/test/HostActivationTests/StandaloneAppActivation.cs
@@ -259,8 +259,7 @@ namespace Microsoft.DotNet.CoreSetup.Test.HostActivation
                 .CaptureStdOut()
                 .Start();
 
-            IntPtr windowHandle = WaitForPopupFromProcess(command.Process);
-            Assert.NotEqual(IntPtr.Zero, windowHandle);
+            WaitForPopupFromProcess(command.Process);
 
             // In theory we should close the window - but it's just easier to kill the process.
             // The popup should be the last thing the process does anyway.
@@ -293,15 +292,12 @@ namespace Microsoft.DotNet.CoreSetup.Test.HostActivation
             UseBuiltAppHost(appExe);
             MarkAppHostAsGUI(appExe);
 
-            string traceFilePath = Path.Combine(Path.GetDirectoryName(appExe), "trace.log");
-
+            string traceFilePath;
             Command command = Command.Create(appExe)
-                .EnvironmentVariable("COREHOST_TRACE", "1")
-                .EnvironmentVariable("COREHOST_TRACEFILE", traceFilePath)
+                .EnableHostTracingToFile(out traceFilePath)
                 .Start();
 
-            IntPtr windowHandle = WaitForPopupFromProcess(command.Process);
-            Assert.NotEqual(IntPtr.Zero, windowHandle);
+            WaitForPopupFromProcess(command.Process);
 
             // In theory we should close the window - but it's just easier to kill the process.
             // The popup should be the last thing the process does anyway.
@@ -311,7 +307,10 @@ namespace Microsoft.DotNet.CoreSetup.Test.HostActivation
 
             result.Should().Fail()
                 .And.FileExists(traceFilePath)
+                .And.FileContains(traceFilePath, "Creating a GUI message box with")
                 .And.FileContains(traceFilePath, "This executable is not bound to a managed DLL to execute.");
+
+            FileUtils.DeleteFileIfPossible(traceFilePath);
         }
 
         [Fact]
@@ -332,8 +331,9 @@ namespace Microsoft.DotNet.CoreSetup.Test.HostActivation
             UseBuiltAppHost(appExe);
             MarkAppHostAsGUI(appExe);
 
+            string traceFilePath;
             Command command = Command.Create(appExe)
-                .EnvironmentVariable("COREHOST_TRACE", "1")
+                .EnableHostTracingToFile(out traceFilePath)
                 .CaptureStdOut()
                 .CaptureStdErr()
                 .EnvironmentVariable(Constants.DisableGuiErrors.EnvironmentVariable, "1")
@@ -354,6 +354,15 @@ namespace Microsoft.DotNet.CoreSetup.Test.HostActivation
 
                 Assert.True(false, "The process failed to exit in the alloted time, it's possible it has a dialog up which should not be there.");
             }
+
+            commandResult
+                .Should().Fail()
+                .And.FileExists(traceFilePath)
+                .And.FileContains(traceFilePath, "Gui errors disabled, keeping errors in stderr.")
+                .And.NotFileContains(traceFilePath, "Creating a GUI message box with")
+                .And.FileContains(traceFilePath, "This executable is not bound to a managed DLL to execute.");
+
+            FileUtils.DeleteFileIfPossible(traceFilePath);
         }
 
 #if WINDOWS
@@ -392,15 +401,9 @@ namespace Microsoft.DotNet.CoreSetup.Test.HostActivation
                 timeRemaining -= 100;
             }
 
-            Assert.True(
-                windowHandle != IntPtr.Zero,
-                $"Waited {longTimeout} milliseconds for the popup window on process {process.Id}, but none was found." +
-                $"{Environment.NewLine}{diagMessages.ToString()}");
-
-            Assert.True(
-                timeRemaining > (longTimeout - timeout),
-                $"Waited {longTimeout - timeRemaining} milliseconds for the popup window on process {process.Id}. " +
-                $"It did show and was detected as HWND {windowHandle}, but it took too long. Consider extending the timeout period for this test.");
+            // Do not fail if the window could be detected, sometimes the check is fragile and doesn't work.
+            // Not worth the trouble trying to figure out why (only happens rarely in the CI system).
+            // We will rely on product tracing in the failure case.
 
             return windowHandle;
         }

diff --git a/src/installer/test/TestUtils/Assertions/CommandResultAssertions.cs b/src/installer/test/TestUtils/Assertions/CommandResultAssertions.cs
index d746e1f..d84b028 100644 (file)
--- a/src/installer/test/TestUtils/Assertions/CommandResultAssertions.cs
+++ b/src/installer/test/TestUtils/Assertions/CommandResultAssertions.cs
@@ -126,15 +126,17 @@ namespace Microsoft.DotNet.CoreSetup.Test
 
         public AndConstraint<CommandResultAssertions> FileContains(string path, string pattern)
         {
-            Execute.Assertion.ForCondition(System.IO.File.ReadAllText(path).Contains(pattern))
-                .FailWith("The command did not write the expected result '{0}' to the file: {1}{2}", pattern, path, GetDiagnosticsInfo());
+            string fileContent = System.IO.File.ReadAllText(path);
+            Execute.Assertion.ForCondition(fileContent.Contains(pattern))
+                .FailWith("The command did not write the expected result '{0}' to the file: {1}{2}\nfile content: {3}", pattern, path, GetDiagnosticsInfo(), fileContent);
             return new AndConstraint<CommandResultAssertions>(this);
         }
 
         public AndConstraint<CommandResultAssertions> NotFileContains(string path, string pattern)
         {
-            Execute.Assertion.ForCondition(!System.IO.File.ReadAllText(path).Contains(pattern))
-                .FailWith("The command did not write the expected result '{1}' to the file: {1}{2}", pattern, path, GetDiagnosticsInfo());
+            string fileContent = System.IO.File.ReadAllText(path);
+            Execute.Assertion.ForCondition(!fileContent.Contains(pattern))
+                .FailWith("The command did not write the expected result '{1}' to the file: {1}{2}\nfile content: {3}", pattern, path, GetDiagnosticsInfo(), fileContent);
             return new AndConstraint<CommandResultAssertions>(this);
         }
 

diff --git a/src/installer/test/TestUtils/FileUtils.cs b/src/installer/test/TestUtils/FileUtils.cs
index ee8c9b3..c7a0479 100644 (file)
--- a/src/installer/test/TestUtils/FileUtils.cs
+++ b/src/installer/test/TestUtils/FileUtils.cs
@@ -146,5 +146,16 @@ namespace Microsoft.DotNet.CoreSetup.Test.HostActivation
             EnsureFileDirectoryExists(filePath);
             File.WriteAllText(filePath, string.Empty);
         }
+
+        public static void DeleteFileIfPossible(string filePath)
+        {
+            try
+            {
+                File.Delete(filePath);
+            }
+            catch (System.IO.IOException)
+            {
+            }
+        }
     }
 }