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

Clarify No installed package matching the input criteria was found in upgrade flow #2877

Merged
merged 24 commits into from
Mar 11, 2023

Conversation

Trenly
Copy link
Contributor

@Trenly Trenly commented Jan 25, 2023

It seems to work, I am sorry. I was wondering about the message: "No installed package matching the input criteria was found". So I thought my installed software wasnt recognized by winget. I expected something like "No updates available" or something else.

This change provides a separate message when there are no matching search results found. In order to keep things clean, I switched the boolean parameter to an enum to allow for more fine-grained control over the messaging and to keep it extensible.

I also decided to not use the existing No applicable upgrade found message, as there is a difference between an upgrade being available and an upgrade being applicable. The message for applicability is still used when an upgrade is available, but an install attempt finds no upgrade that is applicable.


Edit: Since #2861 was merged, I rebased this PR due to a merge conflict and changed the SearchPurpose enum to a higher scope level to increase re-use. I did re-validate all of the happy-paths after the rebase.

Trenly@CORSAIR-JLU545 MINGW64 /d/Git/winget-cli (NoUpdatesFound)
$ winget upgrade windirstat
No applicable upgrade found.

Trenly@CORSAIR-JLU545 MINGW64 /d/Git/winget-cli (NoUpdatesFound)
$ winget list nothinginstalledmatchingthis
No installed package found matching input criteria.

Trenly@CORSAIR-JLU545 MINGW64 /d/Git/winget-cli (NoUpdatesFound)
$ winget search nothinginsourcematchingthis
No package found matching input criteria.

Trenly@CORSAIR-JLU545 MINGW64 /d/Git/winget-cli (NoUpdatesFound)
$ winget uninstall nothinginstalledmatchingthis
No installed package found matching input criteria.
Microsoft Reviewers: Open in CodeFlow

@Trenly
Copy link
Contributor Author

Trenly commented Feb 7, 2023

@yao-msft - Could I trouble you for a few moments of your time?

@yao-msft
Copy link
Contributor

yao-msft commented Feb 7, 2023

@yao-msft - Could I trouble you for a few moments of your time?

Sorry, I'll take a look now.

@yao-msft
Copy link
Contributor

yao-msft commented Feb 7, 2023

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

src/AppInstallerCLICore/Workflows/WorkflowBase.h Outdated Show resolved Hide resolved
src/AppInstallerCLICore/Workflows/WorkflowBase.h Outdated Show resolved Hide resolved
src/AppInstallerCLICore/Commands/ExportCommand.cpp Outdated Show resolved Hide resolved
src/AppInstallerCLICore/Commands/ListCommand.cpp Outdated Show resolved Hide resolved
src/AppInstallerCLICore/Commands/UninstallCommand.cpp Outdated Show resolved Hide resolved
src/AppInstallerCLICore/Workflows/MultiQueryFlow.cpp Outdated Show resolved Hide resolved
src/AppInstallerCLICore/Workflows/MultiQueryFlow.cpp Outdated Show resolved Hide resolved
src/AppInstallerCLICore/Workflows/UpdateFlow.cpp Outdated Show resolved Hide resolved
src/AppInstallerCLICore/Workflows/UpdateFlow.cpp Outdated Show resolved Hide resolved
src/AppInstallerCLICore/Workflows/UpdateFlow.h Outdated Show resolved Hide resolved
src/AppInstallerCLICore/Workflows/WorkflowBase.h Outdated Show resolved Hide resolved
src/AppInstallerCLICore/Workflows/WorkflowBase.h Outdated Show resolved Hide resolved
src/AppInstallerCLICore/Workflows/WorkflowBase.h Outdated Show resolved Hide resolved
src/AppInstallerCLICore/Workflows/WorkflowBase.cpp Outdated Show resolved Hide resolved
@yao-msft
Copy link
Contributor

yao-msft commented Feb 9, 2023

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

yao-msft
yao-msft previously approved these changes Feb 9, 2023
@yao-msft yao-msft dismissed their stale review February 10, 2023 20:07

Unit tests failed..

@Trenly
Copy link
Contributor Author

Trenly commented Feb 13, 2023

##[error]There are one or more test failures detected in result files. Detailed summary of published test results can be viewed in the Tests tab.

I can't seem to find the Tests tab to tell me which one is failing ;-;

@florelis
Copy link
Member

##[error]There are one or more test failures detected in result files. Detailed summary of published test results can be viewed in the Tests tab.

I can't seem to find the Tests tab to tell me which one is failing ;-;

image

Looks like some tests were looking for the strings you changed:
FAILED: REQUIRE( installOutput.str().find(Resource::LocString(Resource::String::UpdateNotApplicable).get()) != std::string::npos )

@Trenly
Copy link
Contributor Author

Trenly commented Mar 2, 2023

@yao-msft - Can you trigger the pipelines and hopefully the unit tests pass this time around?

@denelon
Copy link
Contributor

denelon commented Mar 2, 2023

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yao-msft
Copy link
Contributor

yao-msft commented Mar 2, 2023

The failing test is UpdateFlow_UpdateExeSpecificVersionNotApplicable
With failing stack:
REQUIRE( updateOutput.str().find(Resource::LocString(Resource::String::UpdateNotApplicable).get()) != std::string::npos )
with expansion:
18446744073709551615 (0xffffffffffffffff)
!=
18446744073709551615 (0xffffffffffffffff)
\x1B[0mThe install technology of the newer version specified is different from the current version installed. Please uninstall the package and install the newer version.

at D:\a\1\s\src\AppInstallerCLITests\UpdateFlow.cpp(513)

@Trenly
Copy link
Contributor Author

Trenly commented Mar 2, 2023

The failing test is UpdateFlow_UpdateExeSpecificVersionNotApplicable With failing stack: REQUIRE( updateOutput.str().find(Resource::LocString(Resource::String::UpdateNotApplicable).get()) != std::string::npos ) with expansion: 18446744073709551615 (0xffffffffffffffff) != 18446744073709551615 (0xffffffffffffffff) \x1B[0mThe install technology of the newer version specified is different from the current version installed. Please uninstall the package and install the newer version.

at D:\a\1\s\src\AppInstallerCLITests\UpdateFlow.cpp(513)

Ah. That makes sense. I don't know why I didn't see that before. Thank you for the detail (and the patience with me, as for some reason my computer refuses to run the unit tests, even though I know I've set the certificates)

@yao-msft
Copy link
Contributor

yao-msft commented Mar 2, 2023

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@yao-msft
Copy link
Contributor

yao-msft commented Mar 3, 2023

InstallExeFoundExistingConvertToUpgradeNoAvailableUpgrade e2e test failed

at AppInstallerCLIE2ETests.InstallCommand.InstallExeFoundExistingConvertToUpgradeNoAvailableUpgrade() in \src\AppInstallerCLIE2ETests\InstallCommand.cs:line 591

@yao-msft
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@Trenly
Copy link
Contributor Author

Trenly commented Mar 10, 2023

/azp run

I apologize in advance if the unit tests fail again. I keep getting certificate errors anytime I try to run them locally and I can't figure out why. I've followed the steps in the readme to generate the cert and start the local web server, but no dice

@yao-msft
Copy link
Contributor

/azp run

I apologize in advance if the unit tests fail again. I keep getting certificate errors anytime I try to run them locally and I can't figure out why. I've followed the steps in the readme to generate the cert and start the local web server, but no dice

Those are for e2e tests. And yes it's quite complex to setup. For unit tests, I think you can just run the AppInstallerCliTests.exe, and it should just start. And if there are any test failures, I can help fix them directly if you are ok.

@yao-msft yao-msft merged commit 9acb01b into microsoft:master Mar 11, 2023
@Trenly Trenly deleted the NoUpdatesFound branch March 11, 2023 03:27
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.

4 participants