[Tests] Upgrade to xUnit v3 #2907

NickCraver · 2025-07-12T23:59:40Z

Upgrading xUnit to v3 to see if this helps our stability issues any. This runs as an executable making profiling much easier as well, so lends itself better to investigating bottlenecks.

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

NickCraver · 2025-07-14T14:05:05Z

@mgravell FYI: Seeing if I can get this more stable - found we were spending a lot of time in dispose due to waiting on the quits, but also figured xunit v3 was much easier to profile and such.

mgravell

Looking good to me

mgravell · 2025-07-18T17:49:58Z

Hey @NickCraver - what's the status of this one? I just don't want to get into massive merge hell. Anything I can do to help?

NickCraver · 2025-07-18T23:54:15Z

@mgravell Just running a few suites but I think this makes life better but doesn't solve the stability issues from CI (appears to be net neutral there). It makes profiling way easier though - if you're good here 👍

NickCraver · 2025-07-19T01:47:40Z

Actually - tweaking a few things based on more findings - don't sweat this, I'll eat the merge in.

mgravell · 2025-07-19T07:19:56Z

I'll eat the merge in

Screenshotting this for when you book a flight to convey your regrets mano-a-mano.

NickCraver · 2025-07-19T14:15:03Z

@mgravell Okay realized a big case: shared connection fixture was inadvertently connecting all tests to cluster if any cluster on a Create() ran first due to GetConfiguration() being per-class. Adding some guards and working on stability. I also need to get with Brad/VSTest folks about a problem navigating from test explorer in xUnit v3 latest:

System.ArgumentException: The parameter is incorrect. (Exception from HRESULT: 0x80070057 (E_INVALIDARG))
   at EnvDTE.ItemOperations.OpenFile(String FileName, String ViewKind)
   at Microsoft.VisualStudio.TestWindow.VsHost.IOpenTargetExtensions.<OpenFileAsync>d__2.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at Microsoft.VisualStudio.TestWindow.VsHost.IOpenTargetExtensions.<OpenFileAsync>d__2.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.VisualStudio.TestWindow.VsHost.IOpenTargetExtensions.<OpenAsync>d__0.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.VisualStudio.TestWindow.Internal.TestWindowViewModel.<<OnNavigationRequested>b__140_1>d.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.VisualStudio.TestWindow.Logging.ILoggerExtensions.<CallWithCatchAsync>d__12.MoveNext()
System.ArgumentException: The parameter is incorrect. (Exception from HRESULT: 0x80070057 (E_INVALIDARG))
   at EnvDTE.ItemOperations.OpenFile(String FileName, String ViewKind)
   at Microsoft.VisualStudio.TestWindow.VsHost.IOpenTargetExtensions.<OpenFileAsync>d__2.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at Microsoft.VisualStudio.TestWindow.VsHost.IOpenTargetExtensions.<OpenFileAsync>d__2.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.VisualStudio.TestWindow.VsHost.IOpenTargetExtensions.<OpenAsync>d__0.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.VisualStudio.TestWindow.Internal.TestWindowViewModel.<<OnNavigationRequested>b__140_1>d.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.VisualStudio.TestWindow.Logging.ILoggerExtensions.<CallWithCatchAsync>d__12.MoveNext()
System.ArgumentException: The parameter is incorrect. (Exception from HRESULT: 0x80070057 (E_INVALIDARG))
   at EnvDTE.ItemOperations.OpenFile(String FileName, String ViewKind)
   at Microsoft.VisualStudio.TestWindow.VsHost.IOpenTargetExtensions.<OpenFileAsync>d__2.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at Microsoft.VisualStudio.TestWindow.VsHost.IOpenTargetExtensions.<OpenFileAsync>d__2.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.VisualStudio.TestWindow.VsHost.IOpenTargetExtensions.<OpenAsync>d__0.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.VisualStudio.TestWindow.Internal.TestWindowViewModel.<<OnNavigationRequested>b__140_1>d.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.VisualStudio.TestWindow.Logging.ILoggerExtensions.<CallWithCatchAsync>d__12.MoveNext()
System.ArgumentException: The parameter is incorrect. (Exception from HRESULT: 0x80070057 (E_INVALIDARG))
   at EnvDTE.ItemOperations.OpenFile(String FileName, String ViewKind)
   at Microsoft.VisualStudio.TestWindow.VsHost.IOpenTargetExtensions.<OpenFileAsync>d__2.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at Microsoft.VisualStudio.TestWindow.VsHost.IOpenTargetExtensions.<OpenFileAsync>d__2.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.VisualStudio.TestWindow.VsHost.IOpenTargetExtensions.<OpenAsync>d__0.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.VisualStudio.TestWindow.Internal.TestWindowViewModel.<<OnNavigationRequested>b__140_1>d.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.ExceptionServices.ExceptionDispatchInfo.Throw()
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Microsoft.VisualStudio.TestWindow.Logging.ILoggerExtensions.<CallWithCatchAsync>d__12.MoveNext()

NickCraver · 2025-07-19T14:25:13Z

Also noticed that CancellationTests recent addition is pausing all clients - we should see how to better simulate that without the global pause - moving to non-parallel for now.

The script tests are another category where we're doing script and flush but the problem is the script itself is concurrently run between framework and races...could switch based on target framework/platform dynamically or something and 2 sets of scripts (1 per test suite target framework) would be enough...stupid, but effective. Will keep tweaking.

mgravell · 2025-07-19T14:39:40Z

Maybe the cancellation tests could use a decorator with fake latency / manual activation. Let me take that.

NickCraver · 2025-07-19T15:15:41Z

@mgravell Overall thinking of making a hostile test suite that runs separately and not parallel (with its own container/servers), but that doesn't need to happen in this PR - just aiming at equal or better. Note that xUnit v3 has an explicit test operator we can use but I think a separate assembly may be the way to go - VS kicks all off at once with shared servers is the rub to figure out.

On this I just need to at least fix AppVeyor (broken by this PR) - given how weak those servers are I'm thinking we just run net8.0 tests on AppVeyor in general, and later move packaging into Actions and remove AppVeyor altogether.

Thoughts?

NickCraver · 2025-07-19T18:04:24Z

@mgravell I suggest pulling this and see what you think locally. There's an annoying bug where clicking from test explorer doesn't go to the method - will file against xUnit but let me know if that's a blocker to merging here, you'd see it more than me coming up.

Overall, this should take you from 2min+ down to ~30sec locally and it reproduces some races we've been hunting after getting things much more parallel. I think if you select both suites and run until failure, you'll find some things I can yet fix and some very interesting races...like returning the wrong key, from another test:

It's not perfect yet but close on test stability and I can rev, but if it's easier to merge now and continue that, let's go that way and I'll do a follow-up. Definitely more on the table here.

mgravell · 2025-07-20T21:19:34Z

Seems fine in Rider...

Thanks for the effort - trying it.

mgravell · 2025-07-20T21:27:48Z

Looking great locally; a few Mono-related breaks when running as netfx, but I don't think I am going to obsess about that - even less so than netfx.

mgravell · 2025-07-20T21:28:12Z

(to be clear: those Mono breaks are not related to this PR, which runs very fast)

NickCraver · 2025-07-21T00:00:11Z

@mgravell I've seen this many times now - take a look at: https://github.com/StackExchange/StackExchange.Redis/pull/2907/checks?check_run_id=46350879681

 23:49:14.8058: Full info for: miscellaneous
 23:49:14.8058:   txt  ==>  # CPU
 23:49:14.8058:   used_cpu_sys  ==>  0.059433
 23:49:14.8059:   used_cpu_user  ==>  0.369478
 23:49:14.8059:   used_cpu_sys_children  ==>  0.000000
 23:49:14.8059:   used_cpu_user_children  ==>  0.001609
 23:49:14.8059:   used_cpu_sys_main_thread  ==>  0.058511
 23:49:14.8059:   used_cpu_user_main_thread  ==>  0.369911

Note the txt record, like the RESP3 processing is slightly off. I've seen it specifically on the INFO processor.

NickCraver · 2025-07-21T00:25:07Z

@mgravell I think this is a good pause point, will hands-off. Merge at will if you're good and I'll keep an eye on failures and help chase remaining, I think we're far net better as-is.

mgravell

Amazing stuff, thanks

mgravell · 2025-07-21T08:48:03Z

super happy, thanks; PRs are no longer fighting me with false positives:

NickCraver and others added 8 commits July 12, 2025 19:58

[Tests] Upgrade to xunit v3

04be977

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

More async and stabilization

e2fbd87

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

b9824a6

Fix CodeQL for latest C#

9e0fa42

Explicit .NET 9 install

710e08f

Revert TestAutomaticHeartbeat approach for now

f20cf5e

More IAsyncDisposable

9a82511

IAsyncDisposable for NoConnectionException

049d7c3

mgravell approved these changes Jul 17, 2025

View reviewed changes

NickCraver marked this pull request as ready for review July 18, 2025 23:54

NickCraver added 6 commits July 18, 2025 20:06

Fix shared on aborted connection

797de72

Merge remote-tracking branch 'origin/main' into craver/xunitv3

15547dc

Merge latest in

3fddf24

Tweak DisconnectAndReconnectThrowsConnectionExceptionSync

a9436ad

Buffer on abort tests

424f831

Saveeeeeee

0275776

NickCraver added 2 commits July 19, 2025 09:30

Fix sharing, method overlaps, source info + parallelism

2fc2ffb

Add console runner

bcaf40c

NickCraver marked this pull request as draft July 19, 2025 13:55

Prevent cluster tests running first from hosing the default connection

e2e53d1

More stability

a66ab33

NickCraver added 6 commits July 19, 2025 11:23

Run only net8.0 tests on AppVeyor

585ada8

Cleanup + stability

7d1aed2

Try speeding up Sentinel

6b6aca7

More key conflict cleanup

fe153c9

More optimization + stability

af3d52c

Idle test stability with split for time

81f0311

NickCraver marked this pull request as ready for review July 19, 2025 18:04

Cleanup + more stability/perf

86c67ca

ClusterNodeSubscriptionFailover

e7d0de0

More stability fixes

a409c2a

Cleanup

e0f79dd

mgravell approved these changes Jul 21, 2025

View reviewed changes

mgravell merged commit 5f58617 into main Jul 21, 2025
8 checks passed

mgravell deleted the craver/xunitv3 branch July 21, 2025 08:15

[Tests] Upgrade to xUnit v3 #2907

[Tests] Upgrade to xUnit v3 #2907

Uh oh!

Conversation

NickCraver commented Jul 12, 2025

Uh oh!

NickCraver commented Jul 14, 2025

Uh oh!

mgravell left a comment

Choose a reason for hiding this comment

Uh oh!

mgravell commented Jul 18, 2025

Uh oh!

NickCraver commented Jul 18, 2025

Uh oh!

NickCraver commented Jul 19, 2025

Uh oh!

mgravell commented Jul 19, 2025

Uh oh!

NickCraver commented Jul 19, 2025

Uh oh!

NickCraver commented Jul 19, 2025

Uh oh!

mgravell commented Jul 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NickCraver commented Jul 19, 2025

Uh oh!

NickCraver commented Jul 19, 2025

Uh oh!

mgravell commented Jul 20, 2025

Uh oh!

mgravell commented Jul 20, 2025

Uh oh!

mgravell commented Jul 20, 2025

Uh oh!

NickCraver commented Jul 21, 2025

Uh oh!

NickCraver commented Jul 21, 2025

Uh oh!

mgravell left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mgravell commented Jul 21, 2025

Uh oh!

Uh oh!

mgravell commented Jul 19, 2025 •

edited

Loading