run unit tests in CI against dotnet 8 (#306)

* change test project target framework to net8.0 from netstandard2.0

* update System.Configuration.ConfigurationManager dependency to 6 to match a conflict with System.Diagnostics.PerformanceCounter

* reworked path validation to use Path.GetInvalidPathChars and Path.GetInvalidFileNameChars

* explicitly specify the hash algorithm to use when comparing files as dotnet does not define a default algorithm

* Core tests, add reference to TestHelper project so that it gets copied to the output directory

* remove redundant `/` in output path. Ensure there's always a configuration set so the output path is consistent.

* add a net8.0 target framework to SIL.LCModel.Core.csproj, this ensures that IcuData will be copied to the output directory for dotnet 8 projects that depend on SIL.LCModel.Core

* ensure all TestHelper.exe files are copied to the output directory, including Newtonsoft.Json.dll. Use TestContext.Out instead of Console.WriteLine. Fix assertion in CustomIcuFallbackTests.InitIcuDataDir_NoIcuLibrary due to how the error is different between .NET Framework and .NET Core.

* use dotnet 8 in CI

* inline RhinoMocksToMoq dependency this avoids the strong name issues with the NuGet package.

* change FindOrCreateFile to use the path validation helper as dotnet 8 tests were failing because '<' is valid in a path but not a file name

* add special case for paths with : in the folder name

* fix an edge case in FileUtils.IsFilePathValid where there's no directory

* upgrade to SIL.WritingSystems 14.2.0 beta 21 to fix issue with CollatorSort_DoesNotThrow failing on dotnet 8

* disable spelling tests when running dotnet 8 on linux

* make artifact name unique per OS

---------

Co-authored-by: Jake Oliver <[email protected]>

.github/workflows/ci-cd.yml

@@ -28,9 +28,9 @@ jobs:
 
     # Install the .NET Core workload
     - name: Install .NET Core
-      uses: actions/setup-dotnet@v2
+      uses: actions/setup-dotnet@v4
       with:
-        dotnet-version: 6.0.x
+        dotnet-version: 8.0.x
 
     - name: Install mono-devel package
       run: sudo apt-get install mono-devel
@@ -67,11 +67,11 @@ jobs:
     - name: Test on Linux
       run: |
         . environ
-        dotnet test --no-restore --no-build -p:ParallelizeAssembly=false --configuration Release --logger:"trx" --results-directory ./test-results
+        dotnet test --no-restore --no-build -p:ParallelizeAssembly=false --configuration Release --logger:"trx;LogFilePrefix=results" --results-directory ./test-results
       if: matrix.os == 'ubuntu-latest'
 
     - name: Test on Windows
-      run: dotnet test --no-restore --no-build -p:ParallelizeAssembly=false --configuration Release --logger:"trx" --results-directory ./test-results
+      run: dotnet test --no-restore --no-build -p:ParallelizeAssembly=false --configuration Release --logger:"trx;LogFilePrefix=results" --results-directory ./test-results
       if: matrix.os != 'ubuntu-latest'
     - name: Upload test results
       if: always()
@@ -94,7 +94,7 @@ jobs:
     - name: Publish Artifacts
       uses: actions/upload-artifact@v4
       with:
-        name: NugetPackages
+        name: NugetPackages (${{ matrix.os }})
         path: artifacts/*.nupkg
       if: github.event_name == 'pull_request'
   publish-test-results:

Directory.Build.props

@@ -11,7 +11,8 @@
     <PackageProjectUrl>https://github.com/sillsdev/liblcm</PackageProjectUrl>
     <PackageRequireLicenseAcceptance>false</PackageRequireLicenseAcceptance>
     <Platform>Any CPU</Platform>
-    <OutputPath>$(MSBuildThisFileDirectory)/artifacts/$(Configuration)/$(TargetFramework)</OutputPath>
+	<Configuration Condition="'$(Configuration)' == ''">Debug</Configuration>
+    <OutputPath>$(MSBuildThisFileDirectory)artifacts/$(Configuration)/$(TargetFramework)</OutputPath>
     <AppendTargetFrameworkToOutputPath>false</AppendTargetFrameworkToOutputPath>
     <PackageOutputPath>$(MSBuildThisFileDirectory)/artifacts</PackageOutputPath>
     <SignAssembly>true</SignAssembly>

src/SIL.LCModel.Core/SIL.LCModel.Core.csproj

@@ -1,24 +1,24 @@
 <Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
-    <TargetFrameworks>netstandard2.0;net461</TargetFrameworks>
+    <TargetFrameworks>netstandard2.0;net461;net8.0</TargetFrameworks>
     <RootNamespace>SIL.LCModel.Core</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.
 SIL.LCModel.Core provides a base library with core functionality.</Description>
   </PropertyGroup>
 
   <ItemGroup>
     <PackageReference Include="GitVersion.MsBuild" Version="5.6.10" PrivateAssets="All" />
-    <PackageReference Include="icu.net" Version="2.*-*" />
+    <PackageReference Include="icu.net" Version="3.0.0-*" />
     <PackageReference Include="Icu4c.Win.Fw.Bin" Version="70.1.152" IncludeAssets="build" />
     <PackageReference Include="Icu4c.Win.Fw.Lib" Version="70.1.152" IncludeAssets="build" PrivateAssets="runtime" />
     <PackageReference Include="Microsoft.DotNet.PlatformAbstractions" Version="2.0.4" />
     <PackageReference Include="Microsoft.Extensions.DependencyModel" Version="2.0.4" />
     <PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.0.0" PrivateAssets="All" />
     <PackageReference Include="NHunspell" Version="1.2.5554.16953" GeneratePropertyPath="true" />
-    <PackageReference Include="SIL.Lexicon" Version="12.0.0-*" />
+    <PackageReference Include="SIL.Lexicon" Version="14.2.0-*" />
     <PackageReference Include="SIL.ReleaseTasks" Version="2.5.0" PrivateAssets="All" />
-    <PackageReference Include="SIL.WritingSystems" Version="12.0.0-*" />
+    <PackageReference Include="SIL.WritingSystems" Version="14.2.0-*" />
     <PackageReference Include="System.Drawing.Common" Version="6.0.0" />
     <PackageReference Include="System.Runtime.InteropServices.RuntimeInformation" Version="4.3.0" />
     <PackageReference Include="vswhere" Version="2.8.4" PrivateAssets="all" />
@@ -163,12 +163,12 @@ SIL.LCModel.Core provides a base library with core functionality.</Description>
   <Target Name="BeforeCoreClean" BeforeTargets="CoreClean">
     <!-- Prevent the deletion of IDLImporter.xml. This seems like a bug in msbuild - it shouldn't
     delete files it didn't copy, at least not in the middle of a rebuild -->
-    <Copy SourceFiles="$(OutputPath)/IDLImporter.xml" DestinationFiles="$(OutputPath)/../IDLImporter.xml" Condition="Exists('$(OutputPath)/IDLImporter.xml')" />
+    <Copy SourceFiles="$(OutputPath)IDLImporter.xml" DestinationFiles="$(OutputPath)../IDLImporter.xml" Condition="Exists('$(OutputPath)IDLImporter.xml')" />
   </Target>
 
   <Target Name="AfterCoreClean" AfterTargets="CoreClean">
     <!-- Restore IDLImporter.xml. -->
-    <Copy SourceFiles="$(OutputPath)/../IDLImporter.xml" DestinationFiles="$(OutputPath)/IDLImporter.xml" />
+    <Copy SourceFiles="$(OutputPath)../IDLImporter.xml" DestinationFiles="$(OutputPath)IDLImporter.xml" Condition="Exists('$(OutputPath)../IDLImporter.xml')" />
   </Target>
 
   <Target Name="CollectRuntimeOutputs" BeforeTargets="_GetPackageFiles">

src/SIL.LCModel.Utils/FileUtils.cs

@@ -319,7 +319,7 @@ public static bool AreFilesIdentical(string file1, string file2)
 			byte[] fileHash2;
 
 			// compute the hashes for comparison
-			using(var hash = HashAlgorithm.Create())
+			using(var hash = HashAlgorithm.Create("SHA1"))
 			using(Stream fileStream1 = OpenStreamForRead(file1),
 							 fileStream2 = OpenStreamForRead(file2))
 			{
@@ -473,16 +473,8 @@ public static string ActualFilePath(string sPathname)
 		/// ------------------------------------------------------------------------------------
 		public static void AssertValidFilePath(string filename)
 		{
-			try
-			{
-				new FileInfo(filename);
-			}
-			catch (SecurityException)
-			{
-			}
-			catch (UnauthorizedAccessException)
-			{
-			}
+			if (!IsFilePathValid(filename))
+				throw new ArgumentException("Illegal characters in path.");
 		}
 
 		/// ------------------------------------------------------------------------------------
@@ -492,11 +484,34 @@ public static void AssertValidFilePath(string filename)
 		/// ------------------------------------------------------------------------------------
 		public static bool IsFilePathValid(string filename)
 		{
+			if (filename == null)
+				return false;
 			try
 			{
-				AssertValidFilePath(filename);
+				// will throw in .net framework, but not newer versions
+				_ = Path.GetFullPath(filename);
+				var name = Path.GetFileName(filename);
+				var invalidFileNameChars = Path.GetInvalidFileNameChars();
+				if (name.IndexOfAny(invalidFileNameChars) >= 0)
+					return false;
+				if (name == filename) return true;
+				var directoryPath = filename.Substring(0, filename.Length - name.Length);
+				if (directoryPath.IndexOfAny(Path.GetInvalidPathChars()) >= 0)
+					return false;
+				if (Platform.IsWindows)
+				{
+					// some paths like "C:\bla" are valid, but not C\:bla, we want to catch those by excluding the drive letter and checking for invalid file name chars in each directory
+
+					//trim off the drive letter if it exists
+					directoryPath = directoryPath.Substring(Path.GetPathRoot(directoryPath).Length);
+					//each directory must be a valid file name. Using both / and \ because it could be a mixed path which usually works fine
+					if (directoryPath.Split('\\', '/').Any(dir => dir.IndexOfAny(invalidFileNameChars) >= 0))
+					{
+						return false;
+					}
+				}
 			}
-			catch (Exception)
+			catch
 			{
 				return false;
 			}
@@ -936,7 +951,7 @@ public static string ChangeWindowsPathIfLinux(string windowsPath)
 		public static string ChangeWindowsPathIfLinuxPreservingPrefix(string windowsPath,
 			string prefix)
 		{
-			if (windowsPath == null || prefix == null || !windowsPath.StartsWith(prefix))
+			if (windowsPath == null || prefix == null || !windowsPath.StartsWith(prefix, StringComparison.Ordinal))
 				return ChangeWindowsPathIfLinux(windowsPath);
 			// Preserve prefix
 			windowsPath = windowsPath.Substring(prefix.Length);
@@ -975,7 +990,7 @@ public static string ChangeLinuxPathIfWindows(string linuxPath)
 		public static string ChangeLinuxPathIfWindowsPreservingPrefix(string linuxPath,
 			string prefix)
 		{
-			if (linuxPath == null || prefix == null || !linuxPath.StartsWith(prefix))
+			if (linuxPath == null || prefix == null || !linuxPath.StartsWith(prefix, StringComparison.Ordinal))
 				return ChangeLinuxPathIfWindows(linuxPath);
 			// Preserve prefix
 			linuxPath = linuxPath.Substring(prefix.Length);

src/SIL.LCModel.Utils/SIL.LCModel.Utils.csproj

@@ -10,11 +10,11 @@ SIL.LCModel.Utils provides utility classes.</Description>
   <ItemGroup>
     <PackageReference Include="GitVersion.MsBuild" Version="5.6.10" PrivateAssets="All" />
     <PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.0.0" PrivateAssets="All" />
-    <PackageReference Include="SIL.Core" Version="12.0.0-*" />
+    <PackageReference Include="SIL.Core" Version="14.2.0-*" />
     <PackageReference Include="SIL.ReleaseTasks" Version="2.5.0" PrivateAssets="All" />
     <PackageReference Include="Mono.Unix" Version="7.1.0-final.1.21458.1" />
     <PackageReference Include="Newtonsoft.Json" Version="13.0.2" />
-    <PackageReference Include="System.Configuration.ConfigurationManager" Version="5.0.0" />
+    <PackageReference Include="System.Configuration.ConfigurationManager" Version="6.0.0" />
     <PackageReference Include="System.Diagnostics.PerformanceCounter" Version="6.0.0" />
     <PackageReference Include="System.Management" Version="6.0.0" />
     <Reference Include="System.Management" />

src/SIL.LCModel/DomainServices/DomainObjectServices.cs

@@ -2298,9 +2298,7 @@ public static ICmFile FindOrCreateFile(ICmFolder folder, string srcFile)
 			if (String.IsNullOrEmpty(srcFile))
 				throw new ArgumentException("File path not specified.", "srcFile");
 
-			char[] bad = Path.GetInvalidPathChars();
-			int idx = srcFile.IndexOfAny(bad);
-			if (idx >= 0)
+			if (!FileUtils.IsFilePathValid(srcFile))
 				throw new ArgumentException("File path (" + srcFile + ") contains at least one invalid character.", "srcFile");
 
 			foreach (ICmFile file in folder.FilesOC)

src/SIL.LCModel/LinkedFilesRelativePathHelper.cs

@@ -274,6 +274,8 @@ private static string GetPathWithLowercaseRoot(string path)
 			try
 			{
 				var rootOfPath = Path.GetPathRoot(path);
+				// dotnet 8 on linux, when path contains invalid characters rootOfPath will be null
+				if (rootOfPath is null) return path.ToLowerInvariant();
 				return rootOfPath.ToLowerInvariant()
 					+ path.Substring(rootOfPath.Length, path.Length - rootOfPath.Length);
 			}

src/SIL.LCModel/SIL.LCModel.csproj

@@ -22,9 +22,9 @@
     <PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.0.0" PrivateAssets="All" />
     <PackageReference Include="protobuf-net" Version="2.4.6" />
     <PackageReference Include="SharpZipLib" Version="1.4.0" />
-    <PackageReference Include="SIL.Lexicon" Version="12.0.0-*" />
+    <PackageReference Include="SIL.Lexicon" Version="14.2.0-*" />
     <PackageReference Include="SIL.ReleaseTasks" Version="2.5.0" PrivateAssets="All" />
-    <PackageReference Include="SIL.WritingSystems" Version="12.0.0-*" />
+    <PackageReference Include="SIL.WritingSystems" Version="14.2.0-*" />
     <PackageReference Include="structuremap.patched" Version="4.7.3" />
     <PackageReference Include="System.ServiceProcess.ServiceController" Version="6.0.0" />
   </ItemGroup>

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

@@ -1,7 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
-    <TargetFrameworks>net461;netstandard2.0</TargetFrameworks>
+    <TargetFrameworks>net461;net8.0</TargetFrameworks>
     <RootNamespace>SIL.LCModel.Core</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.Core.</Description>
@@ -16,12 +16,13 @@ This package provides unit tests for SIL.LCModel.Core.</Description>
     <PackageReference Include="NUnit" Version="3.13.3" />
     <PackageReference Include="NUnit3TestAdapter" Version="4.3.2" />
     <PackageReference Include="SIL.ReleaseTasks" Version="2.5.0" PrivateAssets="All" />
-    <PackageReference Include="SIL.TestUtilities" Version="12.0.0-*" />
+    <PackageReference Include="SIL.TestUtilities" Version="14.2.0-*" />
   </ItemGroup>
 
   <ItemGroup>
     <ProjectReference Include="..\..\src\SIL.LCModel.Core\SIL.LCModel.Core.csproj" />
     <ProjectReference Include="..\SIL.LCModel.Utils.Tests\SIL.LCModel.Utils.Tests.csproj" />
+    <ProjectReference Include="..\TestHelper\TestHelper.csproj" />
   </ItemGroup>
 
   <ItemGroup>

tests/SIL.LCModel.Core.Tests/SpellChecking/SpellingHelperTests.cs

@@ -5,6 +5,7 @@
 using System;
 using System.IO;
 using System.Linq;
+using System.Runtime.InteropServices;
 using System.Text;
 using NUnit.Framework;
 using SIL.IO;
@@ -22,6 +23,17 @@ public class SpellingHelperTests
 	{
 		// TODO-Linux: need slightly modified hunspell package installed!
 
+		[OneTimeSetUp]
+		public void FixtureSetUp()
+		{
+			#if NET8_0
+			if (!RuntimeInformation.IsOSPlatform(OSPlatform.Windows))
+			{
+				Assert.Ignore("NHunspell does not work on dotnet 8 on linux");
+			}
+			#endif
+		}
+
 		/// <summary>
 		/// Check how spelling status is set and cleared.
 		/// </summary>

tests/SIL.LCModel.Core.Tests/Text/CustomIcuFallbackTests.cs

@@ -90,8 +90,8 @@ private string RunTestHelper(string workDir, out string stdErr, bool expectFailu
 				if (process.ExitCode != 0)
 				{
 					var expected = expectFailure ? "expected" : "unexpected";
-					Console.WriteLine($"TestHelper.exe failed ({expected}):");
-					Console.WriteLine(stdErr);
+					TestContext.Out.WriteLine($"TestHelper.exe failed ({expected}):");
+					TestContext.Out.WriteLine(stdErr);
 				}
 				return output.TrimEnd('\r', '\n');
 			}
@@ -111,10 +111,10 @@ private static void CopyIcuFiles(string targetDir, string icuVersion)
 
 		private static void CopyTestFiles(string sourceDir, string targetDir)
 		{
-			var testHelper = Path.Combine(sourceDir, "TestHelper.exe");
-			if (!File.Exists(testHelper))
-				testHelper = Path.Combine(sourceDir, "TestHelper.dll");
-			CopyFile(testHelper, targetDir);
+			foreach (var file in Directory.EnumerateFiles(sourceDir, "TestHelper.*"))
+			{
+				CopyFile(file, targetDir);
+			}
 			var targetIcuDataDir = Path.Combine(targetDir, "IcuData");
 			Directory.CreateDirectory(targetIcuDataDir);
 			DirectoryHelper.Copy(Path.Combine(sourceDir, "IcuData"), targetIcuDataDir);
@@ -126,7 +126,8 @@ private static void CopyTestFiles(string sourceDir, string targetDir)
 				"SIL.LCModel.Utils",
 				"icu.net",
 				"SIL.Core",
-				"Microsoft.Extensions.DependencyModel"
+				"Microsoft.Extensions.DependencyModel",
+				"Newtonsoft.Json"
 			})
 			{
 				var sourceFile = Path.Combine(sourceDir, $"{file}.dll");
@@ -222,21 +223,31 @@ private static void PrintIcuDllsOnPath()
 				}
 				catch (Exception e)
 				{
-					Console.WriteLine($"Error enumerating: {e.GetType()}: {e.Message}");
+					TestContext.Out.WriteLine($"Error enumerating: {e.GetType()}: {e.Message}");
 				}
 			}
 			if (files.Any())
 			{
-				Console.WriteLine($"Found the following ICU DLL's lurking around:\r\n{string.Join("\r\n", files)}");
-				Console.WriteLine("(note: DLL's without a version number in the name should not be a problem)");
+				TestContext.Out.WriteLine($"Found the following ICU DLL's lurking around:\r\n{string.Join("\r\n", files)}");
+				TestContext.Out.WriteLine("(note: DLL's without a version number in the name should not be a problem)");
+			}
+			else
+			{
+				TestContext.Out.WriteLine("No ICU DLL's found");
 			}
 		}
 
 		[Test]
 		public void InitIcuDataDir_NoIcuLibrary()
 		{
-			Assert.That(RunTestHelper(_tmpDir, out var stdErr, true), Is.Empty);
-			Assert.That(stdErr, Does.Contain("Unhandled Exception: System.IO.FileLoadException: Can't load ICU library (version 0)"));
+			var result = RunTestHelper(_tmpDir, out var stdErr, true);
+			if (!string.IsNullOrEmpty(result))
+			{
+				PrintIcuDllsOnPath();
+			}
+
+			Assert.That(result, Is.Empty);
+			Assert.That(stdErr, Does.Contain("System.IO.FileLoadException: Can't load ICU library (version 0)"));
 		}
 	}
 }

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

@@ -1,7 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
-    <TargetFrameworks>net461;netstandard2.0</TargetFrameworks>
+    <TargetFrameworks>net461;net8.0</TargetFrameworks>
     <RootNamespace>SIL.LCModel.FixData</RootNamespace>
     <IsPackable>false</IsPackable>
   </PropertyGroup>

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

@@ -252,7 +252,7 @@ public void CmPictureConstructor_FromTextRep_MissingCmPictureToken()
 		public void CmPictureConstructor_FromTextRep_MissingFilename()
 		{
 			Assert.That(() => m_pictureFactory.Create("CmPicture||||||This is a caption||",
-				CmFolderTags.LocalPictures), Throws.ArgumentException.With.Property("Message").Matches("File path not specified.(\\r)?\\nParameter( )?name: srcFile"));
+				CmFolderTags.LocalPictures), Throws.ArgumentException.With.Message.Contains("File path not specified"));
 		}
 
 		/// -------------------------------------------------------------------------------------

tests/SIL.LCModel.Tests/DomainServices/DataStoreInitializationServicesTests.cs

@@ -5,13 +5,15 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using Moq;
 using NUnit.Framework;
 using Rhino.Mocks;
 using SIL.LCModel.Core.KernelInterfaces;
 using SIL.LCModel.Core.Scripture;
 using SIL.LCModel.Core.Text;
 using SIL.LCModel.DomainImpl;
 using SIL.LCModel.Utils;
+using MockRepository = Rhino.Mocks.MockRepository;
 
 namespace SIL.LCModel.DomainServices
 {
@@ -317,7 +319,7 @@ public override void TestSetup()
 			m_para.Stub(p => p.Id).Return(paraId);
 			m_para.Contents = TsStringUtils.EmptyString(Cache.DefaultVernWs);
 
-			GetBtDelegate getBtDelegate = () =>
+			Func<ICmTranslation> getBtDelegate = () =>
 				m_para.TranslationsOC.FirstOrDefault(trans => trans.TypeRA != null &&
 					trans.TypeRA.Guid == CmPossibilityTags.kguidTranBackTranslation);
 			m_para.Stub(p => p.GetBT()).Do(getBtDelegate);