From 6e4533d2a57dd572935a6e481ec22d5fb7a042fe Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Mon, 14 Jul 2025 15:36:46 -0700 Subject: [PATCH 1/5] Fix metric tests timeout issue --- .../tests/MetricsTests.cs | 49 ++++++++++--------- 1 file changed, 26 insertions(+), 23 deletions(-) diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs b/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs index 3a83e2ba346257..0a27231fae404f 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs @@ -13,6 +13,9 @@ namespace System.Diagnostics.Metrics.Tests { public class MetricsTests { + // We increase the timeout for remote execution to allow for longer-running tests. + private static RemoteInvokeOptions s_remoteExecutionOptions = new RemoteInvokeOptions { TimeOut = 600_000 }; + [Fact] public void MeasurementConstructionTest() { @@ -53,7 +56,7 @@ public void MeterConstructionTest() Assert.Equal("v1.0", meter.Version); Assert.Throws(() => new Meter(name: null)); - }).Dispose(); + }, s_remoteExecutionOptions).Dispose(); } [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] @@ -82,7 +85,7 @@ public void InstrumentCreationTest() ObservableGauge observableGauge = meter.CreateObservableGauge("ObservableGauge", () => 10, "Fahrenheit", "Fahrenheit ObservableGauge"); ValidateInstrumentInfo(observableGauge, "ObservableGauge", "Fahrenheit", "Fahrenheit ObservableGauge", false, true); - }).Dispose(); + }, s_remoteExecutionOptions).Dispose(); } [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] @@ -99,7 +102,7 @@ public void CreateInstrumentParametersTest() Assert.Throws(() => meter.CreateObservableUpDownCounter(null, () => 0, "items", "Items ObservableUpDownCounter")); Assert.Throws(() => meter.CreateObservableGauge(null, () => 0, "seconds", "Seconds ObservableGauge")); - }).Dispose(); + }, s_remoteExecutionOptions).Dispose(); } [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] @@ -169,7 +172,7 @@ public void SupportedGenericParameterTypesTest() Assert.Throws(() => meter.CreateHistogram("histogram1", "seconds", "Seconds histogram")); Assert.Throws(() => meter.CreateObservableCounter("observableCounter3", () => 0, "seconds", "Seconds ObservableCounter")); Assert.Throws(() => meter.CreateObservableGauge("observableGauge7", () => 0, "seconds", "Seconds ObservableGauge")); - }).Dispose(); + }, s_remoteExecutionOptions).Dispose(); } [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] @@ -228,7 +231,7 @@ public void ListeningToInstrumentsPublishingTest() // MeasurementsCompleted should be called 4 times for every instrument. Assert.Equal(0, instrumentsEncountered); } - }).Dispose(); + }, s_remoteExecutionOptions).Dispose(); } [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] @@ -280,7 +283,7 @@ public void ThrowingExceptionsFromObservableInstrumentCallbacks() Assert.Equal(11, accumulated); } - }).Dispose(); + }, s_remoteExecutionOptions).Dispose(); } [ConditionalTheory(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] @@ -376,7 +379,7 @@ public void InstrumentMeasurementTest(bool useSpan) Histogram histogram6 = meter.CreateHistogram("decimalHistogram"); InstrumentMeasurementAggregationValidation(histogram6, (value, tags) => { Record(histogram6, value, tags, useSpan); } ); - }, useSpan.ToString()).Dispose(); + }, useSpan.ToString(), s_remoteExecutionOptions).Dispose(); void AddToCounter(Counter counter, T delta, KeyValuePair[] tags, bool useSpan) where T : struct { @@ -723,7 +726,7 @@ public void ObservableInstrumentMeasurementTest() ObservableGauge observableGauge20 = meter.CreateObservableGauge("decimalObservableGauge", () => decimalGaugeMeasurementList); ObservableInstrumentMeasurementAggregationValidation(observableGauge20, decimalGaugeMeasurementList); - }).Dispose(); + }, s_remoteExecutionOptions).Dispose(); } [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] @@ -875,7 +878,7 @@ public void PassingVariableTagsParametersTest() PublishHistogramMeasurement(instrument as Histogram, value, tags); return (decimal)(value * 2); }); - }).Dispose(); + }, s_remoteExecutionOptions).Dispose(); } [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] @@ -963,7 +966,7 @@ public void MeterDisposalsTest() listener.RecordObservableInstruments(); Assert.Equal(13, count); - }).Dispose(); + }, s_remoteExecutionOptions).Dispose(); } [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] @@ -1034,7 +1037,7 @@ public void ListenerDisposalsTest() listener.RecordObservableInstruments(); Assert.Equal(7, count); - }).Dispose(); + }, s_remoteExecutionOptions).Dispose(); } [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] @@ -1102,7 +1105,7 @@ public void ListenerWithoutMeasurementsCompletedDisposalsTest() listener.RecordObservableInstruments(); Assert.Equal(7, count); - }).Dispose(); + }, s_remoteExecutionOptions).Dispose(); } [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] @@ -1147,7 +1150,7 @@ public void MultipleListenersTest() gauge.Record(1); Assert.Equal(15, count); - }).Dispose(); + }, s_remoteExecutionOptions).Dispose(); } [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] @@ -1184,7 +1187,7 @@ public void NullMeasurementEventCallbackTest() Assert.Equal(2, count); Assert.Throws(() => listener.SetMeasurementEventCallback(null)); - }).Dispose(); + }, s_remoteExecutionOptions).Dispose(); } [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] @@ -1225,7 +1228,7 @@ public void EnableListeningMultipleTimesWithDifferentState() listener.Dispose(); Assert.Equal(4, completedCount); - }).Dispose(); + }, s_remoteExecutionOptions).Dispose(); } [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] @@ -1268,7 +1271,7 @@ public void ParallelRunningTest() Task.WaitAll(taskList); Assert.Equal(loopLength * 17, totalCount); - }).Dispose(); + }, s_remoteExecutionOptions).Dispose(); } [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] @@ -1337,7 +1340,7 @@ public void SerializedEventsTest() Task.WaitAll(jobs); listener.Dispose(); Assert.Equal(0, instruments.Count); - }).Dispose(); + }, s_remoteExecutionOptions).Dispose(); } [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] @@ -1441,7 +1444,7 @@ public void TestRecordingMeasurementsWithTagList() expectedTags[8], expectedTags[9], expectedTags[10], expectedTags[11], expectedTags[12] }); } - }).Dispose(); + }, s_remoteExecutionOptions).Dispose(); } [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] @@ -1529,7 +1532,7 @@ public void TestMeterCreationWithOptions() Assert.Equal("10.0", meter10.Version); Assert.Equal("Scope10", meter10.Scope); Assert.Equal("TestMeterCreationWithOptions10", meter10.Name); - }).Dispose(); + }, s_remoteExecutionOptions).Dispose(); } [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] @@ -1662,7 +1665,7 @@ public void TestCachedInstruments() Gauge gauge12 = meter.CreateGauge("name9", null, null, t1); Assert.NotSame(gauge9, gauge12); - }).Dispose(); + }, s_remoteExecutionOptions).Dispose(); } [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] @@ -1728,7 +1731,7 @@ public void TestInstrumentCreationWithTags() { Assert.True(string.Compare(insArray[i].Key, insArray[i + 1].Key, StringComparison.Ordinal) <= 0); } - }).Dispose(); + }, s_remoteExecutionOptions).Dispose(); } [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] @@ -1748,7 +1751,7 @@ public void TestHistogramCreationWithAdvice() Assert.NotNull(histogramWithAdvice.Advice?.HistogramBucketBoundaries); Assert.Equal(explicitBucketBoundaries, histogramWithAdvice.Advice.HistogramBucketBoundaries); - }).Dispose(); + }, s_remoteExecutionOptions).Dispose(); } [ConditionalFact(typeof(RemoteExecutor), nameof(RemoteExecutor.IsSupported))] @@ -1771,7 +1774,7 @@ public void TestRecordingWithEmptyTagList() counter.Add(1, new TagList(Array.Empty>())); Assert.Equal(4, count); - }).Dispose(); + }, s_remoteExecutionOptions).Dispose(); } private void PublishCounterMeasurement(Counter counter, T value, KeyValuePair[] tags) where T : struct From 0a0b9cd22fbb3972632f5b7b655c768d242c077d Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed <10833894+tarekgh@users.noreply.github.com> Date: Mon, 14 Jul 2025 15:39:08 -0700 Subject: [PATCH 2/5] Update src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs b/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs index 0a27231fae404f..0260174e9df927 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs @@ -14,7 +14,7 @@ namespace System.Diagnostics.Metrics.Tests public class MetricsTests { // We increase the timeout for remote execution to allow for longer-running tests. - private static RemoteInvokeOptions s_remoteExecutionOptions = new RemoteInvokeOptions { TimeOut = 600_000 }; + private static readonly RemoteInvokeOptions s_remoteExecutionOptions = new RemoteInvokeOptions { TimeOut = TimeSpan.FromMinutes(10) }; [Fact] public void MeasurementConstructionTest() From 9cad8ddd397c76d643b8819a6186b83e4ce277d9 Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Tue, 15 Jul 2025 15:38:00 -0700 Subject: [PATCH 3/5] Fix the RemoteInvokeOptions initialization --- .../System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs b/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs index 0260174e9df927..0a27231fae404f 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs @@ -14,7 +14,7 @@ namespace System.Diagnostics.Metrics.Tests public class MetricsTests { // We increase the timeout for remote execution to allow for longer-running tests. - private static readonly RemoteInvokeOptions s_remoteExecutionOptions = new RemoteInvokeOptions { TimeOut = TimeSpan.FromMinutes(10) }; + private static RemoteInvokeOptions s_remoteExecutionOptions = new RemoteInvokeOptions { TimeOut = 600_000 }; [Fact] public void MeasurementConstructionTest() From 73636cbb58b8587e7844a8da15ec3bc64e7870dd Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Tue, 15 Jul 2025 15:45:29 -0700 Subject: [PATCH 4/5] mark the propert as readonly --- .../System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs b/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs index 0a27231fae404f..2c09c667e7aa06 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs @@ -14,7 +14,7 @@ namespace System.Diagnostics.Metrics.Tests public class MetricsTests { // We increase the timeout for remote execution to allow for longer-running tests. - private static RemoteInvokeOptions s_remoteExecutionOptions = new RemoteInvokeOptions { TimeOut = 600_000 }; + private static readonly RemoteInvokeOptions s_remoteExecutionOptions = new RemoteInvokeOptions { TimeOut = 600_000 }; [Fact] public void MeasurementConstructionTest() From a79a74f19658a29f6aa54865547012a0f80cb1cc Mon Sep 17 00:00:00 2001 From: Tarek Mahmoud Sayed Date: Wed, 16 Jul 2025 08:32:07 -0700 Subject: [PATCH 5/5] Ensure RemoteExecutor.IsSupported before creating RemoteInvokeOptions --- .../System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs b/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs index 2c09c667e7aa06..0face5c7c0ec16 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/tests/MetricsTests.cs @@ -14,7 +14,8 @@ namespace System.Diagnostics.Metrics.Tests public class MetricsTests { // We increase the timeout for remote execution to allow for longer-running tests. - private static readonly RemoteInvokeOptions s_remoteExecutionOptions = new RemoteInvokeOptions { TimeOut = 600_000 }; + // Ensure RemoteExecutor.IsSupported, otherwise the execution can throw PlatformNotSupportedException. + private static readonly RemoteInvokeOptions? s_remoteExecutionOptions = RemoteExecutor.IsSupported ? new RemoteInvokeOptions { TimeOut = 600_000 } : null; [Fact] public void MeasurementConstructionTest()