Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -102,14 +102,30 @@ public bool DeferEnumeration
{
// Run the row's own factory first, as reflection mode does before it resolves the row's type
// arguments, so the factory's side effects happen and its own exceptions take precedence.
// The exception carries the row so the engine can track its objects for disposal (#7123).
yield return async () =>
{
await rowFactory();
throw new GenericTypeResolutionException(message);
var row = await rowFactory();
throw new GenericConstraintViolationRowException(message, row);
};
}
}

private sealed class InstanceDataAccessing(string message, IDataSourceAttribute innerDataSource)
: GenericConstraintViolationDataSource(message, innerDataSource), IAccessesInstanceData;
}

/// <summary>
/// Raised for a row of a <see cref="GenericConstraintViolationDataSource"/> that wraps another data source.
/// It carries the objects that the wrapped row's factory created, which the failed test never receives,
/// so the engine can track them for disposal. The engine reports a plain
/// <see cref="GenericTypeResolutionException"/> with the same message, as reflection mode does.
/// </summary>
internal sealed class GenericConstraintViolationRowException(string message, object?[]? row)
: GenericTypeResolutionException(message)
{
/// <summary>
/// Gets the values that the wrapped row's factory returned, before unwrapping.
/// </summary>
public object?[]? Row { get; } = row;
}
18 changes: 12 additions & 6 deletions src/TUnit.Core/Discovery/ObjectGraphDiscoverer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -182,18 +182,24 @@ public SortedList<int, HashSet<object>> DiscoverAndTrackObjects(TestContext test
var testDetails = testContext.Metadata.TestDetails;
if (testDetails.TestClassArguments.Length == 0
&& testDetails.TestMethodArguments.Length == 0
&& testDetails.TestClassInjectedPropertyArguments.Count == 0)
&& testDetails.TestClassInjectedPropertyArguments.Count == 0
&& testDetails.FailedDataRowObjects.Length == 0)
{
// No root objects, so nothing to discover — skip the traversal closures.
return visitedObjects;
}

TryAddObjectFunc tryAdd = (obj, depth) => TryAddToHashSet(visitedObjects, depth, obj);
RootObjectCallback onRootObjectAdded = obj => DiscoverNestedObjectsForTracking(obj, visitedObjects, 1, cancellationToken);

// Collect root-level objects and discover nested objects for tracking
CollectRootObjects(
testContext.Metadata.TestDetails,
(obj, depth) => TryAddToHashSet(visitedObjects, depth, obj),
obj => DiscoverNestedObjectsForTracking(obj, visitedObjects, 1, cancellationToken),
cancellationToken);
CollectRootObjects(testDetails, tryAdd, onRootObjectAdded, cancellationToken);

// A test that failed while it was being built never receives its row's objects, so they are
// tracked here only, never initialized: tracking them gives them the same reference-counted
// disposal as arguments, so they are disposed when the failed test completes unless another
// test still holds a shared instance (#7123).
ProcessRootCollection(testDetails.FailedDataRowObjects, tryAdd, onRootObjectAdded, cancellationToken);

return visitedObjects;
}
Expand Down
8 changes: 8 additions & 0 deletions src/TUnit.Core/TestDetails.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,14 @@ public partial class TestDetails : ITestIdentity, ITestClass, ITestMethod, ITest
public required object ClassInstance { get; set; }
public required object?[] TestMethodArguments { get; set; }
public required object?[] TestClassArguments { get; set; }

/// <summary>
/// Objects that data sources produced for a test that failed while it was being built, for example
/// a row whose inferred generic type arguments break a constraint. The test never receives them,
/// but they are tracked like its arguments, so they are disposed when the test completes unless
/// another test still references a shared instance (#7123).
/// </summary>
internal object?[] FailedDataRowObjects { get; init; } = [];
public TimeSpan? Timeout { get; set; }
public int RetryLimit { get; set; }

Expand Down
130 changes: 112 additions & 18 deletions src/TUnit.Engine/Building/TestBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,7 @@ public async ValueTask<IEnumerable<AbstractExecutableTest>> BuildTestsFromMetada

if (!instanceResult.Success)
{
var failedTest = CreateFailedTestForInstanceDataSourceError(metadata, instanceResult.Exception!);
var failedTest = CreateFailedTestForInstanceDataSourceError(metadata, instanceResult.Exception!, classData);
tests.Add(failedTest);
continue;
}
Expand Down Expand Up @@ -310,7 +310,8 @@ public async ValueTask<IEnumerable<AbstractExecutableTest>> BuildTestsFromMetada
// A source-generated row whose inferred type arguments break the method's
// constraints: fail this row only, as reflection mode does when it resolves
// the row's type arguments, and keep building the remaining rows.
tests.Add(CreateFailedTestForDataRowError(metadata, ex, classDataAttributeIndex, classDataLoopIndex, methodDataAttributeIndex, methodDataLoopIndex, i));
var (reportedException, rowData) = UnpackConstraintViolation(ex, metadata.MethodMetadata.Parameters);
tests.Add(CreateFailedTestForDataRowError(metadata, reportedException, classDataAttributeIndex, classDataLoopIndex, methodDataAttributeIndex, methodDataLoopIndex, i, classData, rowData));
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
continue;
}

Expand Down Expand Up @@ -394,15 +395,15 @@ public async ValueTask<IEnumerable<AbstractExecutableTest>> BuildTestsFromMetada
catch (Exception innerEx)
{
// If we still can't resolve, create a failed test
var failedTest = CreateFailedTestForDataRowError(metadata, innerEx, classDataAttributeIndex, classDataLoopIndex, methodDataAttributeIndex, methodDataLoopIndex, i);
var failedTest = CreateFailedTestForDataRowError(metadata, innerEx, classDataAttributeIndex, classDataLoopIndex, methodDataAttributeIndex, methodDataLoopIndex, i, classData, methodData);
tests.Add(failedTest);
continue;
}
}
catch (Exception ex)
{
// If generic resolution fails, create a failed test
var failedTest = CreateFailedTestForDataRowError(metadata, ex, classDataAttributeIndex, classDataLoopIndex, methodDataAttributeIndex, methodDataLoopIndex, i);
var failedTest = CreateFailedTestForDataRowError(metadata, ex, classDataAttributeIndex, classDataLoopIndex, methodDataAttributeIndex, methodDataLoopIndex, i, classData, methodData);
tests.Add(failedTest);
continue;
}
Expand Down Expand Up @@ -505,7 +506,7 @@ public async ValueTask<IEnumerable<AbstractExecutableTest>> BuildTestsFromMetada
// If generic type inference failed, create a failed test instead of skipped
if (genericResolutionException != null)
{
var failedTest = CreateFailedTestForDataGenerationError(metadata, genericResolutionException);
var failedTest = CreateFailedTestForDataGenerationError(metadata, genericResolutionException, new TestDataCombination(), classData);
tests.Add(failedTest);
}
else
Expand Down Expand Up @@ -1349,14 +1350,18 @@ private AbstractExecutableTest CreateFailedTestForDataGenerationError(TestMetada
/// Creates a failed test for one data row. The row's indexes go into the test id, so several failing
/// rows of the same method (or repeats of one row) keep distinct ids, as passing rows do.
/// </summary>
/// <param name="classData">The class row's objects, if the row produced them before it failed.</param>
/// <param name="methodData">The method row's objects, if the row produced them before it failed.</param>
private AbstractExecutableTest CreateFailedTestForDataRowError(
TestMetadata metadata,
Exception exception,
int classDataAttributeIndex,
int classDataLoopIndex,
int methodDataAttributeIndex,
int methodDataLoopIndex,
int repeatIndex)
int repeatIndex,
object?[]? classData = null,
object?[]? methodData = null)
{
return CreateFailedTestForDataGenerationError(metadata, exception, new TestDataCombination
{
Expand All @@ -1365,14 +1370,31 @@ private AbstractExecutableTest CreateFailedTestForDataRowError(
MethodDataSourceIndex = methodDataAttributeIndex,
MethodLoopIndex = methodDataLoopIndex,
RepeatIndex = repeatIndex
});
}, classData, methodData);
}

private AbstractExecutableTest CreateFailedTestForDataGenerationError(TestMetadata metadata, Exception exception, TestDataCombination combination)
/// <param name="metadata">The test's metadata.</param>
/// <param name="exception">The exception to report.</param>
/// <param name="combination">The row's indexes, which go into the test id.</param>
/// <param name="classData">The class row's objects, if the row produced them before it failed.</param>
/// <param name="methodData">The method row's objects, if the row produced them before it failed.</param>
/// <remarks>
/// The failed test never receives the row's objects, but it carries them in
/// <see cref="TestDetails.FailedDataRowObjects"/>, so the object tracker counts them when the test is
/// registered for execution and releases them when it completes. Releasing a reference disposes an
/// object only when no other test still holds it, so shared and keyed instances stay alive (#7123).
/// </remarks>
private AbstractExecutableTest CreateFailedTestForDataGenerationError(
TestMetadata metadata,
Exception exception,
TestDataCombination combination,
object?[]? classData = null,
object?[]? methodData = null)
{
exception = ToReportedException(exception);
var testId = TestIdentifierService.GenerateFailedTestId(metadata, combination);

var testDetails = CreateFailedTestDetails(metadata, testId);
var testDetails = CreateFailedTestDetails(metadata, testId, CombineRowObjects(classData, methodData));
var context = CreateFailedTestContext(metadata, testDetails);
var now = DateTimeOffset.UtcNow;

Expand Down Expand Up @@ -1418,11 +1440,77 @@ private static string GetTestName(TestMetadata metadata, TestData testData)
return TypeNameFormatter.AppendGenericArguments(metadata.TestName, resolvedMethodGenericArguments);
}

private TestDetails CreateFailedTestDetails(TestMetadata metadata, string testId)
private static object?[] CombineRowObjects(object?[]? classData, object?[]? methodData)
{
if (classData is not { Length: > 0 })
{
return methodData ?? [];
}

if (methodData is not { Length: > 0 })
{
return classData;
}

return [.. classData, .. methodData];
}

/// <summary>
/// A <see cref="GenericConstraintViolationDataSource"/> row raises a
/// <see cref="GenericConstraintViolationRowException"/> that carries the wrapped row's values. Returns
/// the exception to report, a plain <see cref="GenericTypeResolutionException"/> as in reflection mode,
/// and the row's unwrapped objects, so the failed test can track them for disposal (#7123).
/// </summary>
/// <remarks>
/// Never throws: callers run it inside a row's catch block, so an exception here would abandon the
/// method's remaining rows.
/// </remarks>
internal static (Exception ReportedException, object?[]? RowData) UnpackConstraintViolation(Exception exception, ParameterMetadata[] parameters)
{
if (exception is not GenericConstraintViolationRowException rowException)
{
return (exception, null);
}

object?[]? rowData = null;
if (rowException.Row is { } row)
{
try
{
rowData = DataUnwrapper.UnwrapWithTypes(row, parameters);
}
catch (Exception)
{
// Unwrapping invokes the inner Func of a TestDataRow<Func<T>>, which can throw. The row
// already fails with the constraint violation, so track what the row holds without
// invoking anything. A TestDataRow wrapper is not itself disposable, so track its value
// instead; a Func that threw created no value, so there is nothing to track for it.
rowData = row is [ITestDataRow testDataRow]
? testDataRow.GetData() is { } data and not Delegate ? [data] : []
: row;
}
}

return (ToReportedException(rowException), rowData);
}

/// <summary>
/// Replaces the internal <see cref="GenericConstraintViolationRowException"/> with the plain
/// <see cref="GenericTypeResolutionException"/> that reflection mode reports. Every failed-test factory
/// calls this, so a catch site that does not use <see cref="UnpackConstraintViolation"/> cannot leak
/// the internal type to users.
/// </summary>
internal static Exception ToReportedException(Exception exception)
=> exception is GenericConstraintViolationRowException rowException
? new GenericTypeResolutionException(rowException.Message)
: exception;

private TestDetails CreateFailedTestDetails(TestMetadata metadata, string testId, object?[] failedDataRowObjects)
{
return new TestDetails([])
{
TestId = testId,
FailedDataRowObjects = failedDataRowObjects,
TestName = metadata.TestName,
ClassType = metadata.TestClassType,
MethodName = metadata.TestMethodName,
Expand Down Expand Up @@ -1546,9 +1634,9 @@ private TestContext CreateFailedTestContext(TestMetadata metadata, TestDetails t



private AbstractExecutableTest CreateFailedTestForInstanceDataSourceError(TestMetadata metadata, Exception exception)
private AbstractExecutableTest CreateFailedTestForInstanceDataSourceError(TestMetadata metadata, Exception exception, object?[] classData)
{
return CreateFailedTestForDataGenerationError(metadata, exception);
return CreateFailedTestForDataGenerationError(metadata, exception, new TestDataCombination(), classData);
}

private AbstractExecutableTest CreateFailedTestForClassDataSourceCircularDependency(TestMetadata metadata)
Expand Down Expand Up @@ -1954,7 +2042,7 @@ public async IAsyncEnumerable<AbstractExecutableTest> BuildTestsStreamingAsync(
if (!instanceResult.Success)
{
// Yield a failed test instead of silently skipping
yield return CreateFailedTestForInstanceDataSourceError(metadata, instanceResult.Exception!);
yield return CreateFailedTestForInstanceDataSourceError(metadata, instanceResult.Exception!, classData);
continue;
}

Expand Down Expand Up @@ -2133,10 +2221,15 @@ private async Task<InstanceCreationResult> CreateInstanceForMethodDataSources(
IDataSourceAttribute? methodDataSource = null,
CancellationToken cancellationToken = default)
{
// Declared outside the try so that a row which fails after its factories ran still hands its
// objects to the failed test, which tracks them for disposal (#7123).
object?[]? classData = null;
object?[]? methodData = null;

try
{
var (classData, classRowMetadata) = DataUnwrapper.UnwrapWithMetadata(await classDataFactory() ?? []);
var (methodData, methodRowMetadata) = DataUnwrapper.UnwrapWithTypesAndMetadata(await methodDataFactory() ?? [], metadata.MethodMetadata.Parameters);
(classData, var classRowMetadata) = DataUnwrapper.UnwrapWithMetadata(await classDataFactory() ?? []);
(methodData, var methodRowMetadata) = DataUnwrapper.UnwrapWithTypesAndMetadata(await methodDataFactory() ?? [], metadata.MethodMetadata.Parameters);

// Extract and merge metadata from data source attributes and TestDataRow wrappers
var classAttrMetadata = DataSourceMetadataExtractor.ExtractFromAttribute(classDataSource);
Expand Down Expand Up @@ -2192,12 +2285,12 @@ private async Task<InstanceCreationResult> CreateInstanceForMethodDataSources(
}
catch (Exception ex)
{
return CreateFailedTestForDataRowError(metadata, ex, classDataAttributeIndex, classDataLoopIndex, methodDataAttributeIndex, methodDataLoopIndex, repeatIndex);
return CreateFailedTestForDataRowError(metadata, ex, classDataAttributeIndex, classDataLoopIndex, methodDataAttributeIndex, methodDataLoopIndex, repeatIndex, classData, methodData);
}
}
catch (Exception ex)
{
return CreateFailedTestForDataRowError(metadata, ex, classDataAttributeIndex, classDataLoopIndex, methodDataAttributeIndex, methodDataLoopIndex, repeatIndex);
return CreateFailedTestForDataRowError(metadata, ex, classDataAttributeIndex, classDataLoopIndex, methodDataAttributeIndex, methodDataLoopIndex, repeatIndex, classData, methodData);
}

if (metadata.TestClassType.IsGenericTypeDefinition && resolvedClassGenericArgs.Length == 0)
Expand Down Expand Up @@ -2266,7 +2359,8 @@ private async Task<InstanceCreationResult> CreateInstanceForMethodDataSources(
{
// Includes a GenericConstraintViolationDataSource row whose inferred type arguments break the
// method's constraints. The row's indexes keep its id distinct from the method's other rows.
return CreateFailedTestForDataRowError(metadata, ex, classDataAttributeIndex, classDataLoopIndex, methodDataAttributeIndex, methodDataLoopIndex, repeatIndex);
var (reportedException, rowData) = UnpackConstraintViolation(ex, metadata.MethodMetadata.Parameters);
return CreateFailedTestForDataRowError(metadata, reportedException, classDataAttributeIndex, classDataLoopIndex, methodDataAttributeIndex, methodDataLoopIndex, repeatIndex, classData, rowData ?? methodData);
}
}

Expand Down
1 change: 1 addition & 0 deletions src/TUnit.Engine/Building/TestBuilderPipeline.cs
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,7 @@ private async Task<AbstractExecutableTest[]> GenerateDynamicTests(TestMetadata m

private AbstractExecutableTest CreateFailedTestForDataGenerationError(TestMetadata metadata, Exception exception)
{
exception = TestBuilder.ToReportedException(exception);
var testId = TestIdentifierService.GenerateFailedTestId(metadata);
var displayName = $"{metadata.TestClassType.Name}.{metadata.TestName}";

Expand Down
Loading
Loading