(#3451) Use availablePackages in GetPackageDependencies

[josh-cooley]

Description Of Changes

This updates the dependency handling of packages to use availablePackages when a package from the same source satisfies the dependency. This improves packages that have dependencies with many versions available, and allows less data being requested from servers in the case we have already have available packages from the source.

Without this fix, the installation will be delayed with repeated resolution of the same package dependencies.

Motivation and Context

Installation of packages with many dependent versions available without reducing repeat calls to ResolvePackages

Testing

1. Download demo-projects.zip
2. Run GeneratePackages.ps1. This will generate a single package for package-a, package-b, and package-c. It will generate 1000 packages each for package-d and package-e.
3. Clone and checkout the branch associated with this PR.
4. Build chocolatey.console.
5. Run choco upgrade package-a --yes --source <path-to-demo-projects>
6. See that install/upgrade completes within seconds.

Repeat test with delivered chocolatey 2.3 and see that the install takes much longer.

Operating Systems Testing

• Windows 10

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 v3 compatibility checked?

Related Issue

Fixes #3451

[corbob]

Overall, this looks good. I have a few questions I still need to formulate around the tests, but have a few comments/suggestions here at the moment.

As well as the noted requests, it seems that the integration tests have some failures here. It looks like at least one of them is a failing due to a change in behaviour here, and we'll need to determine if this is how we'd like to proceed.

You can run the integration tests with the build script by running: ./build.bat --target=test-nunit --exclusive --testExecutionType=all --shouldRunOpenCover=false which will run all of the tests including integration. By default just the unit tests are run as the integration tests can take 20+ minutes.

[corbob]

The integration tests that are failing, are:

1. chocolatey.tests.integration.scenarios.UpgradeScenarios+When_upgrading_a_dependency_with_parent_being_pinned_and_depends_on_a_range_less_than_upgrade_version.Should_have_outputted_conflicting_upgrade_message
2. chocolatey.tests.integration.scenarios.UpgradeScenarios+When_upgrading_a_dependency_with_parent_being_pinned_and_depends_on_a_range_less_than_upgrade_version.Should_upgrade_the_package_to_highest_version_in_range
3. chocolatey.tests.integration.scenarios.UpgradeScenarios+When_upgrading_a_dependency_with_parent_being_pinned_and_depends_on_a_range_less_than_upgrade_version_and_has_missing_dependency.Should_contain_a_message_that_everything_upgraded_successfully
4. chocolatey.tests.integration.scenarios.UpgradeScenarios+When_upgrading_a_dependency_with_parent_being_pinned_and_depends_on_a_range_less_than_upgrade_version_and_has_missing_dependency.Should_have_outputted_conflicting_upgrade_message
5. chocolatey.tests.integration.scenarios.UpgradeScenarios+When_upgrading_a_dependency_with_parent_being_pinned_and_depends_on_a_range_less_than_upgrade_version_and_has_missing_dependency.Should_not_upgrade_the_exact_version_dependency
6. chocolatey.tests.integration.scenarios.UpgradeScenarios+When_upgrading_a_dependency_with_parent_being_pinned_and_depends_on_a_range_less_than_upgrade_version_and_has_missing_dependency.Should_upgrade_the_package_to_highest_version_in_range
7. chocolatey.tests.integration.scenarios.UpgradeScenarios+When_upgrading_a_dependency_with_parent_has_different_pinned_dependency_and_depends_on_a_range_less_than_upgrade_version.Should_have_outputted_conflicting_upgrade_message
8. chocolatey.tests.integration.scenarios.UpgradeScenarios+When_upgrading_a_dependency_with_parent_has_different_pinned_dependency_and_depends_on_a_range_less_than_upgrade_version.Should_upgrade_the_package_to_highest_version_in_range

The more I look at them, the more I'm sure they're correct, and this PR will need to address the failures before we can look to merging it. All of them are in the UpgradeScenarios.cs file found here: https://github.com/chocolatey/choco/blob/3c1806041bcb11702a71640603d6fba64cfe85c1/src/chocolatey.tests.integration/scenarios/UpgradeScenarios.cs

[josh-cooley]

Thanks for the integration test info. I get a 'Ya!' response from the tests when I run them using the command line build script, but I'm able to reproduce failures when I run some of the tests with Visual Studio. I've debugged enough to have some idea where this is going wrong and will look at possible changes.

[josh-cooley]

I've changed NugetService to use parentPackage.Version when calling FindPackage. It was using the version from the config to find the parent version, which resulted in a mismatch when searching for dependencies. An alternative solution would be to make the NugetCommon.GetPackageDependency change work independent of the state of the arguments passed in, but the use of parentPackage.Version seemed like it matches the intent of the NugetService code.

[corbob]

Thank you for submitting this PR @josh-cooley. I've done some testing, and think it is just about ready for merging. I've been writing some Pester tests to encompass the manual testing mentioned above, as well as to pick up on at least one of the integration tests that had failed previously. I've pushed up the mentioned tests to your branch, so if you have any further changes you've made, you may need to fetch the changes and rebase some things.

[corbob]

@josh-cooley, I have rebased your PR off of the latest develop and force pushed it to your fork. I don't expect you to be making further changes to the PR, but in case you were I wanted to let you know. (Normally I would try to avoid a force push to someone else's fork if I wasn't going to be merging immediately, but I have integrations I want to run with the rebased branch)

[josh-cooley]

@corbob Thanks for the thorough review and adding the additional Pester tests. I do not have any additional changes pending. If you have additional changes you'd like me to make, please let me know.

These changes have been made.

[corbob]

I have converted this PR to a draft. This is because there is still some discussion to be had of if we target this as a bug or an improvement. Setting the PR to draft will prevent us from merging it until we have targetted the appropriate branch based on the classification.

[corbob]

Thank you very much for getting this fixed up @josh-cooley 👍