Fix race condition in SBA, fix more flaky tests

.github/workflows/Steeltoe.All.yml

@@ -138,6 +138,7 @@ jobs:
         path: |
           ${{ github.workspace }}/TestOutput/**/*.dmp
           ${{ github.workspace }}/TestOutput/**/Sequence_*.xml
+          ${{ github.workspace }}/TestOutput/**/*.binlog
         if-no-files-found: ignore
 
     - name: Report test results

.github/workflows/component-shared-workflow.yml

@@ -112,6 +112,7 @@ jobs:
         path: |
           ${{ github.workspace }}/TestOutput/**/*.dmp
           ${{ github.workspace }}/TestOutput/**/Sequence_*.xml
+          ${{ github.workspace }}/TestOutput/**/*.binlog
         if-no-files-found: ignore
 
     - name: Report test results

.github/workflows/sonarcube.yml

@@ -101,12 +101,15 @@ jobs:
       run: dotnet build ${{ env.SOLUTION_FILE }} --no-restore --configuration Release --verbosity minimal /p:NuGetAuditLevel=low /p:WarningsNotAsErrors='${{ env.NUGET_VULNERABLE_PACKAGE_WARNINGS }}'
 
     - name: Test
+      id: test
       run: dotnet test ${{ env.SOLUTION_FILE }} --filter "Category!=MemoryDumps&Category!=GitProperties" ${{ env.COMMON_TEST_ARGS }} ${{ env.COVERAGE_COLLECT_ARGS }}
 
     - name: Test (memory dumps)
+      id: test-memory-dumps
       run: dotnet test src/Management/test/Endpoint.Test --filter "Category=MemoryDumps" ${{ env.COMMON_TEST_ARGS }} ${{ env.COVERAGE_COLLECT_ARGS }}
 
     - name: Test (git.properties)
+      id: test-gitproperties
       run: >-
         dotnet dotnet-coverage collect -f cobertura -o ${{ github.workspace }}/TestOutput/GitProperties.Build.Test.cobertura.xml --
         dotnet test src/Management/test/GitProperties.Build.Test ${{ env.COMMON_TEST_ARGS }}
@@ -119,6 +122,17 @@ jobs:
         -RepoRoot ${{ github.workspace }}
         -ProjectRelativePath src/Management/src/GitProperties.Build
 
+    - name: Upload crash/hang dumps (on failure)
+      if: ${{ !cancelled() && (steps.test.outcome == 'failure' || steps.test-memory-dumps.outcome == 'failure' || steps.test-gitproperties.outcome == 'failure') }}
+      uses: actions/upload-artifact@v7
+      with:
+        name: FailedTestOutput
+        path: |
+          ${{ github.workspace }}/TestOutput/**/*.dmp
+          ${{ github.workspace }}/TestOutput/**/Sequence_*.xml
+          ${{ github.workspace }}/TestOutput/**/*.binlog
+        if-no-files-found: ignore
+
     - name: End Sonar .NET scanner
       if: ${{ !cancelled() && steps.sonar_begin.outcome == 'success' }}
       env:

src/Common/src/Common/EventArgs.cs

@@ -0,0 +1,10 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the Apache 2.0 License.
+// See the LICENSE file in the project root for more information.
+
+namespace Steeltoe.Common;
+
+internal sealed class EventArgs<T>(T value) : EventArgs
+{
+    public T Value { get; } = value;
+}

src/Common/test/Certificates.Test/ConfigureCertificateOptionsTest.cs

@@ -189,9 +189,12 @@ public async Task CertificateOptions_update_on_changed_path(string certificateNa
         string firstPrivateKeyFilePath = sandbox.CreateFile(Guid.NewGuid() + ".key", firstPrivateKeyContent);
         using var secondX509 = X509Certificate2.CreateFromPemFile("secondInstance.crt", "secondInstance.key");
         string appSettings = BuildAppSettingsJson(certificateName, firstCertificateFilePath, firstPrivateKeyFilePath);
-        string appSettingsPath = sandbox.CreateFile("appsettings.json", appSettings);
+
+        var fileProvider = new MemoryFileProvider();
+        fileProvider.IncludeAppSettingsJsonFile(appSettings);
+
         var configurationBuilder = new ConfigurationBuilder();
-        configurationBuilder.AddJsonFile(appSettingsPath, false, true);
+        configurationBuilder.AddInMemoryAppSettingsJsonFile(fileProvider);
         IConfiguration configuration = configurationBuilder.Build();
 
         IServiceCollection services = new ServiceCollection();
@@ -204,10 +207,12 @@ public async Task CertificateOptions_update_on_changed_path(string certificateNa
         var optionsMonitor = serviceProvider.GetRequiredService<IOptionsMonitor<CertificateOptions>>();
         optionsMonitor.Get(certificateName).Certificate.Should().BeEquivalentTo(firstX509);
 
-        await WaitUntilCertificateChangedToAsync(certificateName, secondX509, optionsMonitor, async () =>
+        await WaitUntilCertificateChangedToAsync(certificateName, secondX509, optionsMonitor, () =>
         {
             appSettings = BuildAppSettingsJson(certificateName, "secondInstance.crt", "secondInstance.key");
-            await File.WriteAllTextAsync(appSettingsPath, appSettings, TestContext.Current.CancellationToken);
+            fileProvider.ReplaceAppSettingsJsonFile(appSettings);
+            fileProvider.NotifyChanged();
+            return Task.CompletedTask;
         });
 
         optionsMonitor.Get(certificateName).Certificate.Should().Be(secondX509);

src/Discovery/src/Eureka/EurekaClient.cs

@@ -42,6 +42,8 @@ public sealed partial class EurekaClient
     private readonly TimeProvider _timeProvider;
     private readonly ILogger<EurekaClient> _logger;
 
+    internal event EventHandler<EventArgs<TimeSpan>>? HttpClientTimeoutConfigured;
+
     public EurekaClient(IHttpClientFactory httpClientFactory, IOptionsMonitor<EurekaClientOptions> optionsMonitor,
         EurekaServiceUriStateManager eurekaServiceUriStateManager, TimeProvider timeProvider, ILogger<EurekaClient> logger)
     {
@@ -305,6 +307,7 @@ private HttpClient CreateHttpClient(string name, TimeSpan connectTimeout)
     {
         HttpClient httpClient = _httpClientFactory.CreateClient(name);
         httpClient.ConfigureForSteeltoe(connectTimeout);
+        HttpClientTimeoutConfigured?.Invoke(this, new EventArgs<TimeSpan>(httpClient.Timeout));
         return httpClient;
     }
 

src/Discovery/test/Eureka.Test/EurekaServiceCollectionExtensionsTest.cs

@@ -2,8 +2,7 @@
 // The .NET Foundation licenses this file to you under the Apache 2.0 License.
 // See the LICENSE file in the project root for more information.
 
-using System.Diagnostics;
-using System.Runtime.InteropServices;
+using FluentAssertions.Extensions;
 using Microsoft.Extensions.Configuration;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Hosting;
@@ -56,7 +55,7 @@ public async Task AddEurekaDiscoveryClient_UsesManagementOptions()
         options.Value.StatusPageUrlPath.Should().Be("/actuator/info");
     }
 
-    [FactSkippedOnPlatform(nameof(OSPlatform.OSX))]
+    [Fact]
     public async Task AddEurekaDiscoveryClient_UsesServerTimeout()
     {
         var appSettings = new Dictionary<string, string?>
@@ -70,16 +69,15 @@ public async Task AddEurekaDiscoveryClient_UsesServerTimeout()
         var services = new ServiceCollection();
         services.AddSingleton(configuration);
         services.AddEurekaDiscoveryClient();
-
         await using ServiceProvider serviceProvider = services.BuildServiceProvider(true);
 
-        var timer = new Stopwatch();
-        timer.Start();
+        var eurekaClient = serviceProvider.GetRequiredService<EurekaClient>();
+        TimeSpan? capturedTimeout = null;
+        eurekaClient.HttpClientTimeoutConfigured += (_, args) => capturedTimeout = args.Value;
 
         serviceProvider.GetServices<IDiscoveryClient>().Should().ContainSingle().Which.Should().BeOfType<EurekaDiscoveryClient>();
 
-        timer.Stop();
-        timer.ElapsedMilliseconds.Should().BeInRange(0, 3500);
+        capturedTimeout.Should().Be(1.Seconds());
     }
 
     [Fact]

src/Management/src/Endpoint/Actuators/ThreadDump/ThreadDumpEndpointOptions.cs

@@ -14,13 +14,8 @@ public sealed class ThreadDumpEndpointOptions : EndpointOptions
     public override EndpointPermissions RequiredPermissions { get; set; } = EndpointPermissions.Full;
 
     /// <summary>
-    /// Gets or sets the time (in milliseconds) to trace for, before automatically stopping the trace. Default value: 100.
+    /// Gets or sets how long to sample (in milliseconds). Higher values increase accuracy, at the cost of pausing the app for longer intervals. Default
+    /// value: 10.
     /// </summary>
-    /// <remarks>
-    /// This is how long the EventPipe session captures samples from the "Microsoft-DotNETCore-SampleProfiler" provider, which samples on-CPU threads at a
-    /// fixed ~1 ms interval. Threads that are not scheduled on a CPU core during this window (for example, because they are idle or blocked waiting on I/O)
-    /// may get few or no samples, so a short duration risks missing threads entirely. A longer duration increases the odds of observing such threads, at the
-    /// cost of the endpoint taking longer to respond.
-    /// </remarks>
-    public int Duration { get; set; } = 100;
+    public int Duration { get; set; } = 10;
 }

src/Management/src/Endpoint/ConfigurationSchema.json

@@ -784,7 +784,7 @@
                 },
                 "Duration": {
                   "type": "integer",
-                  "description": "Gets or sets the time (in milliseconds) to trace for, before automatically stopping the trace. Default value: 100."
+                  "description": "Gets or sets how long to sample (in milliseconds). Higher values increase accuracy, at the cost of pausing the app for longer intervals. Default value: 10."
                 },
                 "Enabled": {
                   "type": "boolean",

src/Management/src/Endpoint/SpringBootAdminClient/SpringBootAdminPeriodicRefresh.cs

@@ -44,20 +44,15 @@ private async Task TimerLoopAsync(TimeSpan interval)
 
             do
             {
-                // A tick queued just before periodic refresh was disabled would still be delivered here.
-                // Checking the period prevents executing a stale tick when refresh has been turned off.
-                if (isFirstTime || _periodicTimer.Period != Timeout.InfiniteTimeSpan)
+                LogStartingRefreshCycle();
+
+                try
+                {
+                    await _runner.RunAsync(isFirstTime, _timerTokenSource.Token);
+                }
+                catch (Exception exception) when (!exception.IsCancellation())
                 {
-                    LogStartingRefreshCycle();
-
-                    try
-                    {
-                        await _runner.RunAsync(isFirstTime, _timerTokenSource.Token);
-                    }
-                    catch (Exception exception) when (!exception.IsCancellation())
-                    {
-                        LogRefreshCycleFailed(exception);
-                    }
+                    LogRefreshCycleFailed(exception);
                 }
 
                 isFirstTime = false;

src/Management/src/Endpoint/SpringBootAdminClient/SpringBootAdminRefreshRunner.cs

@@ -61,6 +61,15 @@ public async Task RunAsync(bool isFirstTime, CancellationToken cancellationToken
         {
             LogValidatingOptions();
             SpringBootAdminClientOptions clientOptions = _clientOptionsMonitor.CurrentValue;
+
+            if (!isFirstTime && clientOptions.RefreshInterval <= TimeSpan.Zero)
+            {
+                // Periodic refresh was turned off since this cycle got scheduled. Skip it, using the very same options snapshot that
+                // made this decision, so a stale tick can never end up registering with a server that a concurrent configuration
+                // change just switched away from (which reading a separately-updated flag, like a timer's Period, could allow).
+                return;
+            }
+
             ValidateAndSetOptions(clientOptions);
 
             if (_lastGoodOptions?.Url != null && !string.Equals(_lastGoodOptions.Url, clientOptions.Url, StringComparison.OrdinalIgnoreCase))

src/Management/test/Endpoint.Test/Actuators/HeapDump/HeapDumperTest.cs

@@ -14,7 +14,8 @@ namespace Steeltoe.Management.Endpoint.Test.Actuators.HeapDump;
 [Trait("Category", "MemoryDumps")]
 public sealed class HeapDumperTest
 {
-    private static readonly TimeSpan DumpTimeout = TimeSpan.FromMinutes(3);
+    private static readonly TimeSpan DefaultDumpTimeout = TimeSpan.FromMinutes(3);
+    private static readonly TimeSpan LongDumpTimeout = TimeSpan.FromMinutes(5);
 
     [Theory]
     [InlineData(HeapDumpType.Full, "fulldump_", "full dump")]
@@ -45,7 +46,8 @@ public async Task Can_create_heap_dump(HeapDumpType heapDumpType, string fileNam
             return dumper.DumpHeapToFile(TestContext.Current.CancellationToken);
         }, TestContext.Current.CancellationToken);
 
-        string path = await dumpTask.WaitAsync(DumpTimeout, TestContext.Current.CancellationToken);
+        TimeSpan timeout = heapDumpType == HeapDumpType.Full ? LongDumpTimeout : DefaultDumpTimeout;
+        string path = await dumpTask.WaitAsync(timeout, TestContext.Current.CancellationToken);
 
         path.Should().Contain(fileName);
         File.Delete(path);

src/Management/test/Endpoint.Test/Actuators/HttpExchanges/DiagnosticObserverHttpExchangeRecorderTest.cs

@@ -181,12 +181,11 @@ public void Subscribe_to_listener_records_http_exchange()
             HttpContext = context
         });
 
-        SpinWait.SpinUntil(() => false, 1.Seconds());
-
+        activity.SetEndTime(activity.StartTimeUtc + 1.Seconds());
         listener.StopActivity(activity, context);
 
         HttpExchange httpExchange = recorded.Should().ContainSingle().Subject;
-        httpExchange.TimeTaken.Should().BeGreaterThan(900.Milliseconds()).And.BeLessThan(1300.Milliseconds());
+        httpExchange.TimeTaken.Should().Be(1.Seconds());
     }
 
     [Fact]

src/Management/test/Endpoint.Test/Actuators/ThreadDump/EventPipeThreadDumperTest.cs

@@ -32,7 +32,12 @@ public async Task Can_resolve_source_location_from_pdb()
         using var loggerFactory = new LoggerFactory([loggerProvider]);
         ILogger<EventPipeThreadDumper> logger = loggerFactory.CreateLogger<EventPipeThreadDumper>();
 
-        var optionsMonitor = new TestOptionsMonitor<ThreadDumpEndpointOptions>();
+        var optionsMonitor = TestOptionsMonitor.Create(new ThreadDumpEndpointOptions
+        {
+            // For testing, sampling accuracy is more important than app pauses.
+            Duration = 100
+        });
+
         var dumper = new EventPipeThreadDumper(optionsMonitor, logger);
 
         StackTraceElement? callbackFrame = null;

src/Management/test/Endpoint.Test/Actuators/ThreadDump/ThreadDumpActuatorTest.cs

@@ -43,7 +43,7 @@ public async Task Configures_default_settings()
 
         ThreadDumpEndpointOptions options = serviceProvider.GetRequiredService<IOptions<ThreadDumpEndpointOptions>>().Value;
 
-        options.Duration.Should().Be(100);
+        options.Duration.Should().Be(10);
         options.Enabled.Should().BeNull();
         options.Id.Should().Be("threaddump");
         options.Path.Should().Be("threaddump");

src/Management/test/GitProperties.Build.Test/FakeSteeltoeManagementEndpointPackager.cs

@@ -30,7 +30,7 @@ public static async Task<PackageReference> PackAsync(DirectoryInfo sessionDirect
         string projectFilePath = Path.Combine(sourceDirectory, $"{PackageId}.csproj");
         await File.WriteAllTextAsync(projectFilePath, projectContent, TestContext.Current.CancellationToken);
 
-        await ProcessRunner.RunDotNetAsync(sourceDirectory, 0, null, "build", "-c", "Release", $"-p:Version={PackageVersion}",
+        await ProcessRunner.RunDotNetBuildCapturingDiagnosticsOnFailureAsync(sourceDirectory, "fake-endpoint-build", $"-p:Version={PackageVersion}",
             $"-p:PackageOutputPath={nuGetFeedDirectory}");
 
         return new PackageReference(PackageId, PackageVersion, null);

src/Management/test/GitProperties.Build.Test/GitPropertiesSourcePackager.cs

@@ -20,7 +20,7 @@ public static async Task<PackageReference> PackAsync(string nuGetFeedDirectory)
         string sourceDirectory = Path.Combine(repositoryRoot, "src", "Management", "src", "GitProperties.Build");
         string packageVersion = $"{PackageVersionPrefix}.{$"{Guid.NewGuid():N}"[..8]}";
 
-        await ProcessRunner.RunDotNetAsync(sourceDirectory, 0, null, "build", "-c", "Release", $"-p:Version={packageVersion}",
+        await ProcessRunner.RunDotNetBuildCapturingDiagnosticsOnFailureAsync(sourceDirectory, "git-properties-build", $"-p:Version={packageVersion}",
             $"-p:PackageOutputPath={nuGetFeedDirectory}");
 
         return new PackageReference(PackageId, packageVersion, null);