Add ITheoryDataRow.Timeout and DataAttribute.Timeout (and more code r…

…estructuring)

src/common.tests/CultureAwareTesting/CulturedTheoryAttributeDiscoverer.cs

@@ -13,14 +13,12 @@ public class CulturedTheoryAttributeDiscoverer : TheoryDiscoverer
 		_ITestFrameworkDiscoveryOptions discoveryOptions,
 		_ITestMethod testMethod,
 		_IAttributeInfo theoryAttribute,
-		_IAttributeInfo dataAttribute,
 		ITheoryDataRow dataRow,
 		object?[] testMethodArguments)
 	{
 		var cultures = GetCultures(theoryAttribute);
-		var dataAttributeDisplayName = dataAttribute.GetNamedArgument<string>(nameof(DataAttribute.TestDisplayName));
-		var details = TestIntrospectionHelper.GetTestCaseDetails(discoveryOptions, testMethod, theoryAttribute, dataRow, testMethodArguments, baseDisplayName: dataAttributeDisplayName);
-		var traits = TestIntrospectionHelper.GetTraits(testMethod, dataAttribute, dataRow);
+		var details = TestIntrospectionHelper.GetTestCaseDetailsForTheoryDataRow(discoveryOptions, testMethod, theoryAttribute, dataRow, testMethodArguments);
+		var traits = TestIntrospectionHelper.GetTraits(testMethod, dataRow);
 
 		var result = cultures.Select(
 			// TODO: How do we get source information in here?

src/xunit.v3.common/v3/Messages/_TestStarting.cs

@@ -22,6 +22,9 @@ public string TestDisplayName
 		set => testDisplayName = Guard.ArgumentNotNullOrEmpty(value, nameof(TestDisplayName));
 	}
 
+	/// <inheritdoc/>
+	public int Timeout { get; set; }
+
 	/// <inheritdoc/>
 	public IReadOnlyDictionary<string, IReadOnlyList<string>> Traits
 	{

src/xunit.v3.common/v3/Metadata/_ITestMetadata.cs

@@ -17,6 +17,17 @@ public interface _ITestMetadata
 	/// </summary>
 	string TestDisplayName { get; }
 
+	/// <summary>
+	/// A value greater than zero marks the test as having a timeout, and gets or sets the
+	/// timeout (in milliseconds).
+	/// </summary>
+	/// <remarks>
+	/// WARNING: Using this with parallelization turned on will result in undefined behavior.
+	/// Timeout is only supported when parallelization is disabled, either globally or with
+	/// a parallelization-disabled test collection.
+	/// </remarks>
+	int Timeout { get; }
+
 	/// <summary>
 	/// Gets the trait values associated with this test case. If
 	/// there are none, or the framework does not support traits,

src/xunit.v3.core.tests/Acceptance/Xunit3TheoryAcceptanceTests.cs

@@ -1,6 +1,7 @@
 using System;
 using System.Collections;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.Linq;
 using System.Reflection;
 using System.Runtime.InteropServices;
@@ -868,6 +869,56 @@ public void TestMethod(int x)
 		}
 	}
 
+	[Collection("Timeout Tests")]
+	public class DataAttributeTimeoutTests : AcceptanceTestV3
+	{
+		[Theory]
+		[InlineData(true)]
+		[InlineData(false)]
+		public async void TimeoutAcceptanceTest(bool preEnumerateTheories)
+		{
+			var stopwatch = Stopwatch.StartNew();
+			var results = await RunForResultsAsync(typeof(ClassUnderTest), preEnumerateTheories);
+			stopwatch.Stop();
+
+			Assert.Collection(
+				results.OfType<TestPassedWithDisplayName>().OrderBy(x => x.TestDisplayName),
+				passed => Assert.Equal($"{typeof(ClassUnderTest).FullName}.{nameof(ClassUnderTest.LongRunningTask)}(delay: 10)", passed.TestDisplayName),
+				passed => Assert.Equal($"{typeof(ClassUnderTest).FullName}.{nameof(ClassUnderTest.LongRunningTask)}(delay: 100)", passed.TestDisplayName)
+			);
+			Assert.Collection(
+				results.OfType<TestFailedWithDisplayName>().OrderBy(f => f.TestDisplayName),
+				failed =>
+				{
+					Assert.Equal($"{typeof(ClassUnderTest).FullName}.{nameof(ClassUnderTest.LongRunningTask)}(delay: 10000)", failed.TestDisplayName);
+					Assert.Equal("Test execution timed out after 42 milliseconds", failed.Messages.Single());
+				},
+				failed =>
+				{
+					Assert.Equal($"{typeof(ClassUnderTest).FullName}.{nameof(ClassUnderTest.LongRunningTask)}(delay: 11000)", failed.TestDisplayName);
+					Assert.Equal("Test execution timed out after 10 milliseconds", failed.Messages.Single());
+				}
+			);
+
+			Assert.True(stopwatch.ElapsedMilliseconds < 10000, "Elapsed time should be less than 10 seconds");
+		}
+
+		class ClassUnderTest
+		{
+			public static List<TheoryDataRow> MemberDataSource = new()
+			{
+				new TheoryDataRow(11000),
+				new TheoryDataRow(100) { Timeout = 10000 },
+			};
+
+			[Theory(Timeout = 42)]
+			[InlineData(10000)]
+			[InlineData(10, Timeout = 10000)]
+			[MemberData(nameof(MemberDataSource), Timeout = 10)]
+			public Task LongRunningTask(int delay) => Task.Delay(delay);
+		}
+	}
+
 	public class InlineDataTests : AcceptanceTestV3
 	{
 		[Fact]

src/xunit.v3.core/FactAttribute.cs

@@ -35,9 +35,11 @@ public class FactAttribute : Attribute
 	/// <summary>
 	/// A value greater than zero marks the test as having a timeout, and gets or sets the
 	/// timeout (in milliseconds).
+	/// </summary>
+	/// <remarks>
 	/// WARNING: Using this with parallelization turned on will result in undefined behavior.
 	/// Timeout is only supported when parallelization is disabled, either globally or with
 	/// a parallelization-disabled test collection.
-	/// </summary>
+	/// </remarks>
 	public virtual int Timeout { get; set; }
 }

src/xunit.v3.core/ITheoryDataRow.cs

@@ -29,6 +29,18 @@ public interface ITheoryDataRow
 	/// </summary>
 	string? TestDisplayName { get; }
 
+	/// <summary>
+	/// A value greater than zero marks the test as having a timeout, and gets or sets the
+	/// timeout (in milliseconds). A non-<c>null</c> value here overrides any inherited value
+	/// from the <see cref="DataAttribute"/> or the <see cref="TheoryAttribute"/>.
+	/// </summary>
+	/// <remarks>
+	/// WARNING: Using this with parallelization turned on will result in undefined behavior.
+	/// Timeout is only supported when parallelization is disabled, either globally or with
+	/// a parallelization-disabled test collection.
+	/// </remarks>
+	int? Timeout { get; }
+
 	/// <summary>
 	/// Gets the trait values associated with this theory data row. If there are none, you may either
 	/// return a <c>null</c> or empty dictionary.

src/xunit.v3.core/Internal/XunitRunnerHelper.cs

@@ -51,6 +51,7 @@ public static class XunitRunnerHelper
 				TestDisplayName = testCase.TestCaseDisplayName,
 				TestMethodUniqueID = testCase.TestMethod?.UniqueID,
 				TestUniqueID = testUniqueID,
+				Timeout = 0,
 				Traits = testCase.Traits,
 			};
 			messageBus.QueueMessage(testStarting);

src/xunit.v3.core/Sdk/DataAttribute.cs

@@ -16,7 +16,6 @@ namespace Xunit.Sdk;
 [AttributeUsage(AttributeTargets.Method, AllowMultiple = true, Inherited = true)]
 public abstract class DataAttribute : Attribute
 {
-	static readonly Dictionary<string, List<string>> emptyTraits = new();
 	static readonly MethodInfo? tupleIndexerGetter;
 	static readonly MethodInfo? tupleLengthGetter;
 	static readonly Type? tupleType;
@@ -77,6 +76,34 @@ public bool Explicit
 	/// </summary>
 	public string? TestDisplayName { get; set; }
 
+	/// <summary>
+	/// A value greater than zero marks the test as having a timeout, and gets or sets the
+	/// timeout (in milliseconds). Setting a value here overrides any inherited value
+	/// from the <see cref="TheoryAttribute"/>.
+	/// </summary>
+	/// <remarks>
+	/// WARNING: Using this with parallelization turned on will result in undefined behavior.
+	/// Timeout is only supported when parallelization is disabled, either globally or with
+	/// a parallelization-disabled test collection.
+	/// </remarks>
+	public int Timeout
+	{
+		get => TimeoutWithoutDefaultValue ?? 0;
+		set => TimeoutWithoutDefaultValue = value;
+	}
+
+	/// <summary>
+	/// Gets the value for <see cref="Timeout"/>, except that it keeps track of whether it has been set
+	/// or not, and returns <c>null</c> if it hasn't been set.
+	/// </summary>
+	/// <remarks>
+	/// Since attribute initializers cannot accept nullable integer values, this secondary attribute value
+	/// (that cannot be externally set) is required to keep track of whether <see cref="Timeout"/> has been
+	/// set by the user or not. At reflection time, we can peer into the <see cref="CustomAttributeData"/>,
+	/// but once the attribute instance has been created, this is the only way to know for sure.
+	/// </remarks>
+	protected int? TimeoutWithoutDefaultValue { get; set; }
+
 	/// <summary>
 	/// Gets or sets a set of traits for the associated data. The data is pushed as an array of
 	/// string that are key/value pairs (f.e., <c>new[] { "key1", "value1", "key2", "value2" }</c>).
@@ -114,19 +141,36 @@ public bool Explicit
 		Guard.ArgumentNotNull(dataRow);
 
 		if (dataRow is ITheoryDataRow theoryDataRow)
+		{
+			var dataRowTraits = new Dictionary<string, List<string>>(StringComparer.OrdinalIgnoreCase);
+
+			if (theoryDataRow.Traits != null)
+				foreach (var kvp in theoryDataRow.Traits)
+					dataRowTraits.GetOrAdd(kvp.Key).AddRange(kvp.Value);
+
+			MergeTraitsInto(dataRowTraits);
+
 			return new TheoryDataRow(theoryDataRow.GetData())
 			{
 				Explicit = theoryDataRow.Explicit ?? ExplicitWithoutDefaultValue,
 				Skip = theoryDataRow.Skip ?? Skip,
-				TestDisplayName = theoryDataRow.TestDisplayName,
-				Traits = theoryDataRow.Traits ?? emptyTraits,
+				TestDisplayName = theoryDataRow.TestDisplayName ?? TestDisplayName,
+				Timeout = theoryDataRow.Timeout ?? TimeoutWithoutDefaultValue,
+				Traits = dataRowTraits,
 			};
+		}
+
+		var traits = new Dictionary<string, List<string>>(StringComparer.OrdinalIgnoreCase);
+		MergeTraitsInto(traits);
 
 		if (dataRow is object?[] array)
 			return new TheoryDataRow(array)
 			{
 				Explicit = ExplicitWithoutDefaultValue,
 				Skip = Skip,
+				TestDisplayName = TestDisplayName,
+				Timeout = TimeoutWithoutDefaultValue,
+				Traits = traits,
 			};
 
 		if (tupleType != null && tupleIndexerGetter != null && tupleLengthGetter != null)
@@ -145,6 +189,9 @@ public bool Explicit
 					{
 						Explicit = ExplicitWithoutDefaultValue,
 						Skip = Skip,
+						TestDisplayName = TestDisplayName,
+						Timeout = TimeoutWithoutDefaultValue,
+						Traits = traits,
 					};
 				}
 			}
@@ -165,4 +212,7 @@ public bool Explicit
 	/// <returns>One or more rows of theory data. Each invocation of the test method
 	/// is represented by a single instance of <see cref="ITheoryDataRow"/>.</returns>
 	public abstract ValueTask<IReadOnlyCollection<ITheoryDataRow>?> GetData(MethodInfo testMethod);
+
+	void MergeTraitsInto(Dictionary<string, List<string>> traits) =>
+		TestIntrospectionHelper.MergeTraitsInto(traits, Traits);
 }

src/xunit.v3.core/Sdk/Frameworks/TestIntrospectionHelper.cs

@@ -46,6 +46,7 @@ static IReadOnlyCollection<_IAttributeInfo> GetCachedTraitAttributes(_ITypeInfo
 	/// <param name="testMethod">The test method.</param>
 	/// <param name="factAttribute">The fact attribute that decorates the test method.</param>
 	/// <param name="testMethodArguments">The optional test method arguments.</param>
+	/// <param name="timeout">The optional timeout; if not provided, will be looked up from the <paramref name="factAttribute"/>.</param>
 	/// <param name="baseDisplayName">The optional base display name for the test method.</param>
 	public static (
 		string TestCaseDisplayName,
@@ -59,6 +60,7 @@ _ITestMethod ResolvedTestMethod
 		_ITestMethod testMethod,
 		_IAttributeInfo factAttribute,
 		object?[]? testMethodArguments = null,
+		int? timeout = null,
 		string? baseDisplayName = null)
 	{
 		Guard.ArgumentNotNull(discoveryOptions);
@@ -72,7 +74,7 @@ _ITestMethod ResolvedTestMethod
 		baseDisplayName ??= factAttribute.GetNamedArgument<string?>(nameof(FactAttribute.DisplayName));
 		var factExplicit = factAttribute.GetNamedArgument<bool>(nameof(FactAttribute.Explicit));
 		var factSkipReason = factAttribute.GetNamedArgument<string?>(nameof(FactAttribute.Skip));
-		var factTimeout = factAttribute.GetNamedArgument<int>(nameof(FactAttribute.Timeout));
+		timeout ??= factAttribute.GetNamedArgument<int>(nameof(FactAttribute.Timeout));
 
 		if (baseDisplayName == null)
 		{
@@ -93,8 +95,7 @@ _ITestMethod ResolvedTestMethod
 		var testCaseDisplayName = testMethod.Method.GetDisplayNameWithArguments(baseDisplayName, testMethodArguments, methodGenericTypes);
 		var uniqueID = UniqueIDGenerator.ForTestCase(testMethod.UniqueID, methodGenericTypes, testMethodArguments);
 
-
-		return (testCaseDisplayName, factExplicit, factSkipReason, factTimeout, uniqueID, testMethod);
+		return (testCaseDisplayName, factExplicit, factSkipReason, timeout.Value, uniqueID, testMethod);
 	}
 
 	/// <summary>
@@ -107,40 +108,40 @@ _ITestMethod ResolvedTestMethod
 	/// <param name="theoryAttribute">The theory attribute that decorates the test method.</param>
 	/// <param name="dataRow">The data row for the test.</param>
 	/// <param name="testMethodArguments">The test method arguments obtained from the <paramref name="dataRow"/> after being type-resolved.</param>
-	/// <param name="baseDisplayName">The optional base display name (typically from the data attribute).</param>
 	public static (
 		string TestCaseDisplayName,
 		bool Explicit,
 		string? SkipReason,
 		int Timeout,
 		string UniqueID,
 		_ITestMethod ResolvedTestMethod
-	) GetTestCaseDetails(
+	) GetTestCaseDetailsForTheoryDataRow(
 		_ITestFrameworkDiscoveryOptions discoveryOptions,
 		_ITestMethod testMethod,
 		_IAttributeInfo theoryAttribute,
 		ITheoryDataRow dataRow,
-		object?[] testMethodArguments,
-		string? baseDisplayName = null)
+		object?[] testMethodArguments)
 	{
-		var result = GetTestCaseDetails(discoveryOptions, testMethod, theoryAttribute, testMethodArguments, dataRow.TestDisplayName ?? baseDisplayName);
-
-		if (dataRow.Skip != null)
-			result.SkipReason = dataRow.Skip;
+		var result = GetTestCaseDetails(discoveryOptions, testMethod, theoryAttribute, testMethodArguments, dataRow.Timeout, dataRow.TestDisplayName);
 
 		if (dataRow.Explicit.HasValue)
 			result.Explicit = dataRow.Explicit.Value;
 
+		if (dataRow.Skip != null)
+			result.SkipReason = dataRow.Skip;
+
 		return result;
 	}
 
 	/// <summary>
-	///
+	/// Retrieve the traits for a test method (merging in the traits from the optional data row, which is
+	/// assumed to already have traits that were merged from the data row itself and the data attribute).
 	/// </summary>
+	/// <param name="testMethod">The test method to get traits from.</param>
+	/// <param name="dataRow">The data row to get traits from.</param>
 	/// <returns>The traits dictionary</returns>
 	public static Dictionary<string, List<string>> GetTraits(
 		_ITestMethod testMethod,
-		_IAttributeInfo? dataAttribute = null,
 		ITheoryDataRow? dataRow = null)
 	{
 		Guard.ArgumentNotNull(testMethod);
@@ -192,24 +193,32 @@ _ITestMethod ResolvedTestMethod
 				result.GetOrAdd(kvp.Key).Add(kvp.Value);
 		}
 
-		// Traits from the data attribute
-		var traitsArray = dataAttribute?.GetNamedArgument<string[]>(nameof(DataAttribute.Traits));
-		if (traitsArray != null)
-		{
-			var idx = 0;
-
-			while (idx < traitsArray.Length - 1)
-			{
-				result.GetOrAdd(traitsArray[idx]).Add(traitsArray[idx + 1]);
-				idx += 2;
-			}
-		}
-
 		// Traits from the data row
 		if (dataRow?.Traits != null)
 			foreach (var kvp in dataRow.Traits)
 				result.GetOrAdd(kvp.Key).AddRange(kvp.Value);
 
 		return result;
 	}
+
+	/// <summary>
+	/// Merges string-array traits (like from <see cref="DataAttribute"/>) into an existing traits dictionary.
+	/// </summary>
+	/// <param name="traits">The existing traits dictionary.</param>
+	/// <param name="additionalTraits">The additional traits to merge.</param>
+	public static void MergeTraitsInto(
+		Dictionary<string, List<string>> traits,
+		string[]? additionalTraits)
+	{
+		if (additionalTraits == null)
+			return;
+
+		var idx = 0;
+
+		while (idx < additionalTraits.Length - 1)
+		{
+			traits.GetOrAdd(additionalTraits[idx]).Add(additionalTraits[idx + 1]);
+			idx += 2;
+		}
+	}
 }