Added Framework 4.8 to all projects #1326
Conversation
LibPalaso Tests 35 files + 17 35 suites +17 10m 12s ⏱️ + 1m 58s Results for commit 6730607. ± Comparison against base commit 8220e84. This pull request removes 68 tests.♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
I left a question about a change.
Also, I figured out why dotnet test wasn't working and you had to pass in a list of dlls. It's because each test package needs to reference <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.0.0" />. However when I did that I started getting some test failures. That said they seem to be due to some weird reference issues. If I start clean and use dotnet test then I get failures. However if I run a dotnet build first, then dotnet test works and the tests pass. So there's something weird going on. I'd say we would like to be able to use dotnet test without anything else, so we should look into that.
josephmyers
force-pushed
the
net48
branch
2 times, most recently
from
456839c
to
175c41e
Compare
|
SIL.WritingSystems.Tests takes a ridiculous amount of time to finish. It's 8-10 minutes on my machine, and enough on the build server to time out. This is exacerbated by increasing our supported frameworks. Seems an appropriate time to look into how we can get those tests speedier. |
|
The majority of the pain seems to be coming from the SldrTests. We could just not run these on the build server. |
|
4a5ad40 succeeded in 10 minutes. Why did the previous time out at 30? |
|
@josephmyers I was seeing some tests hang when working on my writing systems tests, there's now a timeout in dotnet test to detect that so if it happens you should have some details about it at least, this is now in master. |
There was a problem hiding this comment.
oops, it looks like the following projects actually override TargetFrameworks meaning they don't pickup net48 from Directory.Build.props
- AddSortKey
- SIL.Core.Desktop
- SIL.Core
- SIL.DblBundle
- SIL.DictionaryServices
- SIL.Lexicon
- SIL.Lift
- SIL.Linux.Logging
- SIL.Scripture
- SIL.TestUtilities
- SIL.WritingSystems
Good catch. So why is the test run doubled? Looks to me like it's running everything, though based on the code, it's likely not LibPalaso Tests Results for commit https://github.com/sillsdev/libpalaso/commit/d51226f79643393b134126283aa26325aabef746. ± Comparison against base commit 269c813. |
And fixed all warnings other than those warning about Framework 4.6.1.
Should fix test failures on server
It was requiring an error thrown by .NET code to have a very specific message, where, with later versions, a different error is thrown. The newer message is still related to the original, dealing with the path, and the updated test captures the core intent without being so picky.
.NET behavior changed with 4.6.2. See here for more details: https://learn.microsoft.com/th-th/dotnet/framework/migration-guide/mitigation-path-normalization 4.6.1 DirectoryInfo apparently strips ... (and maybe similar tokens, too), particularly for its FullName property, which is being used here. This changed in 4.6.2. Assuming that the test indicates a desire for support, the simplest thing to do is to strip dots for comparison. I also experimented with "Switch.System.IO.UseLegacyPathHandling=true" in the app.config, but this had no discernible impact and generally seems ill-advised.
A couple are still being executed on the server
And fixed all warnings other than those warning about Framework 4.6.1.
This change is