Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

.NET8 Windows for SIL.Windows.Forms #1358

Merged
merged 13 commits into from
Jan 23, 2025
Merged

Conversation

josephmyers
Copy link
Collaborator

@josephmyers josephmyers commented Nov 1, 2024

Updated L10NSharp to 8, which is the version that adds support for .NET8 Windows.

Removed some old app.config settings that were causing build warnings.

Updated some of the old .NET menu items for the newer style MenuStrip/MenuItem style.

Also removed the old CAS-style PermissionSet attributes. This has precedent in FLEx, I believe.

Also removed ImageToolbox controls -- they appear to be unused within SIL, and the third-party dependency is Framework 4.6.1 only.

The other ancient third-party dependency we have is Enchant, which does appear to be used. But I have marked those two direct references as Obsolete, so we should see build warnings for consumers.


This change is Reviewable

Copy link

github-actions bot commented Nov 1, 2024

LibPalaso Tests

    46 files  +  1      46 suites  +1   10m 49s ⏱️ +17s
 4 823 tests  -   5   4 596 ✅ +  3  226 💤  - 5  1 ❌  - 3 
14 243 runs  +241  13 607 ✅ +236  633 💤 +6  3 ❌  - 1 

For more details on these failures, see this check.

Results for commit 93206a7. ± Comparison against base commit 46fa5f0.

This pull request removes 6 and adds 1 tests. Note that renamed tests count towards both.
SIL.Windows.Forms.Tests.ImageToolbox.ImageToolboxTests ‑ CheckMonoForSelectLargeIconView
SIL.Windows.Forms.Tests.ImageToolbox.ImageToolboxTests ‑ DoubleCheckFileFilterWorks
SIL.Windows.Forms.Tests.ImageToolbox.ImageToolboxTests ‑ ShowGeckoToolbox
SIL.Windows.Forms.Tests.ImageToolbox.ImageToolboxTests ‑ ShowToolbox
SIL.Windows.Forms.Tests.ImageToolbox.ImageToolboxTests ‑ ShowToolboxWith_PreExisting_EnsureRawFormatUnchanged
SIL.Windows.Forms.Tests.ImageToolbox.ImageToolboxTests ‑ ShowToolboxWith_PreExisting_Image_WithMetadata
SIL.Windows.Forms.Tests.SettingsProviderTests ‑ Upgrade_ExtraFields_SettingsMigrated

♻️ This comment has been updated with latest results.

Updated L10NSharp to 8, which is the version that adds support for .NET8 Windows.

Removed some old app.config settings that were causing build warnings.

Updated some of the old .NET menu items for the newer style MenuStrip/MenuItem style.

Also removed the old CAS-style PermissionSet attributes. This has precedent in FLEx, I believe.

Also removed ImageToolbox controls -- they appear to be unused within SIL, and the third-party dependency is Framework 4.6.1 only.

The other ancient third-party dependency we have is Enchant, which does appear to be used. But I have marked those two direct references as Obsolete, so we should see build warnings for consumers.
….Windows.Forms like all the others, but it doesn't.
With .NET8, the LocalApplicationData property has a different value than with Framework, causing a test failure in this class. That test ended reading the temp file from a previous test. I refactored the file pathing and directory creation to be adapted from the SettingsProvider itself, which should make it more flexible.
@josephmyers josephmyers force-pushed the net8windows/sil-windows-forms branch from 93206a7 to ba9a4c5 Compare November 12, 2024 06:28
Copy link

github-actions bot commented Nov 12, 2024

Palaso Tests

     4 files  +  1       4 suites  +1   16m 59s ⏱️ + 1m 26s
 4 880 tests + 56   4 652 ✅ + 59  228 💤  -  3  0 ❌ ±0 
14 372 runs  +376  13 735 ✅ +366  637 💤 +10  0 ❌ ±0 

Results for commit 3591b45. ± Comparison against base commit c06c991.

This pull request removes 9 and adds 65 tests. Note that renamed tests count towards both.
SIL.Tests.IO.FileLocatorTests ‑ GetFromRegistryProgramThatOpensFileType_SendExtensionWithoutPeriod_ReturnsProgramPath
SIL.Tests.IO.FileLocatorTests ‑ GetFromRegistryProgramThatOpensFileType_SendInvalidType_ReturnsNull
SIL.Tests.IO.FileLocatorTests ‑ GetFromRegistryProgramThatOpensFileType_SendValidType_ReturnsProgramPath
SIL.Windows.Forms.Tests.ImageToolbox.ImageToolboxTests ‑ CheckMonoForSelectLargeIconView
SIL.Windows.Forms.Tests.ImageToolbox.ImageToolboxTests ‑ DoubleCheckFileFilterWorks
SIL.Windows.Forms.Tests.ImageToolbox.ImageToolboxTests ‑ ShowGeckoToolbox
SIL.Windows.Forms.Tests.ImageToolbox.ImageToolboxTests ‑ ShowToolbox
SIL.Windows.Forms.Tests.ImageToolbox.ImageToolboxTests ‑ ShowToolboxWith_PreExisting_EnsureRawFormatUnchanged
SIL.Windows.Forms.Tests.ImageToolbox.ImageToolboxTests ‑ ShowToolboxWith_PreExisting_Image_WithMetadata
SIL.Media.Tests.AudioFactoryTests ‑ Construct_FileDoesExistButEmpty_OK
SIL.Media.Tests.AudioFactoryTests ‑ Construct_FileDoesNotExist_DoesNotCreateFile
SIL.Media.Tests.AudioFactoryTests ‑ Construct_FileDoesNotExist_OK
SIL.Media.Tests.AudioPlayerTests ‑ LoadFile_ThenDispose_FileCanBeDeleted
SIL.Media.Tests.AudioPlayerTests ‑ PlayFile_ThenDispose_FileCanBeDeleted
SIL.Media.Tests.AudioSessionTests ‑ CanPlay_WhilePlaying_False
SIL.Media.Tests.AudioSessionTests ‑ CanRecord_ConstructWithEmptyFile_True
SIL.Media.Tests.AudioSessionTests ‑ CanRecord_FileDoesNotExist_True
SIL.Media.Tests.AudioSessionTests ‑ CanStop_NonExistentFile_False
SIL.Media.Tests.AudioSessionTests ‑ CanStop_WhilePlaying_True
…
This pull request removes 5 skipped tests and adds 2 skipped tests. Note that renamed tests count towards both.
SIL.Windows.Forms.Tests.ImageToolbox.ImageToolboxTests ‑ CheckMonoForSelectLargeIconView
SIL.Windows.Forms.Tests.ImageToolbox.ImageToolboxTests ‑ ShowGeckoToolbox
SIL.Windows.Forms.Tests.ImageToolbox.ImageToolboxTests ‑ ShowToolbox
SIL.Windows.Forms.Tests.ImageToolbox.ImageToolboxTests ‑ ShowToolboxWith_PreExisting_EnsureRawFormatUnchanged
SIL.Windows.Forms.Tests.ImageToolbox.ImageToolboxTests ‑ ShowToolboxWith_PreExisting_Image_WithMetadata
SIL.Media.Tests.AudioPlayerTests ‑ PlayFile_ThenDispose_FileCanBeDeleted
SIL.Media.Tests.AudioSessionTests ‑ Record_LongRecording

♻️ This comment has been updated with latest results.

…nstant string.

This was necessary to be more explicit in the test what is actually being fixed, while avoiding .Codebase, which produces a warning in modern .NET.
@josephmyers josephmyers marked this pull request as ready for review November 12, 2024 07:27
Also marked its test class with the same. Also marked another test class Windows-only, though it doesn't have to be Windows-only (just how it was written). This attribute is only supported with .NET5 and later, so on Framework there will be no marking.

These changes fix a large number of build warnings.
Copy link
Contributor

@hahn-kev hahn-kev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one minor issue, otherwise it looks good

@josephmyers
Copy link
Collaborator Author

@hahn-kev I'm seeing the dotnet test command not picking up the tests in the net8-windows folder. It should be. I'm 99% sure the path is right, since it's working for all the other platforms. I confirmed that the build log indicates it is building to the expected net8-windows output folder, so there's nothing that jumps off the page at me on why this isn't working.

Versions between the server and my local are similar, but:
Server: VSTest version 17.12.0 (x64)
Local: Microsoft (R) Test Execution Command Line Tool Version 17.9.0 (x64)

I don't think that would matter?

@josephmyers josephmyers merged commit e3f8d8d into master Jan 23, 2025
4 of 5 checks passed
@josephmyers josephmyers deleted the net8windows/sil-windows-forms branch January 23, 2025 07:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants