From fb0bd1f3579dee92fefda08e1c98a0c6707da892 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Thu, 17 Apr 2025 14:52:05 +0800 Subject: [PATCH 1/3] Add more gRPC interop test logging --- .../InteropTests/Helpers/ClientProcess.cs | 2 ++ .../Helpers/ProcessDebugHelper.cs | 33 +++++++++++++++++++ .../InteropTests/Helpers/WebsiteProcess.cs | 2 ++ .../Interop/test/InteropTests/InteropTests.cs | 5 +++ 4 files changed, 42 insertions(+) create mode 100644 src/Grpc/Interop/test/InteropTests/Helpers/ProcessDebugHelper.cs diff --git a/src/Grpc/Interop/test/InteropTests/Helpers/ClientProcess.cs b/src/Grpc/Interop/test/InteropTests/Helpers/ClientProcess.cs index 0104584535d2..4714e9bb0376 100644 --- a/src/Grpc/Interop/test/InteropTests/Helpers/ClientProcess.cs +++ b/src/Grpc/Interop/test/InteropTests/Helpers/ClientProcess.cs @@ -31,6 +31,8 @@ public ClientProcess(ITestOutputHelper output, string path, string serverPort, s _process.EnableRaisingEvents = true; _process.OutputDataReceived += Process_OutputDataReceived; _process.ErrorDataReceived += Process_ErrorDataReceived; + + output.WriteLine($"Starting process: {ProcessDebugHelper.GetDebugCommand(_process.StartInfo)}"); _process.Start(); _processEx = new ProcessEx(output, _process, timeout: Timeout.InfiniteTimeSpan); diff --git a/src/Grpc/Interop/test/InteropTests/Helpers/ProcessDebugHelper.cs b/src/Grpc/Interop/test/InteropTests/Helpers/ProcessDebugHelper.cs new file mode 100644 index 000000000000..b9a76684b3d7 --- /dev/null +++ b/src/Grpc/Interop/test/InteropTests/Helpers/ProcessDebugHelper.cs @@ -0,0 +1,33 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Diagnostics; + +namespace InteropTests.Helpers; + +public static class ProcessDebugHelper +{ + public static string GetDebugCommand(ProcessStartInfo psi) + { + // Quote the file name if it contains spaces or special characters + string fileName = QuoteIfNeeded(psi.FileName); + + // Arguments are typically already passed as a single string + string arguments = psi.Arguments; + + return $"{fileName} {arguments}".Trim(); + } + + private static string QuoteIfNeeded(string value) + { + if (string.IsNullOrWhiteSpace(value)) return "\"\""; + + // Add quotes if value contains spaces or special characters + if (value.Contains(" ") || value.Contains("\"")) + { + return $"\"{value.Replace("\"", "\\\"")}\""; + } + + return value; + } +} diff --git a/src/Grpc/Interop/test/InteropTests/Helpers/WebsiteProcess.cs b/src/Grpc/Interop/test/InteropTests/Helpers/WebsiteProcess.cs index cfd8dfb34be0..b187f01b913f 100644 --- a/src/Grpc/Interop/test/InteropTests/Helpers/WebsiteProcess.cs +++ b/src/Grpc/Interop/test/InteropTests/Helpers/WebsiteProcess.cs @@ -36,6 +36,8 @@ public WebsiteProcess(string path, ITestOutputHelper output) _process.EnableRaisingEvents = true; _process.OutputDataReceived += Process_OutputDataReceived; _process.ErrorDataReceived += Process_ErrorDataReceived; + + output.WriteLine($"Starting process: {ProcessDebugHelper.GetDebugCommand(_process.StartInfo)}"); _process.Start(); _processEx = new ProcessEx(output, _process, Timeout.InfiniteTimeSpan); diff --git a/src/Grpc/Interop/test/InteropTests/InteropTests.cs b/src/Grpc/Interop/test/InteropTests/InteropTests.cs index ad0f2e898439..991440eae685 100644 --- a/src/Grpc/Interop/test/InteropTests/InteropTests.cs +++ b/src/Grpc/Interop/test/InteropTests/InteropTests.cs @@ -83,10 +83,12 @@ public InteropTests(ITestOutputHelper output) private async Task InteropTestCase(string name) { + _output.WriteLine($"Starting {nameof(WebsiteProcess)}."); using (var serverProcess = new WebsiteProcess(_serverPath, _output)) { try { + _output.WriteLine($"Waiting for {nameof(WebsiteProcess)} to be ready."); await serverProcess.WaitForReady().TimeoutAfter(DefaultTimeout); } catch (Exception ex) @@ -102,12 +104,15 @@ private async Task InteropTestCase(string name) throw new InvalidOperationException(errorMessage, ex); } + _output.WriteLine($"Starting {nameof(ClientProcess)}."); using (var clientProcess = new ClientProcess(_output, _clientPath, serverProcess.ServerPort, name)) { try { + _output.WriteLine($"Waiting for {nameof(ClientProcess)} to be ready."); await clientProcess.WaitForReadyAsync().TimeoutAfter(DefaultTimeout); + _output.WriteLine($"Waiting for {nameof(ClientProcess)} to exit."); await clientProcess.WaitForExitAsync().TimeoutAfter(DefaultTimeout); Assert.Equal(0, clientProcess.ExitCode); From f7a2c4aba1fb8cdae5e7540d880d9180143a8b63 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Thu, 17 Apr 2025 15:25:08 +0800 Subject: [PATCH 2/3] Add retry --- .../Interop/test/InteropTests/InteropTests.cs | 38 ++++++++++++++++++- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/src/Grpc/Interop/test/InteropTests/InteropTests.cs b/src/Grpc/Interop/test/InteropTests/InteropTests.cs index 991440eae685..b57fda2d827f 100644 --- a/src/Grpc/Interop/test/InteropTests/InteropTests.cs +++ b/src/Grpc/Interop/test/InteropTests/InteropTests.cs @@ -9,6 +9,7 @@ namespace InteropTests; // All interop test cases, minus GCE authentication specific tests. // Tests are separate methods so that they can be quarantined separately. +[Retry] public class InteropTests { private static readonly TimeSpan DefaultTimeout = TimeSpan.FromSeconds(100); @@ -82,6 +83,39 @@ public InteropTests(ITestOutputHelper output) public Task ServerCompressedStreaming() => InteropTestCase("server_compressed_streaming"); private async Task InteropTestCase(string name) + { + // Building interop tests processes can be flaky. Sometimes it times out. + // To mitigate this, we retry the test case a few times on timeout. + const int maxRetries = 3; + var attempt = 0; + + while (true) + { + attempt++; + + try + { + await InteropTestCaseCore(name); + break; // Exit loop on success + } + catch (TimeoutException ex) + { + _output.WriteLine($"Attempt {attempt} failed: {ex.Message}"); + + if (attempt == maxRetries) + { + _output.WriteLine("Maximum retry attempts reached. Giving up."); + throw; + } + else + { + await Task.Delay(TimeSpan.FromSeconds(1)); + } + } + } + } + + private async Task InteropTestCaseCore(string name) { _output.WriteLine($"Starting {nameof(WebsiteProcess)}."); using (var serverProcess = new WebsiteProcess(_serverPath, _output)) @@ -91,7 +125,7 @@ private async Task InteropTestCase(string name) _output.WriteLine($"Waiting for {nameof(WebsiteProcess)} to be ready."); await serverProcess.WaitForReady().TimeoutAfter(DefaultTimeout); } - catch (Exception ex) + catch (Exception ex) when (ex is not TimeoutException) { var errorMessage = $@"Error while running server process. @@ -117,7 +151,7 @@ private async Task InteropTestCase(string name) Assert.Equal(0, clientProcess.ExitCode); } - catch (Exception ex) + catch (Exception ex) when (ex is not TimeoutException) { var errorMessage = $@"Error while running client process. From a06ae01cdb62be00caf1cf69ff0b988cb6853113 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Thu, 17 Apr 2025 15:49:12 +0800 Subject: [PATCH 3/3] Fix --- .../test/InteropTests/Helpers/ProcessDebugHelper.cs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/Grpc/Interop/test/InteropTests/Helpers/ProcessDebugHelper.cs b/src/Grpc/Interop/test/InteropTests/Helpers/ProcessDebugHelper.cs index b9a76684b3d7..2305c67953f8 100644 --- a/src/Grpc/Interop/test/InteropTests/Helpers/ProcessDebugHelper.cs +++ b/src/Grpc/Interop/test/InteropTests/Helpers/ProcessDebugHelper.cs @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System.Diagnostics; @@ -10,20 +10,23 @@ public static class ProcessDebugHelper public static string GetDebugCommand(ProcessStartInfo psi) { // Quote the file name if it contains spaces or special characters - string fileName = QuoteIfNeeded(psi.FileName); + var fileName = QuoteIfNeeded(psi.FileName); // Arguments are typically already passed as a single string - string arguments = psi.Arguments; + var arguments = psi.Arguments; return $"{fileName} {arguments}".Trim(); } private static string QuoteIfNeeded(string value) { - if (string.IsNullOrWhiteSpace(value)) return "\"\""; + if (string.IsNullOrWhiteSpace(value)) + { + return "\"\""; + } // Add quotes if value contains spaces or special characters - if (value.Contains(" ") || value.Contains("\"")) + if (value.Contains(' ') || value.Contains('"')) { return $"\"{value.Replace("\"", "\\\"")}\""; }