Skip to content

Commit 8efae51

Browse files
committed
Fixes #332: Handling null/throwing TestCaseOrder
1 parent e9d212e commit 8efae51

9 files changed

Lines changed: 342 additions & 59 deletions

File tree

src/xunit.execution/Sdk/Frameworks/Runners/TestClassRunner.cs

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,15 @@ public abstract class TestClassRunner<TTestCase>
2323
/// <param name="testClass">The test class to be run.</param>
2424
/// <param name="class">The test class that contains the tests to be run.</param>
2525
/// <param name="testCases">The test cases to be run.</param>
26+
/// <param name="diagnosticMessageSink">The message sink used to send diagnostic messages</param>
2627
/// <param name="messageBus">The message bus to report run status to.</param>
2728
/// <param name="testCaseOrderer">The test case orderer that will be used to decide how to order the test.</param>
2829
/// <param name="aggregator">The exception aggregator used to run code and collect exceptions.</param>
2930
/// <param name="cancellationTokenSource">The task cancellation token source, used to cancel the test run.</param>
3031
public TestClassRunner(ITestClass testClass,
3132
IReflectionTypeInfo @class,
3233
IEnumerable<TTestCase> testCases,
34+
IMessageSink diagnosticMessageSink,
3335
IMessageBus messageBus,
3436
ITestCaseOrderer testCaseOrderer,
3537
ExceptionAggregator aggregator,
@@ -38,6 +40,7 @@ public TestClassRunner(ITestClass testClass,
3840
TestClass = testClass;
3941
Class = @class;
4042
TestCases = testCases;
43+
DiagnosticMessageSink = diagnosticMessageSink;
4144
MessageBus = messageBus;
4245
TestCaseOrderer = testCaseOrderer;
4346
Aggregator = aggregator;
@@ -59,6 +62,11 @@ public TestClassRunner(ITestClass testClass,
5962
/// </summary>
6063
protected IReflectionTypeInfo Class { get; set; }
6164

65+
/// <summary>
66+
/// Gets the message sink used to send diagnostic messages.
67+
/// </summary>
68+
protected IMessageSink DiagnosticMessageSink { get; private set; }
69+
6270
/// <summary>
6371
/// Gets or sets the message bus to report run status to.
6472
/// </summary>
@@ -185,7 +193,18 @@ public async Task<RunSummary> RunAsync()
185193
protected virtual async Task<RunSummary> RunTestMethodsAsync()
186194
{
187195
var summary = new RunSummary();
188-
var orderedTestCases = TestCaseOrderer.OrderTestCases(TestCases);
196+
IEnumerable<TTestCase> orderedTestCases;
197+
try
198+
{
199+
orderedTestCases = TestCaseOrderer.OrderTestCases(TestCases);
200+
}
201+
catch (Exception ex)
202+
{
203+
var innerEx = ex.Unwrap();
204+
DiagnosticMessageSink.OnMessage(new DiagnosticMessage("Test case orderer '{0}' threw '{1}' during ordering: {2}", TestCaseOrderer.GetType().FullName, innerEx.GetType().FullName, innerEx.StackTrace));
205+
orderedTestCases = TestCases.ToList();
206+
}
207+
189208
var constructorArguments = CreateTestClassConstructorArguments();
190209

191210
foreach (var method in orderedTestCases.GroupBy(tc => tc.TestMethod, TestMethodComparer.Instance))

src/xunit.execution/Sdk/Frameworks/Runners/XunitTestAssemblyRunner.cs

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,25 @@ protected void Initialize()
101101

102102
var testCaseOrdererAttribute = TestAssembly.Assembly.GetCustomAttributes(typeof(TestCaseOrdererAttribute)).SingleOrDefault();
103103
if (testCaseOrdererAttribute != null)
104-
TestCaseOrderer = ExtensibilityPointFactory.GetTestCaseOrderer(DiagnosticMessageSink, testCaseOrdererAttribute);
104+
{
105+
try
106+
{
107+
var testCaseOrderer = ExtensibilityPointFactory.GetTestCaseOrderer(DiagnosticMessageSink, testCaseOrdererAttribute);
108+
if (testCaseOrderer != null)
109+
TestCaseOrderer = testCaseOrderer;
110+
else
111+
{
112+
var args = testCaseOrdererAttribute.GetConstructorArguments().Cast<string>().ToList();
113+
DiagnosticMessageSink.OnMessage(new DiagnosticMessage("Could not find type '{0}' in {1} for assembly-level test case orderer", args[0], args[1]));
114+
}
115+
}
116+
catch (Exception ex)
117+
{
118+
var innerEx = ex.Unwrap();
119+
var args = testCaseOrdererAttribute.GetConstructorArguments().Cast<string>().ToList();
120+
DiagnosticMessageSink.OnMessage(new DiagnosticMessage("Assembly-level test case orderer '{0}' threw '{1}' during construction: {2}", args[0], innerEx.GetType().FullName, innerEx.StackTrace));
121+
}
122+
}
105123

106124
var testCollectionOrdererAttribute = TestAssembly.Assembly.GetCustomAttributes(typeof(TestCollectionOrdererAttribute)).SingleOrDefault();
107125
if (testCollectionOrdererAttribute != null)

src/xunit.execution/Sdk/Frameworks/Runners/XunitTestClassRunner.cs

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ namespace Xunit.Sdk
1414
public class XunitTestClassRunner : TestClassRunner<IXunitTestCase>
1515
{
1616
readonly IDictionary<Type, object> collectionFixtureMappings;
17-
readonly IMessageSink diagnosticMessageSink;
1817

1918
/// <summary>
2019
/// Initializes a new instance of the <see cref="XunitTestClassRunner"/> class.
@@ -37,9 +36,8 @@ public XunitTestClassRunner(ITestClass testClass,
3736
ExceptionAggregator aggregator,
3837
CancellationTokenSource cancellationTokenSource,
3938
IDictionary<Type, object> collectionFixtureMappings)
40-
: base(testClass, @class, testCases, messageBus, testCaseOrderer, aggregator, cancellationTokenSource)
39+
: base(testClass, @class, testCases, diagnosticMessageSink, messageBus, testCaseOrderer, aggregator, cancellationTokenSource)
4140
{
42-
this.diagnosticMessageSink = diagnosticMessageSink;
4341
this.collectionFixtureMappings = collectionFixtureMappings;
4442

4543
ClassFixtureMappings = new Dictionary<Type, object>();
@@ -68,7 +66,25 @@ protected override Task AfterTestClassStartingAsync()
6866
{
6967
var ordererAttribute = Class.GetCustomAttributes(typeof(TestCaseOrdererAttribute)).SingleOrDefault();
7068
if (ordererAttribute != null)
71-
TestCaseOrderer = ExtensibilityPointFactory.GetTestCaseOrderer(diagnosticMessageSink, ordererAttribute);
69+
{
70+
try
71+
{
72+
var testCaseOrderer = ExtensibilityPointFactory.GetTestCaseOrderer(DiagnosticMessageSink, ordererAttribute);
73+
if (testCaseOrderer != null)
74+
TestCaseOrderer = testCaseOrderer;
75+
else
76+
{
77+
var args = ordererAttribute.GetConstructorArguments().Cast<string>().ToList();
78+
DiagnosticMessageSink.OnMessage(new DiagnosticMessage("Could not find type '{0}' in {1} for class-level test case orderer on test class '{2}'", args[0], args[1], TestClass.Class.Name));
79+
}
80+
}
81+
catch (Exception ex)
82+
{
83+
var innerEx = ex.Unwrap();
84+
var args = ordererAttribute.GetConstructorArguments().Cast<string>().ToList();
85+
DiagnosticMessageSink.OnMessage(new DiagnosticMessage("Class-level test case orderer '{0}' for test class '{1}' threw '{2}' during construction: {3}", args[0], TestClass.Class.Name, innerEx.GetType().FullName, innerEx.StackTrace));
86+
}
87+
}
7288

7389
var testClassTypeInfo = Class.Type.GetTypeInfo();
7490
if (testClassTypeInfo.ImplementedInterfaces.Any(i => i.GetTypeInfo().IsGenericType && i.GetGenericTypeDefinition() == typeof(ICollectionFixture<>)))
@@ -99,7 +115,7 @@ protected override Task BeforeTestClassFinishedAsync()
99115
/// <inheritdoc/>
100116
protected override Task<RunSummary> RunTestMethodAsync(ITestMethod testMethod, IReflectionMethodInfo method, IEnumerable<IXunitTestCase> testCases, object[] constructorArguments)
101117
{
102-
return new XunitTestMethodRunner(testMethod, Class, method, testCases, diagnosticMessageSink, MessageBus, new ExceptionAggregator(Aggregator), CancellationTokenSource, constructorArguments).RunAsync();
118+
return new XunitTestMethodRunner(testMethod, Class, method, testCases, DiagnosticMessageSink, MessageBus, new ExceptionAggregator(Aggregator), CancellationTokenSource, constructorArguments).RunAsync();
103119
}
104120

105121
/// <inheritdoc/>

src/xunit.execution/Sdk/Frameworks/Runners/XunitTestCollectionRunner.cs

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,25 @@ protected override Task AfterTestCollectionStartingAsync()
6161

6262
var ordererAttribute = TestCollection.CollectionDefinition.GetCustomAttributes(typeof(TestCaseOrdererAttribute)).SingleOrDefault();
6363
if (ordererAttribute != null)
64-
TestCaseOrderer = ExtensibilityPointFactory.GetTestCaseOrderer(diagnosticMessageSink, ordererAttribute);
64+
{
65+
try
66+
{
67+
var testCaseOrderer = ExtensibilityPointFactory.GetTestCaseOrderer(diagnosticMessageSink, ordererAttribute);
68+
if (testCaseOrderer != null)
69+
TestCaseOrderer = testCaseOrderer;
70+
else
71+
{
72+
var args = ordererAttribute.GetConstructorArguments().Cast<string>().ToList();
73+
diagnosticMessageSink.OnMessage(new DiagnosticMessage("Could not find type '{0}' in {1} for collection-level test case orderer on test collection '{2}'", args[0], args[1], TestCollection.DisplayName));
74+
}
75+
}
76+
catch (Exception ex)
77+
{
78+
var innerEx = ex.Unwrap();
79+
var args = ordererAttribute.GetConstructorArguments().Cast<string>().ToList();
80+
diagnosticMessageSink.OnMessage(new DiagnosticMessage("Collection-level test case orderer '{0}' for test collection '{1}' threw '{2}' during construction: {3}", args[0], TestCollection.DisplayName, innerEx.GetType().FullName, innerEx.StackTrace));
81+
}
82+
}
6583
}
6684

6785
return Task.FromResult(0);

test/test.utility/Mocks.cs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -217,17 +217,20 @@ public static ITestCase TestCase(Type type, string methodName, string displayNam
217217
return result;
218218
}
219219

220-
public static IReflectionAttributeInfo TestCaseOrdererAttribute<TOrderer>()
220+
public static IReflectionAttributeInfo TestCaseOrdererAttribute(string typeName, string assemblyName)
221221
{
222222
var result = Substitute.For<IReflectionAttributeInfo, InterfaceProxy<IReflectionAttributeInfo>>();
223-
var ordererType = typeof(TOrderer);
224-
var typeName = ordererType.FullName;
225-
var assemblyName = ordererType.Assembly.FullName;
226223
result.Attribute.Returns(new TestCaseOrdererAttribute(typeName, assemblyName));
227224
result.GetConstructorArguments().Returns(new object[] { typeName, assemblyName });
228225
return result;
229226
}
230227

228+
public static IReflectionAttributeInfo TestCaseOrdererAttribute<TOrderer>()
229+
{
230+
var ordererType = typeof(TOrderer);
231+
return TestCaseOrdererAttribute(ordererType.FullName, ordererType.Assembly.FullName);
232+
}
233+
231234
public static ITestClass TestClass(string typeName, IReflectionAttributeInfo[] attributes = null)
232235
{
233236
var testCollection = Mocks.TestCollection();

test/test.xunit.execution/Sdk/Frameworks/Runners/TestClassRunnerTests.cs

Lines changed: 73 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -199,35 +199,79 @@ public static async void SignalingCancellationStopsRunningMethods()
199199
Assert.Equal("Passing", tuple.Item1.Name);
200200
}
201201

202-
[Fact]
203-
public static async void TestsOrdererIsUsedToDetermineRunOrder()
202+
public class TestCaseOrderer
204203
{
205-
var passing1 = Mocks.TestCase<ClassUnderTest>("Passing");
206-
var passing2 = Mocks.TestCase<ClassUnderTest>("Passing");
207-
var other1 = Mocks.TestCase<ClassUnderTest>("Other");
208-
var other2 = Mocks.TestCase<ClassUnderTest>("Other");
209-
var runner = TestableTestClassRunner.Create(testCases: new[] { passing1, other1, passing2, other2 }, orderer: new MockTestCaseOrderer(reverse: true));
204+
[Fact]
205+
public static async void TestsOrdererIsUsedToDetermineRunOrder()
206+
{
207+
var passing1 = Mocks.TestCase<ClassUnderTest>("Passing");
208+
var passing2 = Mocks.TestCase<ClassUnderTest>("Passing");
209+
var other1 = Mocks.TestCase<ClassUnderTest>("Other");
210+
var other2 = Mocks.TestCase<ClassUnderTest>("Other");
211+
var runner = TestableTestClassRunner.Create(testCases: new[] { passing1, other1, passing2, other2 }, orderer: new MockTestCaseOrderer(reverse: true));
212+
213+
await runner.RunAsync();
214+
215+
Assert.Collection(runner.MethodsRun,
216+
tuple =>
217+
{
218+
Assert.Equal("Other", tuple.Item1.Name);
219+
Assert.Collection(tuple.Item2,
220+
testCase => Assert.Same(other2, testCase),
221+
testCase => Assert.Same(other1, testCase)
222+
);
223+
},
224+
tuple =>
225+
{
226+
Assert.Equal("Passing", tuple.Item1.Name);
227+
Assert.Collection(tuple.Item2,
228+
testCase => Assert.Same(passing2, testCase),
229+
testCase => Assert.Same(passing1, testCase)
230+
);
231+
}
232+
);
233+
}
210234

211-
await runner.RunAsync();
235+
[Fact]
236+
public static async void TestCaseOrdererWhichThrowsLogsMessageAndDoesNotReorderTests()
237+
{
238+
var passing1 = Mocks.TestCase<ClassUnderTest>("Passing");
239+
var passing2 = Mocks.TestCase<ClassUnderTest>("Passing");
240+
var other1 = Mocks.TestCase<ClassUnderTest>("Other");
241+
var other2 = Mocks.TestCase<ClassUnderTest>("Other");
242+
var runner = TestableTestClassRunner.Create(testCases: new[] { passing1, other1, passing2, other2 }, orderer: new ThrowingOrderer());
243+
244+
await runner.RunAsync();
245+
246+
Assert.Collection(runner.MethodsRun,
247+
tuple =>
248+
{
249+
Assert.Equal("Passing", tuple.Item1.Name);
250+
Assert.Collection(tuple.Item2,
251+
testCase => Assert.Same(passing1, testCase),
252+
testCase => Assert.Same(passing2, testCase)
253+
);
254+
},
255+
tuple =>
256+
{
257+
Assert.Equal("Other", tuple.Item1.Name);
258+
Assert.Collection(tuple.Item2,
259+
testCase => Assert.Same(other1, testCase),
260+
testCase => Assert.Same(other2, testCase)
261+
);
262+
}
263+
);
264+
var diagnosticMessage = Assert.Single(runner.DiagnosticMessages.Cast<IDiagnosticMessage>());
265+
Assert.StartsWith("Test case orderer 'TestClassRunnerTests+TestCaseOrderer+ThrowingOrderer' threw 'System.DivideByZeroException' during ordering:", diagnosticMessage.Message);
266+
}
212267

213-
Assert.Collection(runner.MethodsRun,
214-
tuple =>
215-
{
216-
Assert.Equal("Other", tuple.Item1.Name);
217-
Assert.Collection(tuple.Item2,
218-
testCase => Assert.Same(other2, testCase),
219-
testCase => Assert.Same(other1, testCase)
220-
);
221-
},
222-
tuple =>
268+
class ThrowingOrderer : ITestCaseOrderer
269+
{
270+
public IEnumerable<TTestCase> OrderTestCases<TTestCase>(IEnumerable<TTestCase> testCases) where TTestCase : ITestCase
223271
{
224-
Assert.Equal("Passing", tuple.Item1.Name);
225-
Assert.Collection(tuple.Item2,
226-
testCase => Assert.Same(passing2, testCase),
227-
testCase => Assert.Same(passing1, testCase)
228-
);
272+
throw new DivideByZeroException();
229273
}
230-
);
274+
}
231275
}
232276

233277
[Fact]
@@ -304,12 +348,14 @@ class TestableTestClassRunner : TestClassRunner<ITestCase>
304348
public bool AfterTestClassStarting_Called;
305349
public Action<ExceptionAggregator> BeforeTestClassFinished_Callback = _ => { };
306350
public bool BeforeTestClassFinished_Called;
351+
public List<IMessageSinkMessage> DiagnosticMessages;
307352
public Exception RunTestMethodAsync_AggregatorResult;
308353
public readonly CancellationTokenSource TokenSource;
309354

310355
TestableTestClassRunner(ITestClass testClass,
311356
IReflectionTypeInfo @class,
312357
IEnumerable<ITestCase> testCases,
358+
List<IMessageSinkMessage> diagnosticMessages,
313359
IMessageBus messageBus,
314360
ITestCaseOrderer testCaseOrderer,
315361
ExceptionAggregator aggregator,
@@ -318,8 +364,9 @@ class TestableTestClassRunner : TestClassRunner<ITestCase>
318364
object[] availableArguments,
319365
RunSummary result,
320366
bool cancelInRunTestMethodAsync)
321-
: base(testClass, @class, testCases, messageBus, testCaseOrderer, aggregator, cancellationTokenSource)
367+
: base(testClass, @class, testCases, SpyMessageSink.Create(messages: diagnosticMessages), messageBus, testCaseOrderer, aggregator, cancellationTokenSource)
322368
{
369+
DiagnosticMessages = diagnosticMessages;
323370
TokenSource = cancellationTokenSource;
324371

325372
this.constructor = constructor;
@@ -352,6 +399,7 @@ public static TestableTestClassRunner Create(IMessageBus messageBus = null,
352399
firstTest.TestMethod.TestClass,
353400
(IReflectionTypeInfo)firstTest.TestMethod.TestClass.Class,
354401
testCases,
402+
new List<IMessageSinkMessage>(),
355403
messageBus ?? new SpyMessageBus(),
356404
orderer ?? new MockTestCaseOrderer(),
357405
aggregator,

0 commit comments

Comments
 (0)