Apply LocalFact and LocalTheory attributes to racy tests (#6012)

* Add LocalTheoryAttribute

* Apply LocalFact and LocalTheory attributes to racy tests

* Fix XML-Doc warnings

* Add LocalTheoryAttribute and WindowsFactAttribute

* Convert skipped tests to LocalFact tests

* Fix WindowsFactAttribute

* Fix specs

* Add missing racy fact

* skip FlattenMerge

* Skip some more racy tests

build-system/azure-pipeline.template.yaml

@@ -17,6 +17,11 @@ jobs:
     timeoutInMinutes: ${{ parameters.timeoutInMinutes }}
     pool:
       vmImage: ${{ parameters.vmImage }}
+    variables:
+      - name: xunit.skip.local.fact
+        value: true
+      - name: xunit.skip.local.theory
+        value: true
     steps:
       - task: UseDotNet@2
         displayName: 'Use .NET 6 SDK 6.0.100'

src/contrib/cluster/Akka.Cluster.Metrics.Tests/MetricsCollectorSpec.cs

@@ -12,6 +12,7 @@
 using Akka.Cluster.Metrics.Tests.Base;
 using Akka.Cluster.Metrics.Tests.Helpers;
 using Akka.TestKit;
+using Akka.TestKit.Xunit2.Attributes;
 using Akka.Util.Extensions;
 using Akka.Util.Internal;
 using FluentAssertions;
@@ -88,8 +89,8 @@ public void MetricsCollector_should_collector_accurate_metrics_for_node()
             });
         }
 
-        [Fact(Skip = "This performance really depends on current load - so while should work well with specified timeouts," +
-                     "let's disable it to avoid flaky failures in future")]
+        [LocalFact(SkipLocal = "This performance really depends on current load - so while should work well with " +
+                               "specified timeouts, let's disable it to avoid flaky failures in future")]
         public async Task MetricsCollector_should_collect_50_node_metrics_samples_in_an_acceptable_duration()
         {
             const int iterationsCount = 50;

src/contrib/cluster/Akka.Cluster.Sharding.Tests/Internal/RememberEntitiesStarterSpec.cs

@@ -13,6 +13,7 @@
 using Akka.Cluster.Tools.Singleton;
 using Akka.Configuration;
 using Akka.TestKit;
+using Akka.TestKit.Xunit2.Attributes;
 using FluentAssertions;
 using Xunit;
 using Xunit.Abstractions;
@@ -139,7 +140,7 @@ public void RememberEntitiesStarter_must_inform_the_shard_when_entities_has_been
             ExpectTerminated(rememberEntityStarter);
         }
 
-        [Fact]
+        [LocalFact(SkipLocal = "Racy in Azure AzDo, strict timing does not work well on AzDo")]
         public void RememberEntitiesStarter_must_try_start_all_entities_in_a_throttled_way_with_entity_recovery_strategy_constant()
         {
             var regionProbe = CreateTestProbe();

src/contrib/testkits/Akka.TestKit.Xunit2/Akka.TestKit.Xunit2.csproj

@@ -1,5 +1,5 @@
 <Project Sdk="Microsoft.NET.Sdk">
-    <Import Project="..\..\..\common.props"/>
+    <Import Project="..\..\..\common.props" />
 
     <PropertyGroup>
         <AssemblyTitle>Akka.TestKit.Xunit2</AssemblyTitle>
@@ -10,8 +10,8 @@
     </PropertyGroup>
 
     <ItemGroup>
-        <ProjectReference Include="..\..\..\core\Akka.TestKit\Akka.TestKit.csproj"/>
-        <PackageReference Include="xunit" Version="$(XunitVersion)"/>
+        <ProjectReference Include="..\..\..\core\Akka.TestKit\Akka.TestKit.csproj" />
+        <PackageReference Include="xunit" Version="$(XunitVersion)" />
     </ItemGroup>
 
     <PropertyGroup Condition=" '$(Configuration)' == 'Release' ">

src/contrib/testkits/Akka.TestKit.Xunit2/Attributes/LocalFactAttribute.cs

@@ -6,27 +6,30 @@
 
 using System;
 using Xunit;
+using Xunit.Sdk;
 
 namespace Akka.TestKit.Xunit2.Attributes
 {
     /// <summary>
     /// <para>
     /// This custom XUnit Fact attribute will skip unit tests if the environment variable
-    /// "XUNIT_SKIP_LOCAL_FACTS" exists and is set to the string "true"
+    /// "XUNIT_SKIP_LOCAL_FACT" exists and is set to the string "true"
     /// </para>
     /// <para>
-    /// Note that the original <see cref="FactAttribute.Skip"/> property takes precedence over this attribute,
-    /// any unit tests with <see cref="LocalFactAttribute"/> with its <see cref="FactAttribute.Skip"/> property
+    /// Note that the original <see cref="Skip"/> property takes precedence over this attribute,
+    /// any unit tests with <see cref="LocalFactAttribute"/> with its <see cref="Skip"/> property
     /// set will always be skipped, regardless of the environment variable content.
     /// </para>
     /// </summary>
     public class LocalFactAttribute: FactAttribute
     {
-        private const string EnvironmentVariableName = "XUNIT_SKIP_LOCAL_FACTS";
+        private const string EnvironmentVariableName = "XUNIT_SKIP_LOCAL_FACT";
 
         private string _skip;
 
-        /// <inheritdoc cref="FactAttribute.Skip"/>
+        /// <summary>
+        /// Marks the test so that it will not be run, and gets or sets the skip reason
+        /// </summary>
         public override string Skip
         {
             get
@@ -46,6 +49,6 @@ public override string Skip
         /// Note that the original <see cref="FactAttribute.Skip"/> property takes precedence over this message. 
         /// </summary>
         public string SkipLocal { get; set; }
-    }    
+    }
 }
 

src/contrib/testkits/Akka.TestKit.Xunit2/Attributes/LocalTheoryAttribute.cs

@@ -0,0 +1,57 @@
+// -----------------------------------------------------------------------
+//  <copyright file="LocalTheoryAttribute.cs" company="Akka.NET Project">
+//      Copyright (C) 2009-2022 Lightbend Inc. <http://www.lightbend.com>
+//      Copyright (C) 2013-2022 .NET Foundation <https://github.com/akkadotnet/akka.net>
+//  </copyright>
+// -----------------------------------------------------------------------
+
+using System;
+using Xunit;
+using Xunit.Sdk;
+
+namespace Akka.TestKit.Xunit2.Attributes
+{
+    /// <summary>
+    /// <para>
+    /// This custom XUnit Fact attribute will skip unit tests if the environment variable
+    /// "XUNIT_SKIP_LOCAL_THEORY" exists and is set to the string "true"
+    /// </para>
+    /// <para>
+    /// Note that the original <see cref="Skip"/> property takes precedence over this attribute,
+    /// any unit tests with <see cref="LocalTheoryAttribute"/> with its <see cref="Skip"/> property
+    /// set will always be skipped, regardless of the environment variable content.
+    /// </para>
+    /// </summary>
+    [XunitTestCaseDiscoverer("Xunit.Sdk.TheoryDiscoverer", "xunit.execution.{Platform}")]
+    [AttributeUsage(AttributeTargets.Method, AllowMultiple = false)]
+    public class LocalTheoryAttribute : TheoryAttribute
+    {
+        private const string EnvironmentVariableName = "XUNIT_SKIP_LOCAL_THEORY";
+
+        private string _skip;
+
+        /// <summary>
+        /// Marks the test so that it will not be run, and gets or sets the skip reason
+        /// </summary>
+        public override string Skip
+        {
+            get
+            {
+                if (_skip != null)
+                    return _skip;
+
+                var skipLocal = Environment.GetEnvironmentVariable(EnvironmentVariableName)?
+                    .ToLowerInvariant();
+                return skipLocal is "true" ? SkipLocal ?? "Local facts are being skipped" : null;
+            }
+            set => _skip = value;
+        }
+
+        /// <summary>
+        /// The reason why this unit test is being skipped by the <see cref="LocalTheoryAttribute"/>.
+        /// Note that the original <see cref="Skip"/> property takes precedence over this message. 
+        /// </summary>
+        public string SkipLocal { get; set; }
+
+    }
+}

src/contrib/testkits/Akka.TestKit.Xunit2/Attributes/WindowsFactAttribute.cs

@@ -0,0 +1,53 @@
+// -----------------------------------------------------------------------
+//  <copyright file="WindowsFactAttribute.cs" company="Akka.NET Project">
+//      Copyright (C) 2009-2022 Lightbend Inc. <http://www.lightbend.com>
+//      Copyright (C) 2013-2022 .NET Foundation <https://github.com/akkadotnet/akka.net>
+//  </copyright>
+// -----------------------------------------------------------------------
+
+using System;
+using Xunit;
+
+namespace Akka.TestKit.Xunit2.Attributes
+{
+    /// <summary>
+    /// <para>
+    /// This custom XUnit Fact attribute will skip unit tests if the run-time environment is not windows
+    /// </para>
+    /// <para>
+    /// Note that the original <see cref="Skip"/> property takes precedence over this attribute,
+    /// any unit tests with <see cref="WindowsFactAttribute"/> with its <see cref="Skip"/> property
+    /// set will always be skipped, regardless of the environment variable content.
+    /// </para>
+    /// </summary>
+    public class WindowsFactAttribute : FactAttribute
+    {
+        private string _skip;
+
+        /// <summary>
+        /// Marks the test so that it will not be run, and gets or sets the skip reason
+        /// </summary>
+        public override string Skip
+        {
+            get
+            {
+                if (_skip != null)
+                    return _skip;
+
+                var platform = Environment.OSVersion.Platform;
+                var notWindows = platform == PlatformID.MacOSX 
+                                 || platform == PlatformID.Unix 
+                                 || platform == PlatformID.Xbox;
+                return notWindows ? SkipUnix ?? "Skipped under Unix platforms" : null;
+            }
+            set => _skip = value;
+        }
+
+        /// <summary>
+        /// The reason why this unit test is being skipped by the <see cref="WindowsFactAttribute"/>.
+        /// Note that the original <see cref="Skip"/> property takes precedence over this message. 
+        /// </summary>
+        public string SkipUnix { get; set; }
+    }
+}
+

src/core/Akka.Cluster.Tests/DowningProviderSpec.cs

@@ -10,6 +10,7 @@
 using Akka.Actor;
 using Akka.Configuration;
 using Akka.TestKit;
+using Akka.TestKit.Xunit2.Attributes;
 using Akka.Util;
 using FluentAssertions;
 using Xunit;
@@ -101,7 +102,7 @@ public void Downing_provider_should_use_specified_downing_provider()
             }
         }
 
-        [Fact(Skip = "Racy")]
+        [LocalFact(SkipLocal = "Racy on Azure DevOps")]
         public void Downing_provider_should_stop_the_cluster_if_the_downing_provider_throws_exception_in_props()
         {
             var config = ConfigurationFactory.ParseString(

src/core/Akka.Docs.Tests/Streams/KillSwitchDocTests.cs

@@ -66,7 +66,7 @@ public void Unique_kill_switch_must_control_graph_completion_with_abort()
             var error = new Exception("boom");
             killSwitch.Abort(error);
 
-            last.ContinueWith(t => { /* Ignore exception */ }).Wait(1.Seconds());
+            AwaitCondition(() => last.IsFaulted);
             last.Exception.GetBaseException().Should().Be(error);
             #endregion
         }
@@ -111,10 +111,10 @@ public void Shared_kill_switch_must_control_graph_completion_with_abort()
             var error = new Exception("boom");
             sharedKillSwitch.Abort(error);
 
-            last1.ContinueWith(t => { /* Ignore exception */ }).Wait(1.Seconds());
+            AwaitCondition(() => last1.IsFaulted);
             last1.Exception.GetBaseException().Should().Be(error);
 
-            last2.ContinueWith(t => { /* Ignore exception */ }).Wait(1.Seconds());
+            AwaitCondition(() => last2.IsFaulted);
             last2.Exception.GetBaseException().Should().Be(error);
             #endregion
         }

src/core/Akka.Persistence.Tests/AtLeastOnceDeliveryCrashSpec.cs

@@ -9,6 +9,7 @@
 using Akka.Actor;
 using Akka.Event;
 using Akka.TestKit;
+using Akka.TestKit.Xunit2.Attributes;
 using Xunit;
 using Xunit.Abstractions;
 
@@ -122,7 +123,7 @@ public AtLeastOnceDeliveryCrashSpec(ITestOutputHelper output)
         {
         }
 
-        [Fact(Skip = "Racy on AzureDevOps")]
+        [LocalFact(SkipLocal = "Racy on Azure DevOps")]
         public void AtLeastOnceDelivery_should_not_send_when_actor_crashes()
         {
             var testProbe = CreateTestProbe();

src/core/Akka.Persistence.Tests/AtLeastOnceDeliveryReceiveActorSpec.cs

@@ -11,6 +11,7 @@
 using Akka.Actor;
 using Akka.Event;
 using Akka.TestKit;
+using Akka.TestKit.Xunit2.Attributes;
 using Xunit;
 
 namespace Akka.Persistence.Tests
@@ -646,7 +647,7 @@ public void PersistentReceive_must_redeliver_many_lost_messages()
             resCarr.Except(c).Any().ShouldBeFalse();
         }
 
-        [Fact(Skip = "Racy on Azure DevOps")]
+        [LocalFact(SkipLocal = "Racy on Azure DevOps")]
         public void PersistentReceive_must_limit_the_number_of_messages_redelivered_at_once()
         {
             var probe = CreateTestProbe();

src/core/Akka.Persistence.Tests/AtLeastOnceDeliverySpec.cs

@@ -12,6 +12,7 @@
 using Akka.Actor.Dsl;
 using Akka.Event;
 using Akka.TestKit;
+using Akka.TestKit.Xunit2.Attributes;
 using Xunit;
 using Xunit.Abstractions;
 
@@ -465,7 +466,7 @@ public void AtLeastOnceDelivery_must_redeliver_lost_messages_after_restart()
             probe.ExpectNoMsg(TimeSpan.FromSeconds(1));
         }
 
-        [Fact(Skip = "Racy")]
+        [LocalFact(SkipLocal = "Racy on Azure DevOps")]
         public void AtLeastOnceDelivery_must_resend_replayed_deliveries_with_an_initially_in_order_strategy_before_delivering_fresh_messages()
         {
             var probe = CreateTestProbe();
@@ -621,7 +622,7 @@ public void AtLeastOnceDelivery_must_redeliver_many_lost_messages()
             resCarr.Except(c).Any().ShouldBeFalse();
         }
 
-        [Fact(Skip = "Racy on Azure DevOps")]
+        [LocalFact(SkipLocal = "Racy on Azure DevOps")]
         public void AtLeastOnceDelivery_must_limit_the_number_of_messages_redelivered_at_once()
         {
             var probe = CreateTestProbe();

src/core/Akka.Remote.Tests/RemotingSpec.cs

@@ -25,6 +25,7 @@
 using ThreadLocalRandom = Akka.Util.ThreadLocalRandom;
 using Akka.Remote.Serialization;
 using Akka.TestKit.Extensions;
+using Akka.TestKit.Xunit2.Attributes;
 using FluentAssertions.Extensions;
 
 namespace Akka.Remote.Tests
@@ -183,8 +184,7 @@ public async Task Remoting_must_support_Ask()
             Assert.IsType<FutureActorRef<(string, IActorRef)>>(actorRef);
         }
 
-        // Previously racy test
-        [Fact]
+        [LocalFact(SkipLocal = "Racy in AzDo CI/CD")]
         public async Task Ask_does_not_deadlock()
         {
             // see https://github.com/akkadotnet/akka.net/issues/2546
@@ -247,8 +247,7 @@ public async Task Remoting_must_not_send_remote_recreated_actor_with_same_name()
             await ExpectMsgAsync(76);
         }
 
-        // Previously racy test
-        [Fact]
+        [LocalFact(SkipLocal = "Racy in AzDo CI/CD")]
         public async Task Remoting_must_lookup_actors_across_node_boundaries()
         {
             Action<IActorDsl> act = dsl =>

src/core/Akka.Remote.Tests/Transport/DotNettySslSupportSpec.cs

@@ -11,6 +11,7 @@
 using Akka.Actor;
 using Akka.Configuration;
 using Akka.TestKit;
+using Akka.TestKit.Xunit2.Attributes;
 using Xunit;
 using Xunit.Abstractions;
 using static Akka.Util.RuntimeDetector;
@@ -140,7 +141,7 @@ public async Task Secure_transport_should_be_possible_between_systems_sharing_th
             }, TimeSpan.FromSeconds(30), TimeSpan.FromMilliseconds(100));
         }
 
-        [Fact]
+        [LocalFact(SkipLocal = "Racy in Azure AzDo CI/CD")]
         public async Task Secure_transport_should_be_possible_between_systems_using_thumbprint()
         {
             // skip this test due to linux/mono certificate issues

src/core/Akka.Streams.Tests/Actor/ActorPublisherSpec.cs

@@ -19,6 +19,7 @@
 using Akka.Streams.Implementation;
 using Akka.Streams.TestKit;
 using Akka.TestKit;
+using Akka.TestKit.Xunit2.Attributes;
 using FluentAssertions;
 using Xunit;
 using Xunit.Abstractions;
@@ -422,7 +423,7 @@ public async Task ActorPublisher_should_work_in_a_GraphDsl()
             }, materializer);
         }
 
-        [Fact(Skip = "Racy")]
+        [LocalFact(SkipLocal = "Racy on Azure DevOps")]
         public async Task ActorPublisher_should_be_able_to_define_a_subscription_timeout_after_which_it_should_shut_down()
         {
             var materializer = Sys.Materializer();