Add Repeat attribute (#1375)

src/Logging/Logging.Testing/src/LoggedTest/LoggedTestBase.cs

@@ -26,7 +26,7 @@ namespace Microsoft.Extensions.Logging.Testing
         // Internal for testing
         internal string ResolvedTestClassName { get; set; }
 
-        internal RetryContext RetryContext { get; set; }
+        internal RepeatContext RepeatContext { get; set; }
 
         public string ResolvedLogOutputDirectory { get; set; }
 

src/Logging/Logging.Testing/src/RepeatContext.cs

@@ -3,14 +3,10 @@
 
 namespace Microsoft.Extensions.Logging.Testing
 {
-    public class RetryContext
+    public class RepeatContext
     {
         internal int Limit { get; set; }
 
-        internal object TestClassInstance { get; set; }
-
-        internal string Reason { get; set; }
-
         internal int CurrentIteration { get; set; }
     }
 }

src/Logging/Logging.Testing/src/Xunit/LoggedTestInvoker.cs

@@ -16,7 +16,7 @@ namespace Microsoft.Extensions.Logging.Testing
     public class LoggedTestInvoker : XunitTestInvoker
     {
         private readonly ITestOutputHelper _output;
-        private readonly RetryContext _retryContext;
+        private readonly RepeatContext _repeatContext; 
         private readonly bool _collectDumpOnFailure;
 
         public LoggedTestInvoker(
@@ -30,12 +30,12 @@ namespace Microsoft.Extensions.Logging.Testing
             ExceptionAggregator aggregator,
             CancellationTokenSource cancellationTokenSource,
             ITestOutputHelper output,
-            RetryContext retryContext,
+            RepeatContext repeatContext,
             bool collectDumpOnFailure)
             : base(test, messageBus, testClass, constructorArguments, testMethod, testMethodArguments, beforeAfterAttributes, aggregator, cancellationTokenSource)
         {
             _output = output;
-            _retryContext = retryContext;
+            _repeatContext = repeatContext;
             _collectDumpOnFailure = collectDumpOnFailure;
         }
 
@@ -51,19 +51,7 @@ namespace Microsoft.Extensions.Logging.Testing
             if (testClass is LoggedTestBase loggedTestBase)
             {
                 // Used for testing
-                loggedTestBase.RetryContext = _retryContext;
+                loggedTestBase.RepeatContext = _repeatContext;
-
-                if (_retryContext != null)
-                {
-                    // Log retry attempt as warning
-                    if (_retryContext.CurrentIteration > 0)
-                    {
-                        loggedTestBase.Logger.LogWarning($"{TestMethod.Name} failed and retry conditions are met, re-executing. The reason for failure is {_retryContext.Reason}.");
-                    }
-
-                    // Save the test class instance for non-static predicates
-                    _retryContext.TestClassInstance = testClass;
-                }
             }
 
             return testClass;

src/Logging/Logging.Testing/src/Xunit/LoggedTestRunner.cs

@@ -49,82 +49,66 @@ namespace Microsoft.Extensions.Logging.Testing
 
         private async Task<decimal> InvokeTestMethodAsync(ExceptionAggregator aggregator, ITestOutputHelper output)
         {
-            var retryAttribute = GetRetryAttribute(TestMethod);
             var collectDump = TestMethod.GetCustomAttribute<CollectDumpAttribute>() != null;
-
+            var repeatAttribute = GetRepeatAttribute(TestMethod);
-            if (!typeof(LoggedTestBase).IsAssignableFrom(TestClass) || retryAttribute == null)
+            
+            if (!typeof(LoggedTestBase).IsAssignableFrom(TestClass) || repeatAttribute == null)
             {
                 return await new LoggedTestInvoker(Test, MessageBus, TestClass, ConstructorArguments, TestMethod, TestMethodArguments, BeforeAfterAttributes, aggregator, CancellationTokenSource, output, null, collectDump).RunAsync();
             }
 
-            var retryPredicateMethodName = retryAttribute.RetryPredicateName;
+            return await RunRepeatTestInvoker(aggregator, output, collectDump, repeatAttribute);
-            var retryPredicateMethod = TestClass.GetMethod(retryPredicateMethodName,
-                BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.Static,
-                null,
-                new Type[] { typeof(Exception) },
-                null)
-                ?? throw new InvalidOperationException($"No valid static retry predicate method {retryPredicateMethodName} was found on the type {TestClass.FullName}.");
-
-            if (retryPredicateMethod.ReturnType != typeof(bool))
-            {
-                throw new InvalidOperationException($"Retry predicate method {retryPredicateMethodName} on {TestClass.FullName} does not return bool.");
-            }
-
-            var retryContext = new RetryContext()
-            {
-                Limit = retryAttribute.RetryLimit,
-                Reason = retryAttribute.RetryReason,
-            };
-
-            var retryAggregator = new ExceptionAggregator();
-            var loggedTestInvoker = new LoggedTestInvoker(Test, MessageBus, TestClass, ConstructorArguments, TestMethod, TestMethodArguments, BeforeAfterAttributes, retryAggregator, CancellationTokenSource, output, retryContext, collectDump);
-            var totalTime = 0.0M;
-
-            do
-            {
-                retryAggregator.Clear();
-                totalTime += await loggedTestInvoker.RunAsync();
-                retryContext.CurrentIteration++;
-            }
-            while (retryAggregator.HasExceptions
-                && retryContext.CurrentIteration < retryContext.Limit
-                && (retryPredicateMethod.IsStatic
-                    ? (bool)retryPredicateMethod.Invoke(null, new object[] { retryAggregator.ToException() })
-                    : (bool)retryPredicateMethod.Invoke(retryContext.TestClassInstance, new object[] { retryAggregator.ToException() }))
-                );
-
-            aggregator.Aggregate(retryAggregator);
-            return totalTime;
         }
 
-        private RetryTestAttribute GetRetryAttribute(MethodInfo methodInfo)
+        private async Task<decimal> RunRepeatTestInvoker(ExceptionAggregator aggregator, ITestOutputHelper output, bool collectDump, RepeatAttribute repeatAttribute)
         {
-            var os = RuntimeInformation.IsOSPlatform(OSPlatform.OSX) ? OperatingSystems.MacOSX
+            var repeatContext = new RepeatContext
-                : RuntimeInformation.IsOSPlatform(OSPlatform.Windows) ? OperatingSystems.Windows
+            {
-                : OperatingSystems.Linux;
+                Limit = repeatAttribute.RunCount
+            };
 
-            var attributeCandidate = methodInfo.GetCustomAttribute<RetryTestAttribute>();
+            var timeTaken = 0.0M;
+            var testLogger = new LoggedTestInvoker(
+                Test,
+                MessageBus,
+                TestClass,
+                ConstructorArguments,
+                TestMethod,
+                TestMethodArguments,
+                BeforeAfterAttributes,
+                aggregator,
+                CancellationTokenSource,
+                output,
+                repeatContext,
+                collectDump);
 
-            if (attributeCandidate != null && (attributeCandidate.OperatingSystems & os) != 0)
+            for (repeatContext.CurrentIteration = 0; repeatContext.CurrentIteration < repeatContext.Limit; repeatContext.CurrentIteration++)
+            {
+                timeTaken = await testLogger.RunAsync();
+                if (aggregator.HasExceptions)
+                {
+                    return timeTaken;
+                }
+            }
+
+            return timeTaken;
+        }
+
+        private RepeatAttribute GetRepeatAttribute(MethodInfo methodInfo)
+        {
+            var attributeCandidate = methodInfo.GetCustomAttribute<RepeatAttribute>();
+            if (attributeCandidate != null)
             {
                 return attributeCandidate;
             }
 
-            attributeCandidate = methodInfo.DeclaringType.GetCustomAttribute<RetryTestAttribute>();
+            attributeCandidate = methodInfo.DeclaringType.GetCustomAttribute<RepeatAttribute>();
-
+            if (attributeCandidate != null)
-            if (attributeCandidate != null && (attributeCandidate.OperatingSystems & os) != 0)
             {
                 return attributeCandidate;
             }
 
-            attributeCandidate = methodInfo.DeclaringType.Assembly.GetCustomAttribute<RetryTestAttribute>();
+            return methodInfo.DeclaringType.Assembly.GetCustomAttribute<RepeatAttribute>();
-
-            if (attributeCandidate != null && (attributeCandidate.OperatingSystems & os) != 0)
-            {
-                return attributeCandidate;
-            }
-
-            return null;
         }
     }
 }

src/Logging/Logging.Testing/src/Xunit/RepeatAttribute.cs

@@ -0,0 +1,28 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+using System.ComponentModel;
+
+namespace Microsoft.Extensions.Logging.Testing
+{
+    /// <summary>
+    /// Runs a test multiple times to stress flaky tests that are believed to be fixed.
+    /// This can be used on an assembly, class, or method name.
+    /// Requires using <see cref="LoggedTest"/> to run.
+    /// </summary>
+    [EditorBrowsable(EditorBrowsableState.Never)]
+    [AttributeUsage(AttributeTargets.Method | AttributeTargets.Class | AttributeTargets.Assembly, AllowMultiple = false)]
+    public class RepeatAttribute : Attribute
+    {
+        public RepeatAttribute(int runCount = 10)
+        {
+            RunCount = runCount;
+        }
+
+        /// <summary>
+        /// The number of times to run a test.
+        /// </summary>
+        public int RunCount { get; }
+    }
+}

src/Logging/Logging.Testing/src/Xunit/RetryTestAttribute.cs

@@ -1,68 +0,0 @@
-// Copyright (c) .NET Foundation. All rights reserved.
-// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
-
-using System;
-using System.ComponentModel;
-using Microsoft.AspNetCore.Testing.xunit;
-
-namespace Microsoft.Extensions.Logging.Testing
-{
-    /// <summary>
-    /// WARNING: This attribute should only be used on well understood flaky test failures caused by external issues and should be removed once the underlying issues have been resolved.
-    /// This is not intended to be a long term solution to ensure passing of flaky tests but instead a method to improve test reliability without reducing coverage.
-    /// Issues should be filed to remove these attributes from affected tests as soon as the underlying issue is fixed.
-    /// </summary>
-    [EditorBrowsable(EditorBrowsableState.Never)]
-    [AttributeUsage(AttributeTargets.Method | AttributeTargets.Class, AllowMultiple = false)]
-    public class RetryTestAttribute : Attribute
-    {
-        /// <summary>
-        /// WARNING: This attribute should only be used on well understood flaky test failures caused by external issues and should be removed once the underlying issues have been resolved.
-        /// This is not intended to be a long term solution to ensure passing of flaky tests but instead a method to improve test reliability without reducing coverage.
-        /// Issues should be filed to remove these attributes from affected tests as soon as the underlying issue is fixed.
-        /// </summary>
-        /// <param name="retryPredicateName">The predicate of the format Func&lt;Exception,bool&gt; that is used to determine if the test should be retried</param>
-        /// <param name="retryReason">The reason for retrying the test</param>
-        /// <param name="retryLimit">The number of retries to attempt before failing the test, for most purposes this this should be kept at 2 to avoid excessive retries.</param>
-        public RetryTestAttribute(string retryPredicateName, string retryReason, int retryLimit = 2)
-            : this(retryPredicateName, retryReason, OperatingSystems.Linux | OperatingSystems.MacOSX | OperatingSystems.Windows, retryLimit) { }
-
-        /// <summary>
-        /// WARNING: This attribute should only be used on well understood flaky test failures caused by external issuesand should be removed once the underlying issues have been resolved.
-        /// This is not intended to be a long term solution to ensure passing of flaky tests but instead a method to improve test reliability without reducing coverage.
-        /// Issues should be filed to remove these attributes from affected tests as soon as the underlying issue is fixed.
-        /// </summary>
-        /// <param name="operatingSystems">The os(es) this retry should be attempted on.</param>
-        /// <param name="retryPredicateName">The predicate of the format Func&lt;Exception,bool&gt; that is used to determine if the test should be retried</param>
-        /// <param name="retryReason">The reason for retrying the test</param>
-        /// <param name="retryLimit">The number of retries to attempt before failing the test, for most purposes this this should be kept at 2 to avoid excessive retries.</param>
-        public RetryTestAttribute(string retryPredicateName, string retryReason, OperatingSystems operatingSystems, int retryLimit = 2)
-        {
-            if (string.IsNullOrEmpty(retryPredicateName))
-            {
-                throw new ArgumentNullException(nameof(RetryPredicateName), "A valid non-empty predicate method name must be provided.");
-            }
-            if (string.IsNullOrEmpty(retryReason))
-            {
-                throw new ArgumentNullException(nameof(retryReason), "A valid non-empty reason for retrying the test must be provided.");
-            }
-            if (retryLimit < 1)
-            {
-                throw new ArgumentOutOfRangeException(nameof(retryLimit), retryLimit, "Retry count must be positive.");
-            }
-
-            OperatingSystems = operatingSystems;
-            RetryPredicateName = retryPredicateName;
-            RetryReason = retryReason;
-            RetryLimit = retryLimit;
-        }
-
-        public string RetryPredicateName { get; }
-
-        public string RetryReason { get; }
-
-        public int RetryLimit { get; }
-
-        public OperatingSystems OperatingSystems { get; }
-    }
-}

src/Logging/Logging.Testing/test/LoggedTestXunitRepeatTests.cs

@@ -0,0 +1,43 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using Xunit;
+
+namespace Microsoft.Extensions.Logging.Testing.Tests
+{
+    [Repeat]
+    public class LoggedTestXunitRepeatTests : LoggedTest
+    {
+        public static int _runCount = 0;
+
+        [Fact]
+        [Repeat(5)]
+        public void RepeatLimitIsSetCorrectly()
+        {
+            Assert.Equal(5, RepeatContext.Limit);
+        }
+
+        [Fact]
+        [Repeat(5)]
+        public void RepeatRunsTestSpecifiedNumberOfTimes()
+        {
+            Assert.Equal(RepeatContext.CurrentIteration, _runCount);
+            _runCount++;
+        }
+
+        [Fact]
+        public void RepeatCanBeSetOnClass()
+        {
+            Assert.Equal(10, RepeatContext.Limit);
+        }
+    }
+
+    public class LoggedTestXunitRepeatAssemblyTests : LoggedTest
+    {
+        [Fact]
+        public void RepeatCanBeSetOnAssembly()
+        {
+            Assert.Equal(1, RepeatContext.Limit);
+        }
+    }
+}

src/Logging/Logging.Testing/test/LoggedTestXunitRetryTests.cs

@@ -1,101 +0,0 @@
-// Copyright (c) .NET Foundation. All rights reserved.
-// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
-
-using System;
-using System.Linq;
-using Microsoft.AspNetCore.Testing.xunit;
-using Xunit;
-
-namespace Microsoft.Extensions.Logging.Testing.Tests
-{
-    [RetryTest(nameof(RetryAllPredicate), "sample reason")]
-    public class LoggedTestXunitRetryTests : LoggedTest
-    {
-        [Fact]
-        public void CompletesWithoutRetryOnSuccess()
-        {
-            Assert.Equal(2, RetryContext.Limit);
-
-            // This assert would fail on the second run
-            Assert.Equal(0, RetryContext.CurrentIteration);
-        }
-
-        [Fact]
-        public void RetriesUntilSuccess()
-        {
-            // This assert will fail the first time but pass on the second
-            Assert.Equal(1, RetryContext.CurrentIteration);
-
-            // This assert will ensure a message is logged for retried tests.
-            Assert.Equal(1, TestSink.Writes.Count);
-            var loggedMessage = TestSink.Writes.ToArray()[0];
-            Assert.Equal(LogLevel.Warning, loggedMessage.LogLevel);
-            Assert.Equal($"{nameof(RetriesUntilSuccess)} failed and retry conditions are met, re-executing. The reason for failure is sample reason.", loggedMessage.Message);
-        }
-
-        [ConditionalFact]
-        [OSSkipCondition(OperatingSystems.Windows)]
-        [RetryTest(nameof(RetryAllPredicate), "sample reason", OperatingSystems.Windows, 3)]
-        public void RetryCountNotOverridenWhenOSDoesNotMatch()
-        {
-            Assert.Equal(2, RetryContext.Limit);
-        }
-
-        [ConditionalFact]
-        [OSSkipCondition(OperatingSystems.Linux | OperatingSystems.MacOSX)]
-        [RetryTest(nameof(RetryAllPredicate), "sample reason", OperatingSystems.Windows, 3)]
-        public void RetryCountOverridenWhenOSMatches()
-        {
-            Assert.Equal(3, RetryContext.Limit);
-        }
-
-        [Fact]
-        [RetryTest(nameof(RetryInvalidOperationExceptionPredicate), "sample reason")]
-        public void RetryIfPredicateIsTrue()
-        {
-            if (RetryContext.CurrentIteration == 0)
-            {
-                Logger.LogWarning("Throw on first iteration");
-                throw new Exception();
-            }
-
-            // This assert will ensure a message is logged for retried tests.
-            Assert.Equal(1, TestSink.Writes.Count);
-            var loggedMessage = TestSink.Writes.ToArray()[0];
-            Assert.Equal(LogLevel.Warning, loggedMessage.LogLevel);
-            Assert.Equal($"{nameof(RetryIfPredicateIsTrue)} failed and retry conditions are met, re-executing. The reason for failure is sample reason.", loggedMessage.Message);
-        }
-
-        // Static predicates are valid
-        private static bool RetryAllPredicate(Exception e)
-            => true;
-
-        // Instance predicates are valid
-        private bool RetryInvalidOperationExceptionPredicate(Exception e)
-            => TestSink.Writes.Any(m => m.Message.Contains("Throw on first iteration"));
-    }
-
-    [RetryTest(nameof(RetryAllPredicate), "sample reason")]
-    public class LoggedTestXunitRetryConstructorTest : LoggedTest
-    {
-        private static int _constructorInvocationCount;
-
-        public LoggedTestXunitRetryConstructorTest()
-        {
-            _constructorInvocationCount++;
-        }
-
-        [Fact]
-        public void RetriesUntilSuccess()
-        {
-            // The constructor is invoked before the test method but the current iteration is updated after
-            Assert.Equal(_constructorInvocationCount, RetryContext.CurrentIteration + 1);
-
-            // This assert will fail the first time but pass on the second
-            Assert.Equal(1, RetryContext.CurrentIteration);
-        }
-
-        private static bool RetryAllPredicate(Exception e)
-            => true;
-    }
-}

src/Logging/Logging.Testing/test/LoggedTestXunitTests.cs

@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation. All rights reserved.
+// Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
 using System.Linq;

src/Logging/Logging.Testing/test/Properties/AssemblyInfo.cs

@@ -2,3 +2,4 @@ using Microsoft.Extensions.Logging;
 using Microsoft.Extensions.Logging.Testing;
 
 [assembly: LogLevel(LogLevel.Trace)]
+[assembly: Repeat(1)]