(#3489) Return early when folder doesn't exist

[gep13]

Description Of Changes

This commit addresses the issue by first checking to see if the package folder exists. If it doesn't, then it doesn't attempt to create the .chocolateyPending file, and simply returns early. This then allows the remaining operations, i.e. returning the package folder from the lib-bkp folder, back to the lib folder.

Motivation and Context

The MarkPackagePending method is used to create a .chocolateyPending file in the package folder to indicate that there is a pending operation that needs to be completed.

However, during an upgrade operation, the package folder is moved to the lib-bkp folder, and as a result, doesn't exist anymore in the lib folder.

If during the upgrade operation, something fails, for example, an exception when trying to download the nupkg file, a process of returning the package folder to the correct place begins. The first thing that is attempted is to create the .chocolateyPending file in the package folder. However, as mentioned, this folder no longer exists in the expected location. As a result, it is possible for an exception to be thrown, as the file cannot be created in the package folder.

This problem can lead to another related problem in that the package is now "lost" from lib folder, and will not show up when doing for example choco list.

Imagine you were attempting the following command, and an exception happened during the downloading of the nupkg:

choco upgrade audacity

Chocolatey CLI would start the upgrade operation, and then it would fail downloading the package. It would attempt to create the .chocolateyPending file, and another exception would be thrown. The package folder is not returned to the lib folder, and now the package is "lost". If you attempt to run choco upgrade audacity again, the operation would succeed, as it would see that the package is not installed and it would be installed, however, if this happened during the process of running choco upgrade all, then the underlying audacity application would not be upgraded, as Chocolatey CLI is no longer managing the package.

Testing

In order to test this, you have to mimic an exception being thrown during the download of the nupkg package. The simplest way to do that is to add the following line of code after line 1619 of the NugetService.cs file.

throw new Exception("Mimic failure of package download");

With this change in place, the code should look like the following:

1. Run choco install audacity --version 3.6.0 -n
2. The audacity package should be installed without any issues
3. Run choco list to verify that the package is installed and listed as version 3.6.0
4. Run choco upgrade audacity -n
5. This operation will fail, and it will state that the package wasn't upgrade
6. Run choco list to verify that 3.6.0 package of audacity is still there

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 #3489

[gep13]

@pauby Steph was automatically added as the Code Owner for the review on this, but are you able to take a look instead? Thanks!

[gep13]

Some additional information from the second commit message:

In order to fully test out the code path where the attempt to write the
.chocolateyPending file was failing, we needed to be able to return a
503/504 error from the server. Realistically, this isn't feasible,
since it would mean having a server in place, that would be able to
respond with the right responses, at the right time.

After digging around for a little bit, I found the WireMock.Net
project, which seemed to do exactly what was needed, namely:

WireMock.Net is a flexible product for stubbing and mocking web HTTP
responses using advanced request matching and response templating.

I took this for a spin, and was able to start/stop the server within
our test harnesses, and then was able to mock the required HTTP
requests, to get to the point during an Upgrade scenario, to then send
a 503 response. This then started the code path for the
.chocolateyPending file, and I was able to make assertions that the
lib/lib-bad/lib-bkp folders worked as expected.

The only "change" that was needed to a normal test scenario, was to
change this line:

Configuration.Sources = "http://localhost:24626/api/v2/";

to force Chocolatey CLI to direct requests to the WireMock.Net server.

There is likely LOTS of things that we could start doing with this
server, and there is likely some code that needs to be added to
encapsulate the creation of the responses, to make them more re-usable,
but for now, the required responses for this code path has been
hard-coded into the responses.

NOTE: There are a LOT of packages added in this commit, and all of them
came in as a result of installing the WireMock.Net package. Since this
is a test project, i.e. we are not shipping any of these, I don't
believe this to be a concern.

[gep13]

@AdmiringWorm I have also pinged you on a review of this change as well. Can you let me know if you have any concerns/questions.

Thanks!

[AdmiringWorm]

LGTM

[pauby]

Approving this based on the conversation we had around adding MockServer.Net to be able to test this. I'm approving the addition of that, and its dependencies.

[gep13]

@AdmiringWorm are you able to merge this? Thanks!

[AdmiringWorm]

@gep13 Great work, thanks for getting this fixed up 👍

[StefH]

@gep13
A tip for using WireMock.Net:

When using WireMock.Net in (automated)-unit tests, it's better to let WireMock.Net use a random port, this makes the test more stable. See this article: https://github.com/WireMock-Net/WireMock.Net/wiki/Using-WireMock-in-UnitTests#wiremock-with-your-favorite-unittest-framework

In your case a small change is needed:

// Start WireMockServer using a random port
_wireMockServer = WireMockServer.Start();

// Force outgoing Chocolatey CLI HTTP requests to go to WireMock.NET Server
Configuration.Sources = $"{_wireMockServer.Url}/api/v2/";

// Set configuration to prevent re-use of cached HTTP Requests
Configuration.CacheExpirationInMinutes = -1;

[gep13]

@StefH said...
A tip for using WireMock.Net:

Thank you for the tip, I really appreciate it! I will get those changes made, as this might explain why the latest Integration Tests build on GitHub Actions failed for us.

This is my first time using WireMock.Net, so happy to take any advice that you can offer! We got to the point of needing to mock these HTTP Requests, and I went hunting and found WireMock.Net, and it seems to offer exactly what we needed!

Thank you for a great resource!