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

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

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

     3 files  ± 0       3 suites  ±0   15m 48s ⏱️ +15s
 4 832 tests + 8   4 606 ✅ +13  226 💤  -  5  0 ❌ ±0 
14 012 runs  +16  13 395 ✅ +26  617 💤  - 10  0 ❌ ±0 

Results for commit f4259bc. ± Comparison against base commit c06c991.

This pull request removes 9 and adds 17 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.Tests.IO.FileLocatorTests ‑ GetDefaultProgramForFileType_SendExtensionWithoutPeriod_ReturnsProgramPath
SIL.Tests.IO.FileLocatorTests ‑ GetDefaultProgramForFileType_SendInvalidType_ReturnsNull
SIL.Tests.IO.FileLocatorTests ‑ GetDefaultProgramForFileType_SendValidType_ReturnsProgramPath
SIL.Windows.Forms.Tests.Hotspot.HotSpotProviderTestForms ‑ DisableHotSpotProvider_MouseEventsNoLongerFired
SIL.Windows.Forms.Tests.Hotspot.HotSpotProviderTestForms ‑ HotSpotMouseClick_MouseClickOverHotSpot_FiresMouseClick
SIL.Windows.Forms.Tests.Hotspot.HotSpotProviderTestForms ‑ HotSpotMouseDown_MouseDownOverHotSpot_FiresMouseDown
SIL.Windows.Forms.Tests.Hotspot.HotSpotProviderTestForms ‑ HotSpotMouseEnter_MouseNotOverHotSpot_DoesNotFireMouseEnter
SIL.Windows.Forms.Tests.Hotspot.HotSpotProviderTestForms ‑ HotSpotMouseEnter_MouseOverHotSpotThenOffControlThenBackOverSameHotSpot_FiresMouseEnter
SIL.Windows.Forms.Tests.Hotspot.HotSpotProviderTestForms ‑ HotSpotMouseEnter_MouseOverHotSpot_FiresMouseEnter
SIL.Windows.Forms.Tests.Hotspot.HotSpotProviderTestForms ‑ HotSpotMouseHover_MouseHoverNotOverHotSpot_DoesNotFireMouseHover
…

♻️ 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?

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