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

(#508) Fix force dependencies to not remove existing non-dependency packages #3013

Merged
merged 3 commits into from
Feb 4, 2023

Conversation

corbob
Copy link
Member

@corbob corbob commented Feb 4, 2023

Description Of Changes

When forcing dependency reinstallations, don't mark local non-dependency packages as requiring uninstall/reinstall.

Motivation and Context

When doing choco install hasdependencies --force --forcedependencies Chocolatey would mark all local packages for uninstallation and reinstallation. It would then remove the local packages, but fail to reinstall any that weren't dependencies.

Testing

  1. Ran in test kitchen against current develop branch (only InstallCommand) - Confirmed failed the updated tests

image

  1. Ran in test kitchen against changes (only InstallCommand) - Confirmed no longer failed the updated tests

image

  1. Ran Integration Tests after the first commit - Confirmed new tests failed
  2. Ran Integration Tests after the second commit - Confirmed new tests no longer fail

Operating Systems Testing

  • Windows 10 (Manual testing with debug build/integration tests)
  • Windows Server 2016 (Test Kitchen)
  • Windows Server 2019 (Test Kitchen)

Change Types Made

  • Bug fix (non-breaking change).
  • Feature / Enhancement (non-breaking change).
  • Breaking change (fix or feature that could cause existing functionality to change).
  • Documentation changes.
  • PowerShell code changes.

Change Checklist

  • Requires a change to the documentation.
  • Documentation has been updated.
  • Tests to cover my changes, have been added.
  • All new and existing tests passed?
  • PowerShell code changes: PowerShell v2 compatibility checked?

Related Issue

Update the integration test scenarios for install scenario to ensure
that forcing reinstallation does not erroneously remove any existing
packages.
When forcing dependencies, we need to ensure that we're only giving the
dependencies to the service to force reinstallation. This updates the
logic to only get the packages that are dependencies.
Remove the broken tag from the force dependencies pester tests as this
scenario has been fixed.
@corbob
Copy link
Member Author

corbob commented Feb 4, 2023

Opened PR as a Draft while the Test Kitchen tests run. Once they're done, will mark as ready.

@corbob corbob marked this pull request as ready for review February 4, 2023 17:25
@corbob corbob requested a review from gep13 February 4, 2023 17:25
Copy link
Member

@gep13 gep13 left a comment

Choose a reason for hiding this comment

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

LGTM!

@gep13
Copy link
Member

gep13 commented Feb 4, 2023

@corbob thanks for getting this fixed up! Will move forward with the merge, even though the Mac build here is failing. This build started failing as a result of another change, and I haven't had a chance to look into this yet.

@gep13 gep13 merged commit a1c77d5 into chocolatey:develop Feb 4, 2023
@corbob corbob deleted the fix-force-dependencies branch February 4, 2023 19:18
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.

Upgrade to target version 3.1.0 of Chocolatey.NuGet.Client assemblies
2 participants