[AppInstallerCLIE2ETests] If "StaticFileRootPath" in the .runsettings is not set before unit tests run, unit test run will try to remove user's entire %TEMP% directory

[ameltzerMSFT]

Brief description of your issue

When AppInstallerCLIE2ETests's setup code runs, it attempts to set a variable called TestCommon.StaticFileRootPath, first based on a parameter called StaticRootFilePath. If this is not set, it uses Path.GetTempPath:

https://github.com/microsoft/winget-cli/blob/3aa7aa444b862d7a186ad13cdc7322c3e6186f07/src/AppInstallerCLIE2ETests/SetUpFixture.cs#L85

Then, based on the output of this statement the setup will eventually attempt to entirely remove that directory and its contents: https://github.com/microsoft/winget-cli/blob/082b9907644244c8697b4cdd4616ea111f15b73a/src/AppInstallerCLIE2ETests/TestIndexSetup.cs#L167

This presents a major problem if it falls through to the Path.GetTempPath()-derived value as it will try to remove the entirety of the user's %TEMP% directory.

This has a couple of obvious problems, the first being of course that it tries to remove the user's %TEMP% directory which doesn't seem like a desirable behavior. From a more practical standpoint, this path will very likely have files in it that are in use by some other process and there's no error tolerance for the directory cleanup. As a result will end up causing the unit test to fail during the setup step.

I discovered this by accident when I was trying to run a unit test here to perform some debugging into an issue and found that the setup step kept failing to remove a file in my %TEMP% directory. Digging further I found how it was trying to remove my entire %TEMP% directory since I was missing a setting in the .runsettings.

The README.md does note that StaticFileRoot must be set, so it seems reasonable that the unit test setup should fail right away rather than falling back to Path.GetTempPath(). However, if this is a desired fallback (even though this contradicts the documentation), it should Path.Combine(...) the output of Path.GetTempPath() with a sub-path so that is what gets cleaned up rather than the user's entire %TEMP% path.

Steps to reproduce

Run AppInstallerCLIE2ETests without setting a StaticFileRootPath in the runsettings.

Expected behavior

Not try to delete the entirety of the user's %TEMP% folder

Actual behavior

Tries to delete the entirety of the user's %TEMP% folder

Environment

Windows Package Manager (Preview) v1.4.2161-preview
Copyright (c) Microsoft Corporation. All rights reserved.

Windows: Windows.Desktop v10.0.19044.2130
System Architecture: X64
Package: Microsoft.DesktopAppInstaller v1.19.2161.0