[Tests] Upgrade to xUnit v3 (#2907)

* [Tests] Upgrade to xunit v3

Upgrading xunit to v3 to see if this helps our stability issues any.

* More async and stabilization

* Don't install .NET 6 + 7 anymore on AppVeyor

* Fix CodeQL for latest C#

* Explicit .NET 9 install

* Revert TestAutomaticHeartbeat approach for now

* More IAsyncDisposable

* IAsyncDisposable for NoConnectionException

* Fix shared on aborted connection

* Merge latest in

* Tweak DisconnectAndReconnectThrowsConnectionExceptionSync

* Buffer on abort tests

* Saveeeeeee

* Fix sharing, method overlaps, source info + parallelism

* Add console runner

* Prevent cluster tests running first from hosing the default connection

* More stability

* Run only net8.0 tests on AppVeyor

* Cleanup + stability

* Try speeding up Sentinel

* More key conflict cleanup

* More optimization + stability

* Idle test stability with split for time

* Cleanup + more stability/perf

* ClusterNodeSubscriptionFailover

* More stability fixes

* Cleanup

Author: NickCraver

.github/workflows/codeql.yml

@@ -29,13 +29,18 @@ jobs:
 
     steps:
     - name: Checkout repository
-      uses: actions/checkout@v3
+      uses: actions/checkout@v4
       with:
         fetch-depth: 0
 
+    - name: Setup .NET
+      uses: actions/setup-dotnet@v4
+      with:
+        dotnet-version: '9.0.x'
+
     # Initializes the CodeQL tools for scanning.
     - name: Initialize CodeQL
-      uses: github/codeql-action/init@v2
+      uses: github/codeql-action/init@v3
       with:
         languages: ${{ matrix.language }}
         # If you wish to specify custom queries, you can do so here or in a config file.
@@ -45,18 +50,11 @@ jobs:
         # For more details on CodeQL's query packs, refer to: https://docs.github.com/en/code-security/code-scanning/automatically-scanning-your-code-for-vulnerabilities-and-errors/configuring-code-scanning#using-queries-in-ql-packs
         # queries: security-extended,security-and-quality
 
-
-    # Autobuild attempts to build any compiled languages (C/C++, C#, Go, Java, or Swift).
-    # If this step fails, then you should remove it and run the build manually (see below)
-    - if: matrix.language != 'csharp'
-      name: Autobuild
-      uses: github/codeql-action/autobuild@v2
-
     - if: matrix.language == 'csharp'
       name: .NET Build
       run: dotnet build Build.csproj -c Release /p:CI=true
 
     - name: Perform CodeQL Analysis
-      uses: github/codeql-action/analyze@v2
+      uses: github/codeql-action/analyze@v3
       with:
         category: "/language:${{matrix.language}}"

Directory.Build.props

@@ -15,7 +15,7 @@
     <PackageProjectUrl>https://stackexchange.github.io/StackExchange.Redis/</PackageProjectUrl>
     <PackageLicenseExpression>MIT</PackageLicenseExpression>
 
-    <LangVersion>11</LangVersion>
+    <LangVersion>13</LangVersion>
     <RepositoryType>git</RepositoryType>
     <RepositoryUrl>https://github.com/StackExchange/StackExchange.Redis/</RepositoryUrl>
 

Directory.Packages.props

@@ -15,6 +15,7 @@
     <PackageVersion Include="Microsoft.NETFramework.ReferenceAssemblies" Version="1.0.3" />
     <PackageVersion Include="Microsoft.NET.Test.Sdk" Version="17.14.1" />
     <PackageVersion Include="Microsoft.SourceLink.GitHub" Version="8.0.0" />
+    <PackageVersion Include="Microsoft.Testing.Platform" Version="1.7.3" />
     <PackageVersion Include="Nerdbank.GitVersioning" Version="3.7.115" />
     <PackageVersion Include="Newtonsoft.Json" Version="13.0.3" />
     <PackageVersion Include="NSubstitute" Version="5.3.0" />
@@ -25,7 +26,8 @@
     <!-- For binding redirect testing, main package gets this transitively -->
     <PackageVersion Include="System.IO.Pipelines" Version="9.0.0" />
     <PackageVersion Include="System.Runtime.Caching" Version="9.0.0" />
-    <PackageVersion Include="xunit" Version="2.9.3" />
-    <PackageVersion Include="xunit.runner.visualstudio" Version="3.1.1" />
+    <PackageVersion Include="xunit.v3" Version="3.0.0" />
+    <PackageVersion Include="xunit.v3.runner.console" Version="3.0.0" />
+    <PackageVersion Include="xunit.runner.visualstudio" Version="3.1.3" />
   </ItemGroup>
 </Project>

appveyor.yml

@@ -6,10 +6,6 @@ init:
 
 install:
 - cmd: >-
-    choco install dotnet-6.0-sdk
-
-    choco install dotnet-7.0-sdk
-    
     choco install dotnet-9.0-sdk
 
     cd tests\RedisConfigs\3.0.503
@@ -71,7 +67,7 @@ nuget:
   disable_publish_on_pr: true
 
 build_script:
-- ps: .\build.ps1 -PullRequestNumber "$env:APPVEYOR_PULL_REQUEST_NUMBER" -CreatePackages ($env:OS -eq "Windows_NT")
+- ps: .\build.ps1 -PullRequestNumber "$env:APPVEYOR_PULL_REQUEST_NUMBER" -CreatePackages ($env:OS -eq "Windows_NT") -NetCoreOnlyTests
 
 test: off
 artifacts:

build.ps1

@@ -3,7 +3,8 @@ param(
     [bool] $CreatePackages,
     [switch] $StartServers,
     [bool] $RunTests = $true,
-    [string] $PullRequestNumber
+    [string] $PullRequestNumber,
+    [switch] $NetCoreOnlyTests
 )
 
 Write-Host "Run Parameters:" -ForegroundColor Cyan
@@ -29,7 +30,11 @@ if ($RunTests) {
         Write-Host "Servers Started." -ForegroundColor "Green"
     }
     Write-Host "Running tests: Build.csproj traversal (all frameworks)" -ForegroundColor "Magenta"
-    dotnet test ".\Build.csproj" -c Release --no-build --logger trx
+    if ($NetCoreOnlyTests) {
+        dotnet test ".\Build.csproj" -c Release -f net8.0 --no-build --logger trx
+    } else {
+        dotnet test ".\Build.csproj" -c Release --no-build --logger trx
+    }
     if ($LastExitCode -ne 0) {
         Write-Host "Error with tests, aborting build." -Foreground "Red"
         Exit 1

src/StackExchange.Redis/ConnectionMultiplexer.cs

@@ -535,7 +535,7 @@ static void LogWithThreadPoolStats(ILogger? log, string message, out int busyWor
                     }
                     try
                     {
-                        await Task.WhenAny(task, Task.Delay(remaining)).ObserveErrors().ForAwait();
+                        await task.TimeoutAfter(remaining).ObserveErrors().ForAwait();
                     }
                     catch
                     { }

src/StackExchange.Redis/TaskExtensions.cs

@@ -34,7 +34,7 @@ internal static Task<T> ObserveErrors<T>(this Task<T> task)
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         internal static ConfiguredValueTaskAwaitable<T> ForAwait<T>(this in ValueTask<T> task) => task.ConfigureAwait(false);
 
-        internal static void RedisFireAndForget(this Task task) => task?.ContinueWith(t => GC.KeepAlive(t.Exception), TaskContinuationOptions.OnlyOnFaulted);
+        internal static void RedisFireAndForget(this Task task) => task?.ContinueWith(static t => GC.KeepAlive(t.Exception), TaskContinuationOptions.OnlyOnFaulted);
 
         /// <summary>
         /// Licensed to the .NET Foundation under one or more agreements.

tests/StackExchange.Redis.Tests/AbortOnConnectFailTests.cs

@@ -2,18 +2,15 @@
 using System.Threading.Tasks;
 using StackExchange.Redis.Tests.Helpers;
 using Xunit;
-using Xunit.Abstractions;
 
 namespace StackExchange.Redis.Tests;
 
-public class AbortOnConnectFailTests : TestBase
+public class AbortOnConnectFailTests(ITestOutputHelper output) : TestBase(output)
 {
-    public AbortOnConnectFailTests(ITestOutputHelper output) : base(output) { }
-
     [Fact]
-    public void NeverEverConnectedNoBacklogThrowsConnectionNotAvailableSync()
+    public async Task NeverEverConnectedNoBacklogThrowsConnectionNotAvailableSync()
     {
-        using var conn = GetFailFastConn();
+        await using var conn = GetFailFastConn();
         var db = conn.GetDatabase();
         var key = Me();
 
@@ -26,7 +23,7 @@ public void NeverEverConnectedNoBacklogThrowsConnectionNotAvailableSync()
     [Fact]
     public async Task NeverEverConnectedNoBacklogThrowsConnectionNotAvailableAsync()
     {
-        using var conn = GetFailFastConn();
+        await using var conn = GetFailFastConn();
         var db = conn.GetDatabase();
         var key = Me();
 
@@ -37,13 +34,13 @@ public async Task NeverEverConnectedNoBacklogThrowsConnectionNotAvailableAsync()
     }
 
     [Fact]
-    public void DisconnectAndReconnectThrowsConnectionExceptionSync()
+    public async Task DisconnectAndReconnectThrowsConnectionExceptionSync()
     {
-        using var conn = GetWorkingBacklogConn();
+        await using var conn = GetWorkingBacklogConn();
 
         var db = conn.GetDatabase();
         var key = Me();
-        _ = db.Ping(); // Doesn't throw - we're connected
+        await db.PingAsync(); // Doesn't throw - we're connected
 
         // Disconnect and don't allow re-connection
         conn.AllowConnect = false;
@@ -54,7 +51,7 @@ public void DisconnectAndReconnectThrowsConnectionExceptionSync()
         var ex = Assert.ThrowsAny<Exception>(() => db.Ping());
         Log("Exception: " + ex.Message);
         Assert.True(ex is RedisConnectionException or RedisTimeoutException);
-        Assert.StartsWith("The message timed out in the backlog attempting to send because no connection became available (400ms) - Last Connection Exception: ", ex.Message);
+        Assert.StartsWith("The message timed out in the backlog attempting to send because no connection became available (1000ms) - Last Connection Exception: ", ex.Message);
         Assert.NotNull(ex.InnerException);
         var iex = Assert.IsType<RedisConnectionException>(ex.InnerException);
         Assert.Contains(iex.Message, ex.Message);
@@ -63,11 +60,11 @@ public void DisconnectAndReconnectThrowsConnectionExceptionSync()
     [Fact]
     public async Task DisconnectAndNoReconnectThrowsConnectionExceptionAsync()
     {
-        using var conn = GetWorkingBacklogConn();
+        await using var conn = GetWorkingBacklogConn();
 
         var db = conn.GetDatabase();
         var key = Me();
-        _ = db.Ping(); // Doesn't throw - we're connected
+        await db.PingAsync(); // Doesn't throw - we're connected
 
         // Disconnect and don't allow re-connection
         conn.AllowConnect = false;
@@ -77,25 +74,25 @@ public async Task DisconnectAndNoReconnectThrowsConnectionExceptionAsync()
         // Exception: The message timed out in the backlog attempting to send because no connection became available (400ms) - Last Connection Exception: SocketFailure (InputReaderCompleted, last-recv: 7) on 127.0.0.1:6379/Interactive, Idle/ReadAsync, last: PING, origin: SimulateConnectionFailure, outstanding: 0, last-read: 0s ago, last-write: 0s ago, keep-alive: 100s, state: ConnectedEstablished, mgr: 8 of 10 available, in: 0, in-pipe: 0, out-pipe: 0, last-heartbeat: never, last-mbeat: 0s ago, global: 0s ago, v: 2.6.120.51136, command=PING, timeout: 100, inst: 0, qu: 0, qs: 0, aw: False, bw: CheckingForTimeout, last-in: 0, cur-in: 0, sync-ops: 1, async-ops: 1, serverEndpoint: 127.0.0.1:6379, conn-sec: n/a, aoc: 0, mc: 1/1/0, mgr: 8 of 10 available, clientName: CRAVERTOP7(SE.Redis-v2.6.120.51136), IOCP: (Busy=0,Free=1000,Min=16,Max=1000), WORKER: (Busy=6,Free=32761,Min=16,Max=32767), POOL: (Threads=33,QueuedItems=0,CompletedItems=5547,Timers=60), v: 2.6.120.51136 (Please take a look at this article for some common client-side issues that can cause timeouts: https://stackexchange.github.io/StackExchange.Redis/Timeouts)
         var ex = await Assert.ThrowsAsync<RedisConnectionException>(() => db.PingAsync());
         Log("Exception: " + ex.Message);
-        Assert.StartsWith("The message timed out in the backlog attempting to send because no connection became available (400ms) - Last Connection Exception: ", ex.Message);
+        Assert.StartsWith("The message timed out in the backlog attempting to send because no connection became available (1000ms) - Last Connection Exception: ", ex.Message);
         Assert.NotNull(ex.InnerException);
         var iex = Assert.IsType<RedisConnectionException>(ex.InnerException);
         Assert.Contains(iex.Message, ex.Message);
     }
 
     private ConnectionMultiplexer GetFailFastConn() =>
-        ConnectionMultiplexer.Connect(GetOptions(BacklogPolicy.FailFast).Apply(o => o.EndPoints.Add($"doesnot.exist.{Guid.NewGuid():N}:6379")), Writer);
+        ConnectionMultiplexer.Connect(GetOptions(BacklogPolicy.FailFast, 400).Apply(o => o.EndPoints.Add($"doesnot.exist.{Guid.NewGuid():N}:6379")), Writer);
 
     private ConnectionMultiplexer GetWorkingBacklogConn() =>
-        ConnectionMultiplexer.Connect(GetOptions(BacklogPolicy.Default).Apply(o => o.EndPoints.Add(GetConfiguration())), Writer);
+        ConnectionMultiplexer.Connect(GetOptions(BacklogPolicy.Default, 1000).Apply(o => o.EndPoints.Add(GetConfiguration())), Writer);
 
-    private ConfigurationOptions GetOptions(BacklogPolicy policy) => new ConfigurationOptions()
+    private static ConfigurationOptions GetOptions(BacklogPolicy policy, int duration) => new ConfigurationOptions()
     {
         AbortOnConnectFail = false,
         BacklogPolicy = policy,
         ConnectTimeout = 500,
-        SyncTimeout = 400,
-        KeepAlive = 400,
+        SyncTimeout = duration,
+        KeepAlive = duration,
         AllowAdmin = true,
     }.WithoutSubscriptions();
 }

tests/StackExchange.Redis.Tests/AdhocTests.cs

@@ -1,17 +1,14 @@
-using Xunit;
-using Xunit.Abstractions;
+using System.Threading.Tasks;
+using Xunit;
 
 namespace StackExchange.Redis.Tests;
 
-[Collection(SharedConnectionFixture.Key)]
-public class AdhocTests : TestBase
+public class AdhocTests(ITestOutputHelper output, SharedConnectionFixture fixture) : TestBase(output, fixture)
 {
-    public AdhocTests(ITestOutputHelper output, SharedConnectionFixture fixture) : base(output, fixture) { }
-
     [Fact]
-    public void TestAdhocCommandsAPI()
+    public async Task TestAdhocCommandsAPI()
     {
-        using var conn = Create();
+        await using var conn = Create();
         var db = conn.GetDatabase();
 
         // needs explicit RedisKey type for key-based

tests/StackExchange.Redis.Tests/AggressiveTests.cs

@@ -1,18 +1,16 @@
 using System.Threading;
 using System.Threading.Tasks;
 using Xunit;
-using Xunit.Abstractions;
 
 namespace StackExchange.Redis.Tests;
 
 [Collection(NonParallelCollection.Name)]
-public class AggressiveTests : TestBase
+public class AggressiveTests(ITestOutputHelper output) : TestBase(output)
 {
-    public AggressiveTests(ITestOutputHelper output) : base(output) { }
-
-    [FactLongRunning]
+    [Fact]
     public async Task ParallelTransactionsWithConditions()
     {
+        Skip.UnlessLongRunning();
         const int Muxers = 4, Workers = 20, PerThread = 250;
 
         var muxers = new IConnectionMultiplexer[Muxers];
@@ -24,7 +22,7 @@ public async Task ParallelTransactionsWithConditions()
             RedisKey hits = Me(), trigger = Me() + "3";
             int expectedSuccess = 0;
 
-            await muxers[0].GetDatabase().KeyDeleteAsync(new[] { hits, trigger }).ForAwait();
+            await muxers[0].GetDatabase().KeyDeleteAsync([hits, trigger]).ForAwait();
 
             Task[] tasks = new Task[Workers];
             for (int i = 0; i < tasks.Length; i++)
@@ -73,10 +71,11 @@ public async Task ParallelTransactionsWithConditions()
 
     private const int IterationCount = 5000, InnerCount = 20;
 
-    [FactLongRunning]
-    public void RunCompetingBatchesOnSameMuxer()
+    [Fact]
+    public async Task RunCompetingBatchesOnSameMuxer()
     {
-        using var conn = Create();
+        Skip.UnlessLongRunning();
+        await using var conn = Create();
         var db = conn.GetDatabase();
 
         Thread x = new Thread(state => BatchRunPings((IDatabase)state!))
@@ -132,10 +131,11 @@ private static void BatchRunPings(IDatabase db)
         }
     }
 
-    [FactLongRunning]
+    [Fact]
     public async Task RunCompetingBatchesOnSameMuxerAsync()
     {
-        using var conn = Create();
+        Skip.UnlessLongRunning();
+        await using var conn = Create();
         var db = conn.GetDatabase();
 
         var x = Task.Run(() => BatchRunPingsAsync(db));
@@ -189,10 +189,11 @@ private static async Task BatchRunPingsAsync(IDatabase db)
         }
     }
 
-    [FactLongRunning]
-    public void RunCompetingTransactionsOnSameMuxer()
+    [Fact]
+    public async Task RunCompetingTransactionsOnSameMuxer()
     {
-        using var conn = Create(logTransactionData: false);
+        Skip.UnlessLongRunning();
+        await using var conn = Create(logTransactionData: false);
         var db = conn.GetDatabase();
 
         Thread x = new Thread(state => TranRunPings((IDatabase)state!))
@@ -252,10 +253,11 @@ private void TranRunPings(IDatabase db)
         }
     }
 
-    [FactLongRunning]
+    [Fact]
     public async Task RunCompetingTransactionsOnSameMuxerAsync()
     {
-        using var conn = Create(logTransactionData: false);
+        Skip.UnlessLongRunning();
+        await using var conn = Create(logTransactionData: false);
         var db = conn.GetDatabase();
 
         var x = Task.Run(() => TranRunPingsAsync(db));