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

Winget Repair - Eliminate installer type mapping for MSI/WIX and MSIX NonStore ,code refactoring & E2E Test Coverage #4534

Conversation

Madhusudhan-MSFT
Copy link
Contributor

@Madhusudhan-MSFT Madhusudhan-MSFT commented Jun 4, 2024

Repair Command updates:

  • Code changes eliminate installer type mapping for MSI/WIX and MSIX NonStore, leveraging native platform support for repairs.
  • Refactored code for better readability and maintainability.

AppInstallerTestExeInstaller - Repair support

  • Refactored AppInstallerTestExeInstaller to support repair operations. The code now accommodates Modify Repair, Uninstaller Repair, and Installer Repair.
  • Minor refactoring is also part of this update

E2E Test Coverage:

  • Added E2E tests for winget, targeting installer types such as
    • MSI,
    • NonStore MSIX,
    • Burn, Nullsoft, Inno, and Exe.
      • These tests aim to ensure reliability across Modify Repair, Uninstaller Repair, and Installer Repair scenarios.

How Validated:

  • Configured LocalhostWebServer for local execution.
  • Executed WingetRepair Tests via Visual Studio TestExplorer, confirming all tests passed on the local machine

image


Microsoft Reviewers: Open in CodeFlow

… MSI/WIX and MSIX NonStore & code refactoring

The changes includes:
- Code changes eliminate installer type mapping for MSI/WIX and MSIX NonStore, leveraging native platform support for repairs.
- Refactored code for better readability and maintainability.
- Refactored AppInstallerTestExeInstaller to support repair operations. The code now accommodates Modify Repair, Uninstaller Repair, and Installer Repair.
- Minor refactoring is also part of this update
- Added E2E tests for winget, targeting installer types such as
  - MSI,
  - NonStore MSIX,
  - Burn, Nullsoft, Inno, and Exe.
    - These tests aim to ensure reliability across Modify Repair, Uninstaller Repair, and Installer Repair scenarios.
@Madhusudhan-MSFT Madhusudhan-MSFT requested a review from a team as a code owner June 4, 2024 03:57

This comment has been minimized.

This comment has been minimized.

src/AppInstallerCLICore/Workflows/RepairFlow.h Outdated Show resolved Hide resolved
src/AppInstallerCLICore/Workflows/RepairFlow.cpp Outdated Show resolved Hide resolved
src/AppInstallerCLICore/Workflows/RepairFlow.cpp Outdated Show resolved Hide resolved
src/AppInstallerCLICore/Workflows/RepairFlow.cpp Outdated Show resolved Hide resolved
src/AppInstallerCLICore/Workflows/RepairFlow.cpp Outdated Show resolved Hide resolved
src/AppInstallerCLICore/Workflows/RepairFlow.cpp Outdated Show resolved Hide resolved
src/AppInstallerCLICore/Workflows/RepairFlow.cpp Outdated Show resolved Hide resolved
src/AppInstallerCLICore/Workflows/RepairFlow.cpp Outdated Show resolved Hide resolved
src/AppInstallerCLICore/Workflows/RepairFlow.cpp Outdated Show resolved Hide resolved
src/AppInstallerCLICore/Workflows/RepairFlow.cpp Outdated Show resolved Hide resolved
- In `RepairFlow.h`, the inclusion of `"winget/ManifestCommon.h"` has been removed.
- In `AppInstallerCLIE2ETests.csproj`, several test data files, including 'AppInstallerTestMsiInstallerV2.msi` and various YAML files, have been removed
- The function `SelectApplicableInstallerWhenEssential` has been renamed to `SelectApplicableInstallerIfNecessary` and modified to include new checks and calls.
- The function `HandleUnknownRepair` has been removed and its error handling code has been directly inserted where it was previously called.
- The function `GetInstalledPackageInstallerType` has been renamed to `GetInstalledType`.
- Comments in `RepairFlow.cpp` and `RepairFlow.h` have been updated to reflect these changes.
`Moved the line 'context << SetPackageFamilyNamesInContext;' fRepairMsixPackage. This change ensures that the SetPackageFamilyNamesInContext operation is only executed for non-store MSIX packages, possibly when the package is not from the MSStore.`
- Updated the `ExecuteRepair` and `GetRepairInfo` functions to handle different installer types more flexibly
- Removed `RepairMsixPackage` function. Updated function declarations in `RepairFlow.h` to reflect these changes.
- Simplified the determination of `repairPackage` in `ReportRepairResult`.
Renamed the function `RepairMsixNonStorePackage` to `RepairMsixPackage` in `RepairFlow.cpp` and `RepairFlow.h` files. This change is also reflected in the context where this function is called in the `InstallerTypeEnum::Msix` case.
…cpp`

- The `IsInstallerMappingRequired` function now uses a `switch` statement for different `InstallerTypeEnum` values and checks remotepackage source for MSIX type to confirm MSStore package check
- `SelectApplicablePackageVersion` function now returns early for `Msi` types and `Msix` packages not from the Microsoft Store.
- The `SelectApplicableInstallerIfNecessary` function has been updated to select the applicable package version before checking if installer mapping is required.
@ryfu-msft
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

src/AppInstallerCLICore/Workflows/RepairFlow.h Outdated Show resolved Hide resolved
src/AppInstallerCLIE2ETests/RepairCommand.cs Outdated Show resolved Hide resolved
src/AppInstallerCLICore/Workflows/RepairFlow.cpp Outdated Show resolved Hide resolved
src/AppInstallerCLICore/Workflows/RepairFlow.cpp Outdated Show resolved Hide resolved
- Refined the logic for MSIX package handling in the repair flow, focusing on Microsoft Store packages and improving installer selection accuracy.
- Simplified installer type checks by removing redundant MSI checks, streamlining the repair process.
- Adjusted repair command tests for clarity, removing unnecessary string interpolation.
- Enhanced documentation and comments for better clarity on the repair flow and installer selection logic.
@Madhusudhan-MSFT Madhusudhan-MSFT merged commit 04b2c4c into microsoft:master Jun 15, 2024
8 checks passed
@Madhusudhan-MSFT Madhusudhan-MSFT deleted the user/masudars/winget_repair_refactoring_e2e_tests branch June 15, 2024 06:31
ryfu-msft pushed a commit to ryfu-msft/winget-cli that referenced this pull request Jun 25, 2024
… NonStore ,code refactoring & E2E Test Coverage (microsoft#4534)

**Repair Command updates:**
- Code changes eliminate installer type mapping for MSI/WIX and MSIX
NonStore, leveraging native platform support for repairs.
- Refactored code for better readability and maintainability.

**AppInstallerTestExeInstaller - Repair support**
- Refactored AppInstallerTestExeInstaller to support repair operations.
The code now accommodates Modify Repair, Uninstaller Repair, and
Installer Repair.
- Minor refactoring is also part of this update

**E2E Test Coverage:**
- Added E2E tests for winget, targeting installer types such as
  - MSI,
  - NonStore MSIX,
  - Burn, Nullsoft, Inno, and Exe.
- These tests aim to ensure reliability across Modify Repair,
Uninstaller Repair, and Installer Repair scenarios.

**How Validated:**
- Configured LocalhostWebServer for local execution. 
- Executed WingetRepair Tests via Visual Studio TestExplorer, confirming
all tests passed on the local machine


![image](https://github.com/microsoft/winget-cli/assets/53235553/e3b935b6-4be3-419e-a81d-dd639f5d9377)

<!-- To check a checkbox place an "x" between the brackets. e.g: [x] -->

- [x] I have signed the [Contributor License
Agreement](https://cla.opensource.microsoft.com/microsoft/winget-pkgs).
- [ ] This pull request is related to an issue.

-----

###### Microsoft Reviewers: [Open in
CodeFlow](https://microsoft.github.io/open-pr/?codeflow=https://github.com/microsoft/winget-cli/pull/4534)
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.

3 participants