Ensure tests run on ci (#305)

* Roll back nunit3testadapter to 4.3.2 which was the last version to support .net 461
* Change CI to publish test results and fail if no tests are found
* Update artifacts actions as they must all be in sync. Use pattern to download Test results artifacts
* Match SIL.TestUtilities reference to other palaso libs to avoid pulling in newer versions of SIL.Core
* Fix bug in LcmSet.CopyTo where it would only work if the arrayIndex was 0 and add a test for this case

---------

Co-authored-by: Jason Naylor <jason_naylor@sil.org>

.github/workflows/ci-cd.yml

@@ -67,12 +67,18 @@ jobs:
     - name: Test on Linux
       run: |
         . environ
-        dotnet test --no-restore --no-build -p:ParallelizeAssembly=false --configuration Release -- RunConfiguration.FailWhenNoTestsFound=true
+        dotnet test --no-restore --no-build -p:ParallelizeAssembly=false --configuration Release --logger:"trx" --results-directory ./test-results
       if: matrix.os == 'ubuntu-latest'
 
     - name: Test on Windows
-      run: dotnet test --test-adapter-path "%UserProfile%\.nuget\packages\nunit3testadapter\4.5.0\build\net462" --no-restore --no-build -p:ParallelizeAssembly=false --configuration Release -- RunConfiguration.FailWhenNoTestsFound=true
+      run: dotnet test --no-restore --no-build -p:ParallelizeAssembly=false --configuration Release --logger:"trx" --results-directory ./test-results
       if: matrix.os != 'ubuntu-latest'
+    - name: Upload test results
+      if: always()
+      uses: actions/upload-artifact@v4
+      with:
+        name: Test results (${{ matrix.os }})
+        path: ./test-results
 
     - name: Package
       run: dotnet pack --include-symbols --no-restore --no-build -p:SymbolPackageFormat=snupkg --configuration Release
@@ -86,8 +92,25 @@ jobs:
       run: dotnet nuget push artifacts/*.nupkg -s https://nuget.pkg.github.com/sillsdev/index.json -k ${{secrets.GITHUB_TOKEN}} --skip-duplicate
 
     - name: Publish Artifacts
-      uses: actions/upload-artifact@v3
+      uses: actions/upload-artifact@v4
       with:
         name: NugetPackages
         path: artifacts/*.nupkg
       if: github.event_name == 'pull_request'
+  publish-test-results:
+    runs-on: ubuntu-latest
+    needs: build
+    if: always()
+    steps:
+      - name: Download test results
+        uses: actions/download-artifact@v4
+        with:
+          path: artifacts
+          pattern: Test results *
+      - name: Publish test results
+        uses: EnricoMi/publish-unit-test-result-action@8885e273a4343cd7b48eaa72428dea0c3067ea98 # v2.14.0
+        with:
+          check_name: LCM Tests
+          files: artifacts/**/*.trx
+          action_fail: true
+          action_fail_on_inconclusive: true

src/SIL.LCModel/DomainImpl/Vectors.cs

@@ -421,19 +421,13 @@ public void CopyTo(T[] array, int arrayIndex)
 			// TODO: Check for multidimensional 'array' and throw ArgumentException, if it is.
 			lock (SyncRoot)
 			{
-				//if (arrayIndex >= array.Length || m_items.Count - arrayIndex >= array.Length)
-				// equals sign causes spurious ArgumentException when copying entire array
-				if (array.Length == 0)
-					return;
-				if (arrayIndex >= array.Length || m_items.Count - arrayIndex > array.Length)
-					throw new ArgumentException();
-
-				int currentIndex = 0;
-				int currentcopiedIndex = 0;
-				foreach (var objOrId in m_items.ToArray())
+				if (m_items.Count + arrayIndex > array.Length)
+					throw new ArgumentOutOfRangeException("arrayIndex");
+
+				int currentcopiedIndex = arrayIndex;
+				foreach (var cmObjectOrId in m_items)
 				{
-					if (currentIndex++ < arrayIndex) continue;
-					array.SetValue(FluffUpObjectIfNeeded(objOrId), currentcopiedIndex++);
+					array[currentcopiedIndex++] = FluffUpObjectIfNeeded(cmObjectOrId);
 				}
 			}
 		}

tests/SIL.LCModel.Core.Tests/SIL.LCModel.Core.Tests.csproj

@@ -14,9 +14,9 @@ This package provides unit tests for SIL.LCModel.Core.</Description>
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.0.0" />
     <PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.0.0" PrivateAssets="All" />
     <PackageReference Include="NUnit" Version="3.13.3" />
-    <PackageReference Include="NUnit3TestAdapter" Version="4.5.0" />
+    <PackageReference Include="NUnit3TestAdapter" Version="4.3.2" />
     <PackageReference Include="SIL.ReleaseTasks" Version="2.5.0" PrivateAssets="All" />
-    <PackageReference Include="SIL.TestUtilities" Version="13.0.1" />
+    <PackageReference Include="SIL.TestUtilities" Version="12.0.0-*" />
   </ItemGroup>
 
   <ItemGroup>

tests/SIL.LCModel.FixData.Tests/SIL.LCModel.FixData.Tests.csproj

@@ -11,7 +11,7 @@
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.0.0" />
     <PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.0.0" PrivateAssets="All" />
     <PackageReference Include="NUnit" Version="3.13.3" />
-    <PackageReference Include="NUnit3TestAdapter" Version="4.5.0" />
+    <PackageReference Include="NUnit3TestAdapter" Version="4.3.2" />
     <PackageReference Include="SIL.ReleaseTasks" Version="2.5.0" PrivateAssets="All" />
   </ItemGroup>
 

tests/SIL.LCModel.Tests/DomainImpl/VectorTests.cs

@@ -396,11 +396,11 @@ private ILexEntry MakeEntry()
 
 		/// ------------------------------------------------------------------------------------
 		/// <summary>
-		/// Tests the CopyTo method.
+		/// Tests the CopyTo LcmList method.
 		/// </summary>
 		/// ------------------------------------------------------------------------------------
 		[Test]
-		public void CopyTo_OneItemInLongListTest()
+		public void CopyTo_LcmList_OneItemInLongListTest()
 		{
 			ILcmServiceLocator servLoc = Cache.ServiceLocator;
 			IScrBookFactory bookFact = servLoc.GetInstance<IScrBookFactory>();
@@ -418,13 +418,37 @@ public void CopyTo_OneItemInLongListTest()
 			Assert.IsNull(bookArray[4]);
 		}
 
+		/// ------------------------------------------------------------------------------------
+		/// <summary>
+		/// Tests the CopyTo LcmSet method.
+		/// </summary>
+		/// ------------------------------------------------------------------------------------
+		[Test]
+		public void CopyTo_LcmSet_OneItemInLongListTest()
+		{
+			ILcmServiceLocator servLoc = Cache.ServiceLocator;
+			IScrDraftFactory draftFactory = servLoc.GetInstance<IScrDraftFactory>();
+
+			// Setup the source sequence using the drafts collection.
+			IScrDraft draft0 = draftFactory.Create("draft");
+
+			IScrDraft[] draftArray = new IScrDraft[5];
+			m_scr.ArchivedDraftsOC.CopyTo(draftArray, 3);
+
+			Assert.IsNull(draftArray[0]);
+			Assert.IsNull(draftArray[1]);
+			Assert.IsNull(draftArray[2]);
+			Assert.AreEqual(draft0, draftArray[3]);
+			Assert.IsNull(draftArray[4]);
+		}
+
 		/// ------------------------------------------------------------------------------------
 		/// <summary>
 		/// Tests the CopyTo method when we are copying into a one-item list.
 		/// </summary>
 		/// ------------------------------------------------------------------------------------
 		[Test]
-		public void CopyTo_OneItemInOneItemListTest()
+		public void CopyTo_LcmList_OneItemInOneItemListTest()
 		{
 			ILcmServiceLocator servLoc = Cache.ServiceLocator;
 			IScrBookFactory bookFact = servLoc.GetInstance<IScrBookFactory>();
@@ -438,13 +462,33 @@ public void CopyTo_OneItemInOneItemListTest()
 			Assert.AreEqual(book0, bookArray[0]);
 		}
 
+		/// ------------------------------------------------------------------------------------
+		/// <summary>
+		/// Tests the CopyTo method when we are copying into a one-item set.
+		/// </summary>
+		/// ------------------------------------------------------------------------------------
+		[Test]
+		public void CopyTo_LcmSet_OneItemInOneItemListTest()
+		{
+			ILcmServiceLocator servLoc = Cache.ServiceLocator;
+			IScrDraftFactory draftFactory = servLoc.GetInstance<IScrDraftFactory>();
+
+			// Setup the source sequence using the drafts collection.
+			IScrDraft draft0 = draftFactory.Create("draft");
+
+			IScrDraft[] draftArray = new IScrDraft[1];
+			m_scr.ArchivedDraftsOC.CopyTo(draftArray, 0);
+
+			Assert.AreEqual(draft0, draftArray[0]);
+		}
+
 		/// ------------------------------------------------------------------------------------
 		/// <summary>
 		/// Tests the CopyTo method when we are copying no items into an empty list.
 		/// </summary>
 		/// ------------------------------------------------------------------------------------
 		[Test]
-		public void CopyTo_NoItemsInEmptyItemListTest()
+		public void CopyTo_LcmList_NoItemsInEmptyItemListTest()
 		{
 			ILcmServiceLocator servLoc = Cache.ServiceLocator;
 
@@ -454,6 +498,20 @@ public void CopyTo_NoItemsInEmptyItemListTest()
 			// This fixes creating a new List<> when giving a LcmVector as the parameter.
 		}
 
+		/// ------------------------------------------------------------------------------------
+		/// <summary>
+		/// Tests the CopyTo method when we are copying no items into an empty set.
+		/// </summary>
+		/// ------------------------------------------------------------------------------------
+		[Test]
+		public void CopyTo_LcmSet_NoItemsInEmptyItemListTest()
+		{
+			IScrDraft[] draftArray = new IScrDraft[0];
+			m_scr.ArchivedDraftsOC.CopyTo(draftArray, 0);
+			// This test makes sure that an exception is not thrown when the array is empty.
+			// This fixes creating a new List<> when giving a LcmVector as the parameter.
+		}
+
 		/// ------------------------------------------------------------------------------------
 		/// <summary>
 		/// Tests the CopyTo method when we are copying from one reference sequence to another.

tests/SIL.LCModel.Tests/SIL.LCModel.Tests.csproj

@@ -13,7 +13,7 @@ This package provides unit tests for SIL.LCModel.</Description>
     <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.0.0" />
     <PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.0.0" PrivateAssets="All" />
     <PackageReference Include="NUnit" Version="3.13.3" />
-    <PackageReference Include="NUnit3TestAdapter" Version="4.5.0" />
+    <PackageReference Include="NUnit3TestAdapter" Version="4.3.2" />
     <PackageReference Include="RhinoMocks" Version="3.6.1" />
     <PackageReference Include="SIL.ReleaseTasks" Version="2.5.0" PrivateAssets="All" />
   </ItemGroup>

tests/SIL.LCModel.Utils.Tests/SIL.LCModel.Utils.Tests.csproj

@@ -1,7 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
-    <TargetFrameworks>net461</TargetFrameworks>
+    <TargetFramework>net461</TargetFramework>
     <RootNamespace>SIL.LCModel.Utils</RootNamespace>
     <Description>The liblcm library is the core FieldWorks model for linguistic analyses of languages. Tools in this library provide the ability to store and interact with language and culture data, including anthropological, text corpus, and linguistics data.
 This package provides unit tests for SIL.LCModel.Utils and test utility classes</Description>
@@ -14,7 +14,7 @@ This package provides unit tests for SIL.LCModel.Utils and test utility classes<
     <PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.0.0" PrivateAssets="All" />
     <PackageReference Include="Mono.Unix" Version="7.1.0-final.1.21458.1" />
     <PackageReference Include="NUnit" Version="3.13.3" />
-    <PackageReference Include="NUnit3TestAdapter" Version="4.5.0" />
+    <PackageReference Include="NUnit3TestAdapter" Version="4.3.2" />
     <PackageReference Include="SIL.ReleaseTasks" Version="2.5.0" PrivateAssets="All" />
   </ItemGroup>