From 7249d70e9ca52cb42aaefddd95747e775c5f6adf Mon Sep 17 00:00:00 2001
From: eric sciple <ericsciple@users.noreply.github.com>
Date: Wed, 7 Aug 2024 12:08:17 -0700
Subject: [PATCH] feedback

---
 src/Runner.Worker/OSWarningChecker.cs | 47 +++++++++++++++------------
 1 file changed, 27 insertions(+), 20 deletions(-)

diff --git a/src/Runner.Worker/OSWarningChecker.cs b/src/Runner.Worker/OSWarningChecker.cs
index 464e49ec005..bd1ccc70297 100644
--- a/src/Runner.Worker/OSWarningChecker.cs
+++ b/src/Runner.Worker/OSWarningChecker.cs
@@ -1,10 +1,9 @@
 using System;
 using System.Collections.Generic;
 using System.IO;
+using System.Threading;
 using System.Threading.Tasks;
-using System.Text.RegularExpressions;
 using GitHub.DistributedTask.WebApi;
-using GitHub.DistributedTask.Pipelines;
 using GitHub.Runner.Common;
 using GitHub.Runner.Sdk;
 
@@ -27,10 +26,10 @@ public async Task CheckOSAsync(IExecutionContext context)
             }
 
             context.Output("Testing runner upgrade compatibility");
+            List<string> output = new();
+            object outputLock = new();
             try
             {
-                List<string> output = new();
-                object outputLock = new();
                 using (var process = HostContext.CreateService<IProcessInvoker>())
                 {
                     process.OutputDataReceived += delegate (object sender, ProcessDataReceivedEventArgs stdout)
@@ -57,24 +56,26 @@ public async Task CheckOSAsync(IExecutionContext context)
                         }
                     };
 
-                    int exitCode = await process.ExecuteAsync(
-                        workingDirectory: HostContext.GetDirectory(WellKnownDirectory.Root),
-                        fileName: Path.Combine(HostContext.GetDirectory(WellKnownDirectory.Bin), "testDotNet8Compatibility", $"TestDotNet8Compatibility{IOUtil.ExeExtension}"),
-                        arguments: string.Empty,
-                        environment: null,
-                        cancellationToken: context.CancellationToken);
-
-                    var outputStr = string.Join("\n", output).Trim();
-                    if (exitCode != 0 || !string.Equals(outputStr, "Hello from .NET 8!", StringComparison.Ordinal))
+                    using (var cancellationTokenSource = new CancellationTokenSource(TimeSpan.FromSeconds(10)))
                     {
-                        var warningMessage = context.Global.Variables.System_DotNet8CompatibilityWarning;
-                        if (!string.IsNullOrEmpty(warningMessage))
+                        int exitCode = await process.ExecuteAsync(
+                            workingDirectory: HostContext.GetDirectory(WellKnownDirectory.Root),
+                            fileName: Path.Combine(HostContext.GetDirectory(WellKnownDirectory.Bin), "testDotNet8Compatibility", $"TestDotNet8Compatibility{IOUtil.ExeExtension}"),
+                            arguments: string.Empty,
+                            environment: null,
+                            cancellationToken: cancellationTokenSource.Token);
+
+                        var outputStr = string.Join("\n", output).Trim();
+                        if (exitCode != 0 || !string.Equals(outputStr, "Hello from .NET 8!", StringComparison.Ordinal))
                         {
-                            context.Warning(warningMessage);
-                        }
+                            var warningMessage = context.Global.Variables.System_DotNet8CompatibilityWarning;
+                            if (!string.IsNullOrEmpty(warningMessage))
+                            {
+                                context.Warning(warningMessage);
+                            }
 
-                        var shortOutput = outputStr.Length > 200 ? string.Concat(outputStr.Substring(0, 200), "[...]") : outputStr;
-                        context.Global.JobTelemetry.Add(new JobTelemetry() { Type = JobTelemetryType.General, Message = $".NET 8 OS compatibility test failed with exit code '{exitCode}' and output: {shortOutput}" });
+                            context.Global.JobTelemetry.Add(new JobTelemetry() { Type = JobTelemetryType.General, Message = $".NET 8 OS compatibility test failed with exit code '{exitCode}' and output: {GetShortOutput(output)}" });
+                        }
                     }
                 }
             }
@@ -82,8 +83,14 @@ public async Task CheckOSAsync(IExecutionContext context)
             {
                 Trace.Error("An error occurred while testing .NET 8 compatibility'");
                 Trace.Error(ex);
-                context.Global.JobTelemetry.Add(new JobTelemetry() { Type = JobTelemetryType.General, Message = $"An error occurred while testing .NET 8 compatibility; exception type '{ex.GetType().FullName}'; message: {ex.Message}" });
+                context.Global.JobTelemetry.Add(new JobTelemetry() { Type = JobTelemetryType.General, Message = $".NET 8 OS compatibility test encountered exception type '{ex.GetType().FullName}', message: '{ex.Message}', process output: '{GetShortOutput(output)}'" });
             }
         }
+
+        private static string GetShortOutput(List<string> output)
+        {
+            var outputStr = string.Join("\n", output).Trim();
+            return outputStr.Length > 200 ? string.Concat(outputStr.Substring(0, 200), "[...]") : outputStr;
+        }
     }
 }