Add a system for testing correlation E2E by JohnMcPMS · Pull Request #2071 · microsoft/winget-cli

JohnMcPMS · 2022-04-06T00:27:40Z

This change adds a few scripts, a test project, and one small product change, all around enabling end-to-end validation of our correlation between system artifacts and packages in external sources.

Change

The product change is to expose publisher on the PackageVersion COM object. This enables the test system to collect that data for further use, as well as allowing human verification of the correlation (if one is made).

The test project uses the COM API to first install a package, then attempts to check for correlation in two ways (directions):

When the remote package identity is known, the caller will usually look it up via that remote identity. This path can enable additional information to be retrieved to make the correlation with.
When listing all of the local packages, the remote identity must be determined for each one. We can use only data known locally to make the correlation.

The primary script, Test-CorrelationInSandbox.ps1, is based off of the sandbox test script in winget-pkgs (thanks to many people). It sets up the sandbox and initial script to run there for each package to test, then waits for a sentinel file to be created in the output location. The results of all of running the test project exe, as well as the ARP differences and winget logs are put in that output location, then the sandbox is destroyed for the next package to run.

Test-CorrelationInSandbox.ps1
[[-PackageIdentifiers] <string[]>] :: A set of package ids to test
[[-Source] <string>] :: The name of the source that the packages are from, ex. "winget"
[-ExePath <string>] :: Path to the test exe; defaults to the Release output location
[-UseDev] :: Switch to use the local dev winget build
[-DevPackagePath <string>] :: Path to the local dev *Release* winget build; defaults to the normal location
[-ResultsPath <string>] :: Path to output the results to; defaults to a temp directory
[-RegFileDirectory <string>] :: Path to a directory containing .reg files to insert before the test, creating noise for correlation

Once testing is done, Process-CorrelationResults.ps1 will take all of the results JSON files and put them into results.csv in the directory. If any tests failed to run, they will be in failed.csv. There are correlation columns in the CSV that can be averaged in Excel to get the correlation percentage.

Microsoft Reviewers: Open in CodeFlow

github-actions · 2022-04-06T00:28:54Z

check-spelling-bot Report

Unrecognized words, please review:

csv
DAEABC
EEBE
esktop
estination
ev
HKCU
HKLM
imeout
irectory
Logon
ools
SDKs
Tls
uninit
WDAG
wsb

Previously acknowledged words that are now absent

activatable amd Archs dsc Globals hackathon mytool Packagedx parametermap UWP whatif

To accept these unrecognized words as correct (and remove the previously acknowledged and now absent words), run the following commands

... in a clone of the git@github.com:JohnMcPMS/winget-cli.git repository
on the sandbox-correlation-test branch:

update_files() {
perl -e '
my @expect_files=qw('".github/actions/spelling/expect.txt"');
@ARGV=@expect_files;
my @stale=qw('"$patch_remove"');
my $re=join "|", @stale;
my $suffix=".".time();
my $previous="";
sub maybe_unlink { unlink($_[0]) if $_[0]; }
while (<>) {
if ($ARGV ne $old_argv) { maybe_unlink($previous); $previous="$ARGV$suffix"; rename($ARGV, $previous); open(ARGV_OUT, ">$ARGV"); select(ARGV_OUT); $old_argv = $ARGV; }
next if /^(?:$re)(?:(?:\r|\n)*$| .*)/; print;
}; maybe_unlink($previous);'
perl -e '
my $new_expect_file=".github/actions/spelling/expect.txt";
use File::Path qw(make_path);
use File::Basename qw(dirname);
make_path (dirname($new_expect_file));
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"$patch_add"');
my %items; @items{@words} = @words x (1); @items{@add} = @add x (1);
@words = sort {lc($a)."-".$a cmp lc($b)."-".$b} keys %items;
open FILE, q{>}, $new_expect_file; for my $word (@words) { print FILE "$word\n" if $word =~ /\w/; };
close FILE;
system("git", "add", $new_expect_file);
'
}

comment_json=$(mktemp)
curl -L -s -S \
  --header "Content-Type: application/json" \
  "https://api.github.com/repos/microsoft/winget-cli/issues/comments/1089572615" > "$comment_json"
comment_body=$(mktemp)
jq -r .body < "$comment_json" > $comment_body
rm $comment_json

patch_remove=$(perl -ne 'next unless s{^</summary>(.*)</details>$}{$1}; print' < "$comment_body")
  

patch_add=$(perl -e '$/=undef;
$_=<>;
s{<details>.*}{}s;
s{^#.*}{};
s{\n##.*}{};
s{(?:^|\n)\s*\*}{}g;
s{\s+}{ }g;
print' < "$comment_body")
  
update_files
rm $comment_body
git add -u

Flor Chacón (florelis)

Can you add a README or something like that for how to use the scripts?

Flor Chacón (florelis) · 2022-04-06T18:02:05Z

+  Remove-Item -Recurse $ResultsPath -Force
+}
+
+New-Item -ItemType Directory $ResultsPath > $nul


Nit 1: Did you mean $null?
Nit 2: You are using both > $nul and | Out-Null. Should it be consistent?

Flor Chacón (florelis) · 2022-04-06T18:09:24Z

+  $devVCLibsPathInSandbox = Join-Path -Path $desktopInSandbox -ChildPath (Join-Path -Path $tempFolderName -ChildPath $devVCLibsFileName)
+}
+else
+{


Is something missing here?

No, I just didn't indent the entire existing block at the time.

Flor Chacón (florelis) · 2022-04-06T18:26:07Z

+--> Starting Windows Sandbox, and:
+    - Mounting the following directories:
+        - $tempFolder as read-only
+        - $outPath as read-write


Nit: Missing Dev Package and Reg File folders

Flor Chacón (florelis) · 2022-04-06T18:33:19Z

            /// Checks if this package version has at least one applicable installer.
            Boolean HasApplicableInstaller { get; };
+
+            String Publisher { get; };


Do we need to do any review for these API changes?

Not for these simple ones, no.

Flor Chacón (florelis) · 2022-04-06T18:34:59Z

        RelativePath,
        // Returned in hexadecimal format
        ManifestSHA256Hash,
+        Publisher,


Looks like something I'll need to use in my change for matching heuristics. 🤔

Yes, although it just makes getting the value easier since you don't have to get all of the metadata, so it isn't really critical to move to it.

Flor Chacón (florelis) · 2022-04-06T18:37:54Z

Why did all my comments show up twice...

Flor Chacón (florelis) · 2022-04-06T23:21:54Z

+
+    $outputFileBlockerPath = Join-Path $outPath "done.txt"
+
+    while (-not (Test-Path $outputFileBlockerPath))


I would add a timeout here. When I tried doing something similar, I saw many cases where the app would still require interaction and it would block the script until I went to click something.

I think that shouldn't happen, but it seems the msstore source doesn't always gives us the right arguments for the installers :(

Flor Chacón (florelis) · 2022-04-06T23:44:53Z

+        <ReadOnly>true</ReadOnly>
+    </MappedFolder>
+    <MappedFolder>
+        <HostFolder>$ExePath</HostFolder>


Seems like these need to be expanded to absolute paths for it to work.

Flor Chacón (florelis) · 2022-04-06T23:47:10Z

+
+$arpCompared | Select-Object -Property * -ExcludeProperty SideIndicator | Format-Table | Out-File (Join-Path $OutputPath "ARPCompare.txt")
+
+"Done" | Out-File (Join-Path $OutputPath "done.txt")


This won't happen if there is any error during the script, so the main one will just hang waiting for this. I don't know if that is good or bad

Flor Chacón (florelis) · 2022-04-07T19:30:46Z

+    Remove-Variable sandbox
+}
+
+Close-WindowsSandbox


Can you do this before line 85 where you remove the results folder?
If the script fails and the sandbox stays open, it will hold on to the results files and block removing the folder so we have to manually close the sandbox before running again.

Flor Chacón (florelis)

yao-msft · 2022-04-08T22:12:08Z

  <Dependencies>
    <!-- Minimum supported version is 1809 (October 2018 Update, aka RS5) -->
    <TargetDeviceFamily Name="Windows.Desktop" MinVersion="10.0.17763.0" MaxVersionTested="10.0.19033.0" />
-    <PackageDependency Name="Microsoft.VCLibs.140.00.UWPDesktop" MinVersion="14.0.25426.0" Publisher="CN=Microsoft Corporation, O=Microsoft Corporation, L=Redmond, S=Washington, C=US" />


Is this removal intentional?

Yes, it is being injected by VS anyway. This one was leading to debug needing the release package (as VS left this and injected the debug on as well).

yao-msft · 2022-04-08T22:14:15Z

                    // Values coming from the index will always be localized/independent.
-                    return LocIndString{ GetReferenceSource()->GetIndex().GetPropertyByManifestId(m_manifestId, property).value() };
+                    std::optional<std::string> optValue = GetReferenceSource()->GetIndex().GetPropertyByManifestId(m_manifestId, property);
+                    return LocIndString{ optValue ? optValue.value() :std::string{} };


:st

nit: spacing

yao-msft

JohnMcPMS added 2 commits March 24, 2022 15:12

Script can setup, install, and wait for completion to loop through items

bbc1298

Pretty much all of it

34b8b36

JohnMcPMS requested a review from a team as a code owner April 6, 2022 00:27

Flor Chacón (florelis) reviewed Apr 6, 2022

View reviewed changes

Flor Chacón (florelis) reviewed Apr 7, 2022

View reviewed changes

PR feedback

54403c4

Flor Chacón (florelis) approved these changes Apr 8, 2022

View reviewed changes

yao-msft reviewed Apr 8, 2022

View reviewed changes

yao-msft approved these changes Apr 8, 2022

View reviewed changes

Merge from master

37808d3

JohnMcPMS merged commit 7ad39fe into microsoft:master Apr 11, 2022

JohnMcPMS deleted the sandbox-correlation-test branch April 11, 2022 20:20


		$outputFileBlockerPath = Join-Path $outPath "done.txt"

		while (-not (Test-Path $outputFileBlockerPath))


		$arpCompared \| Select-Object -Property * -ExcludeProperty SideIndicator \| Format-Table \| Out-File (Join-Path $OutputPath "ARPCompare.txt")

		"Done" \| Out-File (Join-Path $OutputPath "done.txt")

Conversation

JohnMcPMS commented Apr 6, 2022 • edited by ghost Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Change

Microsoft Reviewers: Open in CodeFlow

Uh oh!

github-actions Bot commented Apr 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

check-spelling-bot Report

Unrecognized words, please review:

Uh oh!

Flor Chacón (florelis) left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Flor Chacón (florelis) commented Apr 6, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Flor Chacón (florelis) left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yao-msft left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

JohnMcPMS commented Apr 6, 2022 •

edited by ghost

Loading

github-actions Bot commented Apr 6, 2022 •

edited

Loading