[UnitTesting] Fix possible null reference exception

Fixed bug #57783 - Null reference when running unit tests
https://bugzilla.xamarin.com/show_bug.cgi?id=57783

When building the test results the builder now handles null
returned from UnitTest.GetLastResult.

2 files changed, 139 insertions, 7 deletions

diff --git a/main/src/addins/MonoDevelop.UnitTesting/MonoDevelop.UnitTesting.Tests/MonoDevelop.UnitTesting.Tests/TestResultBuilderTests.cs b/main/src/addins/MonoDevelop.UnitTesting/MonoDevelop.UnitTesting.Tests/MonoDevelop.UnitTesting.Tests/TestResultBuilderTests.cs
index 1723a82095..1296df640b 100644
--- a/main/src/addins/MonoDevelop.UnitTesting/MonoDevelop.UnitTesting.Tests/MonoDevelop.UnitTesting.Tests/TestResultBuilderTests.cs
+++ b/main/src/addins/MonoDevelop.UnitTesting/MonoDevelop.UnitTesting.Tests/MonoDevelop.UnitTesting.Tests/TestResultBuilderTests.cs
@@ -50,14 +50,19 @@ namespace MonoDevelop.UnitTesting.Tests
 			builder = new TestResultBuilder (context, testProvider);
 		}
 
-		VsTestUnitTest CreateVsUnitTest (string fullyQualifedName)
+		VsTestUnitTest CreateVsUnitTest (string fullyQualifiedName)
 		{
-			var testCase = new TestCase {
-				DisplayName = fullyQualifedName,
-				FullyQualifiedName = fullyQualifedName,
+			var testCase = CreateTestCase (fullyQualifiedName);
+			return new VsTestUnitTest (null, testCase, null);
+		}
+
+		TestCase CreateTestCase (string fullyQualifiedName)
+		{
+			return new TestCase {
+				DisplayName = fullyQualifiedName,
+				FullyQualifiedName = fullyQualifiedName,
 				CodeFilePath = "test.cs"
 			};
-			return new VsTestUnitTest (null, testCase, null);
 		}
 
 		TestRunChangedEventArgs CreateTestRunChangedEventArgsWithTestResults (params TestResult[] newTestResults)
@@ -65,6 +70,11 @@ namespace MonoDevelop.UnitTesting.Tests
 			return new TestRunChangedEventArgs (null, newTestResults, null);
 		}
 
+		VsTestNamespaceTestGroup CreateVsUnitTestNamespace (string name)
+		{
+			return new VsTestNamespaceTestGroup (null, null, null, name);
+		}
+
 		[Test]
 		public void ConsoleOutputText_SingleTestUsingTestOutputHelperWriteLine_ConsoleOutputInTestResultOutput ()
 		{
@@ -110,5 +120,125 @@ namespace MonoDevelop.UnitTesting.Tests
 			Assert.AreEqual (0, builder.TestResult.Passed);
 			Assert.AreEqual (0, builder.TestResult.Inconclusive);
 		}
+
+		[Test]
+		public void AddTests_TestClassUsesRootNamespace ()
+		{
+			var testNamespace = CreateVsUnitTestNamespace ("RootNamespace");
+			var testCase = CreateTestCase ("RootNamespace.MyClass.MyTest");
+
+			testNamespace.AddTests (new [] { testCase });
+
+			Assert.AreEqual ("RootNamespace", testNamespace.FixtureTypeNamespace);
+			Assert.AreEqual ("RootNamespace", testNamespace.Name);
+			Assert.AreEqual ("RootNamespace", testNamespace.FullName);
+			Assert.IsTrue (testNamespace.HasTests);
+			Assert.AreEqual (1, testNamespace.Tests.Count);
+
+			var testClass = testNamespace.Tests [0] as VsTestTestClass;
+			Assert.AreEqual ("MyClass", testClass.Name);
+			Assert.AreEqual ("RootNamespace.MyClass", testClass.FullName);
+			Assert.IsTrue (testClass.HasTests);
+			Assert.AreEqual (1, testClass.Tests.Count);
+
+			var test = testClass.Tests [0] as VsTestUnitTest;
+			Assert.AreEqual ("MyTest", test.Name);
+			Assert.AreEqual ("RootNamespace", test.FixtureTypeNamespace);
+			Assert.AreEqual ("RootNamespace.MyClass.MyTest", test.FullName);
+		}
+
+		[Test]
+		public void OnTestRunChanged_SingleTest_RunTestsFromNamespace_TestPasses ()
+		{
+			var testNamespace = CreateVsUnitTestNamespace ("RootNamespace");
+			var testCase = CreateTestCase ("RootNamespace.MyClass.MyTest");
+			testNamespace.AddTests (new [] { testCase });
+			CreateTestResultBuilder (testNamespace);
+			var testClass = testNamespace.Tests [0] as VsTestTestClass;
+			var test = testClass.Tests [0] as VsTestUnitTest;
+			testClass.Status = TestStatus.Running;
+			test.Status = TestStatus.Running;
+			var testResult = new TestResult (testCase) {
+				Outcome = TestOutcome.Passed
+			};
+			var eventArgs = CreateTestRunChangedEventArgsWithTestResults (testResult);
+
+			builder.OnTestRunChanged (eventArgs);
+
+			var result = test.GetLastResult ();
+			Assert.AreEqual (ResultStatus.Success, result.Status);
+			result = testClass.GetLastResult ();
+			Assert.AreEqual (ResultStatus.Success, result.Status);
+			Assert.AreEqual (TestStatus.Ready, testClass.Status);
+			Assert.AreEqual (TestStatus.Ready, test.Status);
+		}
+
+		[Test]
+		public void OnTestRunChanged_TwoTests_RunOneTestInNamespace_TestPasses ()
+		{
+			var testNamespace = CreateVsUnitTestNamespace ("RootNamespace");
+			var testCase1 = CreateTestCase ("RootNamespace.MyClass.MyTest1");
+			testNamespace.AddTests (new [] { testCase1 });
+			var testCase2 = CreateTestCase ("RootNamespace.MyClass.MyTest2");
+			testNamespace.AddTests (new [] { testCase2 });
+			var testClass = testNamespace.Tests [0] as VsTestTestClass;
+			var test1 = testClass.Tests [0] as VsTestUnitTest;
+			var test2 = testClass.Tests [1] as VsTestUnitTest;
+			testClass.Status = TestStatus.Running;
+			test1.Status = TestStatus.Running;
+			test2.Status = TestStatus.Running;
+			CreateTestResultBuilder (testNamespace);
+			var testResult = new TestResult (testCase1) {
+				Outcome = TestOutcome.Passed
+			};
+			var eventArgs = CreateTestRunChangedEventArgsWithTestResults (testResult);
+
+			builder.OnTestRunChanged (eventArgs);
+
+			var result = test1.GetLastResult ();
+			Assert.AreEqual (ResultStatus.Success, result.Status);
+			result = test2.GetLastResult ();
+			Assert.IsNull (result);
+			Assert.AreEqual (TestStatus.Running, testClass.Status);
+			Assert.AreEqual (TestStatus.Ready, test1.Status);
+			Assert.AreEqual (TestStatus.Running, test2.Status);
+		}
+
+		/// <summary>
+		/// As above but a result is not obtained for the second test but its
+		/// status is changed to Ready and then a test result is returned which
+		/// will cause a null test result to be returned from the UnitTest's
+		/// GetLastResult method. Not sure exactly how this happens in practice
+		/// but all existing code that uses the GetLastResult method always checks
+		/// for null which the TestResultBuilder was not doing.
+		/// </summary>
+		[Test]
+		public void OnTestRunChanged_TwoTests_RunOneTestInNamespace_NullTestResult_TestPasses ()
+		{
+			var testNamespace = CreateVsUnitTestNamespace ("RootNamespace");
+			var testCase1 = CreateTestCase ("RootNamespace.MyClass.MyTest1");
+			testNamespace.AddTests (new [] { testCase1 });
+			var testCase2 = CreateTestCase ("RootNamespace.MyClass.MyTest2");
+			testNamespace.AddTests (new [] { testCase2 });
+			var testClass = testNamespace.Tests [0] as VsTestTestClass;
+			var test1 = testClass.Tests [0] as VsTestUnitTest;
+			var test2 = testClass.Tests [1] as VsTestUnitTest;
+			testClass.Status = TestStatus.Running;
+			test1.Status = TestStatus.Running;
+			test2.Status = TestStatus.Ready;
+			CreateTestResultBuilder (testNamespace);
+			var testResult = new TestResult (testCase1) {
+				Outcome = TestOutcome.Passed
+			};
+			var eventArgs = CreateTestRunChangedEventArgsWithTestResults (testResult);
+
+			builder.OnTestRunChanged (eventArgs);
+
+			var result = test1.GetLastResult ();
+			Assert.AreEqual (ResultStatus.Success, result.Status);
+			result = test2.GetLastResult ();
+			Assert.IsNull (result);
+			Assert.AreEqual (TestStatus.Ready, test1.Status);
+		}
 	}
 }
diff --git a/main/src/addins/MonoDevelop.UnitTesting/MonoDevelop.UnitTesting.VsTest/TestResultBuilder.cs b/main/src/addins/MonoDevelop.UnitTesting/MonoDevelop.UnitTesting.VsTest/TestResultBuilder.cs
index 98102b6f70..1f5226860b 100644
--- a/main/src/addins/MonoDevelop.UnitTesting/MonoDevelop.UnitTesting.VsTest/TestResultBuilder.cs
+++ b/main/src/addins/MonoDevelop.UnitTesting/MonoDevelop.UnitTesting.VsTest/TestResultBuilder.cs
@@ -247,8 +247,10 @@ namespace MonoDevelop.UnitTesting.VsTest
 			var result = UnitTestResult.CreateSuccess ();
 			foreach (UnitTest test in parent.Tests) {
 				UnitTestResult childResult = test.GetLastResult ();
-				result.Add (childResult);
-				UpdateCounts (result, childResult);
+				if (childResult != null) {
+					result.Add (childResult);
+					UpdateCounts (result, childResult);
+				}
 			}
 			return result;
 		}