NotificationsYou must be signed in to change notification settings
Fork7.2k
Star121k

[cmdpal] Migrate some plugin's unit tests from PT run to cmdpal.#40462

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Open

moooyo wants to merge16 commits intomain

base:main

Choose a base branch

fromyuleng/cmdpal/ut/timeDate-test

Open

[cmdpal] Migrate some plugin's unit tests from PT run to cmdpal.#40462

moooyo wants to merge16 commits intomainfromyuleng/cmdpal/ut/timeDate-test

Conversation

Copy link

Contributor

moooyo commentedJul 8, 2025•
edited
Loading

Summary of the Pull Request

Migrate blow plugin's from UT to cmdpal:

TimeDate
WindowWalker
System
Registry

This PR is mostly helped by Copilot. Please feel free to change cases in the future.

PR Checklist

Closes:Migrate PT run plugin's UT to CmdPal #40461
Communication: I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected
Tests: Added/updated and all pass
Localization: All end-user-facing strings can be localized
Dev docs: Added/updated
New binaries: Added on the required places
- JSON for signing for new binaries
- WXS for installer for new binaries and localization folder
- YML for CI pipeline for new test projects
- YML for signed pipeline
Documentation updated: If checked, please file a pull request onour docs repo and link it here: #xxx

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

Yu Leng added9 commits

July 4, 2025 17:37

init

cd06563

[Tests] Enhance date and time formatting tests and add query tests fo…

95dc1d5

…r cmdpal

update

0dc11a6

feat(tests): Migrate unit tests for CmdPal extensions

3bea027

- Migrated unit tests for four PowerToys launcher plugins (Registry, Calculator, WindowWalker, System) to the CmdPal extension framework.- Created new test projects with updated namespaces and structure.- Added comprehensive tests for each extension, ensuring functionality aligns with the new CmdPal architecture.- Updated project configurations to target .NET 9.0 and use MSTest framework with Moq for mocking.- Ensured all tests compile and run correctly in the new environment.

Add unit tests and improve project structure for cmdpal extensions

68de9bd

- Refactored ExtendedCalculatorParserTests to use SettingsManager for input validation.- Simplified BasicStructureTest and ensured proper test class structure.- Enhanced BasicTests with additional assertions for command and icon helpers.- Updated QueryTests to reflect changes in command properties and added placeholder tests for network commands.- Improved ImageTests to dynamically access icon properties using reflection.- Added InternalsVisibleTo attributes to allow unit tests access to internal members of extension projects.- Fixed project nesting in PowerToys.sln for better organization of unit test projects.- Documented changes and troubleshooting steps for Visual Studio project visibility issues.

refactor(tests): remove obsolete unit test files and project references

597c5f0

test: Refactor unit tests to improve structure and add new test cases

5e932a3

update

07589e4

update

6e87bef

moooyo requested a review fromhtcfreek

July 8, 2025 07:48

Yu Leng (from Dev Box) added2 commits

July 8, 2025 15:49

Remove ai docs

d649cf2

Revert sone changes

d468f90

moooyo self-assigned this

Jul 8, 2025

moooyo added the Product-Command PaletteRefers to the Command Palette utility label

Jul 8, 2025

moooyo added this to thePowerToys 0.93 milestone

Jul 8, 2025

moooyo marked this pull request as ready for review

July 8, 2025 07:56

crutkas changed the title~~[cmdpal] Migrate some plugin's UT from PT run to cmdpal.~~[cmdpal] Migrate some plugin's unit tests from PT run to cmdpal.

Jul 8, 2025

Copy link

ContributorAuthor

azure-pipelinesbot commentedJul 9, 2025

Azure Pipelines successfully started running 1 pipeline(s).

htcfreek suggested changes

Jul 9, 2025

View reviewed changes

...modules/cmdpal/ExtTests/Microsoft.CmdPal.Ext.TimeDate.UnitTests/FallbackTimeDateItemTests.csShow resolvedHide resolved

...modules/cmdpal/ExtTests/Microsoft.CmdPal.Ext.TimeDate.UnitTests/FallbackTimeDateItemTests.cs OutdatedShow resolvedHide resolved

src/modules/cmdpal/ExtTests/Microsoft.CmdPal.Ext.TimeDate.UnitTests/IconTests.csShow resolvedHide resolved

src/modules/cmdpal/ExtTests/Microsoft.CmdPal.Ext.TimeDate.UnitTests/QueryTests.cs Outdated

Comment on lines 247 to 248

		// [DataRow("ime", "", false)] // Partial match should not work
		// OK, we need to investigate why ime case does not work

Copy link

Collaborator

Choose a reason for hiding this comment

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

only shouldn't work for fallback

Copy link

ContributorAuthor

Choose a reason for hiding this comment

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

I did some investigate. The reason for this gap is in the cmdpal we assume all query is "keyword search" but for v1 is not.

I think we need to change the related logic? I don't know which one is "correct". any idea?

src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.TimeDate/Helpers/TimeAndDateHelper.cs

		@@ -298,6 +298,7 @@ internal static bool ParseStringAsDateTime(in string input, out DateTime timesta
		}
		else
		{
		inputParsingErrorMsg = Resources.Microsoft_plugin_timedate_InvalidInput_ErrorMessageTitle;

Copy link

Collaborator

Choose a reason for hiding this comment

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

why this chabge?

Copy link

ContributorAuthor

Choose a reason for hiding this comment

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

I see all other code path will have inputParsingErrorMsg if return false. Seems we miss it in the main branch?

src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.TimeDate/Helpers/AvailableResult.cs OutdatedShow resolvedHide resolved

src/modules/cmdpal/ExtTests/Microsoft.CmdPal.Ext.System.UnitTests/BasicTests.csShow resolvedHide resolved

yeelam-gordon added the Area-Testsissues that relate to tests label

Jul 10, 2025

yeelam-gordon requested a review fromCopilot

July 10, 2025 01:54

CopilotAI reviewed

Jul 10, 2025

View reviewed changes

Copy link

Contributor

CopilotAI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR migrates unit tests for the TimeDate, WindowWalker, System, and Registry plugins from PT run to the cmdpal framework, adds internal visibility for tests, and patches error messaging in the TimeAndDateHelper.

AddedInternalsVisibleTo attributes in each extension’s.csproj to allow unit tests to access internal members.
Inserted assignment toinputParsingErrorMsg inTimeAndDateHelper to populate the out parameter on parse failure.
Added or updated numerous test projects underExtTests/, along with ausing System; import inAvailableResult.cs.

Reviewed Changes

Copilot reviewed 31 out of 31 changed files in this pull request and generated no comments.

File	Description
Microsoft.CmdPal.Ext.*.csproj	Grant test assemblies internal visibility
TimeAndDateHelper.cs	Populate error message on parse failure
AvailableResult.cs	Added`using System;` import
PowerToys.sln	Registered new test projects in solution

Comments suppressed due to low confidence (6)

src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.TimeDate/Helpers/TimeAndDateHelper.cs:301

[nitpick] The resource key name uses underscores and lowercase segments; consider renaming it to follow PascalCase (e.g.,TimeDate_InvalidInput_ErrorMessageTitle) for consistency with other resource identifiers.

            inputParsingErrorMsg = Resources.Microsoft_plugin_timedate_InvalidInput_ErrorMessageTitle;

src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.WindowWalker/Microsoft.CmdPal.Ext.WindowWalker.csproj:50

If the extension assembly is strong-named, InternalsVisibleTo should include the test assembly’s public key token; without it, internal visibility may not be granted to the signed assembly.

    <AssemblyAttribute Include="System.Runtime.CompilerServices.InternalsVisibleTo">

src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.TimeDate/Microsoft.CmdPal.Ext.TimeDate.csproj:15

Consider specifying the public key token in the InternalsVisibleTo attribute if this assembly is signed, so that the test project can access internal members.

    <AssemblyAttribute Include="System.Runtime.CompilerServices.InternalsVisibleTo">

src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.System/Microsoft.CmdPal.Ext.System.csproj:37

When using InternalsVisibleTo on a strong-named assembly, include the public key token of the consuming test assembly to ensure proper internal access.

    <AssemblyAttribute Include="System.Runtime.CompilerServices.InternalsVisibleTo">

src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.Registry/Microsoft.CmdPal.Ext.Registry.csproj:15

Add the test assembly’s public key to the InternalsVisibleTo attribute if this assembly is strong-named; otherwise the visibility grant may fail at runtime.

    <AssemblyAttribute Include="System.Runtime.CompilerServices.InternalsVisibleTo">

src/modules/cmdpal/ext/Microsoft.CmdPal.Ext.TimeDate/Helpers/AvailableResult.cs:4

[nitpick] Theusing System; directive appears unused in this file; consider removing it to keep the imports clean.

using System;

Address some comments

291076f

github-advanced-securitybot found potential problems

Jul 10, 2025

View reviewed changes

src/modules/cmdpal/ExtTests/Microsoft.CmdPal.Ext.Registry.UnitTests/RegistryHelperTest.cs

		[TestMethod]
		public void GetRegistryBaseKeyTestMoreThanOneBaseKey()
		{
		var (baseKeyList, _) = RegistryHelper.GetRegistryBaseKey("HKC\\Control Panel\\Accessibility"); /* #no-spell-check-line */

Check warning

Code scanning / check-spelling

Candidate Pattern Warning

Line matches candidate pattern^.*/\* #no-spell-check-line */.*$ (candidate-pattern)

src/modules/cmdpal/ExtTests/Microsoft.CmdPal.Ext.Registry.UnitTests/RegistryHelperTest.cs

		[TestMethod]
		public void GetRegistryBaseKeyTestMoreThanOneBaseKey()
		{
		var (baseKeyList, _) = RegistryHelper.GetRegistryBaseKey("HKC\\Control Panel\\Accessibility"); /* #no-spell-check-line */

Check warning

Code scanning / check-spelling

Candidate Pattern Warning

Line matches candidate pattern^.*\\bno-spell-check(?:-line|)(?:\\s.*|)$ (candidate-pattern)

Yu Leng (from Dev Box) added4 commits

July 10, 2025 17:08

Improve some test case

f704f76

Remove one case

3a2dc77

Add new test cases

19bb41b

Remove one case

0b380a9

Copy link

Collaborator

htcfreek commentedJul 12, 2025

what about calculator plugin?

Copy link

ContributorAuthor

moooyo commentedJul 12, 2025 via email

leilei(our teammate) will take calculator one. I've discussed with him last week.

Copy link

ContributorAuthor

moooyo commentedJul 12, 2025

what about calculator plugin?

We will complete all plugin ut migration before next release. If possible.

Labels

Area-Tests

issues that relate to tests

Product-Command Palette

Refers to the Command Palette utility

3 participants

Movatterモバイル変換

[cmdpal] Migrate some plugin's unit tests from PT run to cmdpal.#40462

Are you sure you want to change the base?

[cmdpal] Migrate some plugin's unit tests from PT run to cmdpal.#40462

Uh oh!

Conversation

moooyo commentedJul 8, 2025• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Summary of the Pull Request

PR Checklist

Detailed Description of the Pull Request / Additional comments

Validation Steps Performed

Uh oh!

moooyo commentedJul 9, 2025

Uh oh!

azure-pipelinesbot commentedJul 9, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

htcfreekJul 9, 2025

Choose a reason for hiding this comment

Uh oh!

moooyoJul 10, 2025

Choose a reason for hiding this comment

Uh oh!

htcfreekJul 9, 2025

Choose a reason for hiding this comment

Uh oh!

moooyoJul 10, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

CopilotAI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Check warning

Check warning

htcfreek commentedJul 12, 2025

Uh oh!

moooyo commentedJul 12, 2025 via email

Uh oh!

moooyo commentedJul 12, 2025

Uh oh!

Uh oh!

moooyo commentedJul 8, 2025•
edited
Loading