Add a system for testing correlation E2E #2071

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 [email protected]: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

florelis

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

florelis · 2022-04-06T18:02:05Z

tools/CorrelationTestbed/Test-CorrelationInSandbox.ps1

+  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?

florelis · 2022-04-06T18:09:24Z

tools/CorrelationTestbed/Test-CorrelationInSandbox.ps1

+  $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.

tools/CorrelationTestbed/Test-CorrelationInSandbox.ps1

florelis · 2022-04-06T18:26:07Z

tools/CorrelationTestbed/Test-CorrelationInSandbox.ps1

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


Nit: Missing Dev Package and Reg File folders

tools/CorrelationTestbed/Test-CorrelationInSandbox.ps1

src/Microsoft.Management.Deployment/PackageManager.idl

florelis · 2022-04-06T18:33:19Z

src/Microsoft.Management.Deployment/PackageManager.idl

@@ -270,6 +270,8 @@ namespace Microsoft.Management.Deployment
        {
            /// 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.

src/Microsoft.Management.Deployment/PackageManager.idl

florelis · 2022-04-06T18:34:59Z

src/AppInstallerRepositoryCore/Public/winget/RepositorySearch.h

@@ -134,6 +134,7 @@ namespace AppInstaller::Repository
        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.

src/AppInstallerRepositoryCore/Public/winget/RepositorySearch.h

florelis · 2022-04-06T18:37:54Z

Why did all my comments show up twice...

florelis · 2022-04-06T23:21:54Z

tools/CorrelationTestbed/Test-CorrelationInSandbox.ps1

+
+    $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 :(

florelis · 2022-04-06T23:44:53Z

tools/CorrelationTestbed/Test-CorrelationInSandbox.ps1

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


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

florelis · 2022-04-06T23:47:10Z

tools/CorrelationTestbed/InSandboxScript.ps1

+
+$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

florelis · 2022-04-07T19:30:46Z

tools/CorrelationTestbed/Test-CorrelationInSandbox.ps1

+    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.

florelis

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

src/AppInstallerCLIPackage/Package.appxmanifest

@@ -16,7 +16,6 @@
  <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

src/AppInstallerRepositoryCore/Microsoft/SQLiteIndexSource.cpp

@@ -48,7 +48,8 @@ namespace AppInstaller::Repository::Microsoft
                    return LocIndString{ GetReferenceSource()->GetDetails().Name };
                default:
                    // 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

florelis reviewed Apr 6, 2022

View reviewed changes

florelis reviewed Apr 7, 2022

View reviewed changes

PR feedback

54403c4

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a system for testing correlation E2E #2071

Add a system for testing correlation E2E #2071

JohnMcPMS commented Apr 6, 2022 •

edited by ghost

Loading

github-actions bot commented Apr 6, 2022 •

edited

Loading

florelis left a comment

florelis Apr 6, 2022

florelis Apr 6, 2022

JohnMcPMS Apr 8, 2022

florelis Apr 6, 2022

florelis Apr 6, 2022

JohnMcPMS Apr 8, 2022

florelis Apr 6, 2022

JohnMcPMS Apr 8, 2022

florelis commented Apr 6, 2022

florelis Apr 6, 2022

florelis Apr 6, 2022

florelis Apr 6, 2022

florelis Apr 7, 2022

florelis left a comment

yao-msft Apr 8, 2022

JohnMcPMS Apr 8, 2022

yao-msft Apr 8, 2022

yao-msft left a comment


		$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")

Add a system for testing correlation E2E #2071

Add a system for testing correlation E2E #2071

Conversation

JohnMcPMS commented Apr 6, 2022 • edited by ghost Loading

Change

Microsoft Reviewers: Open in CodeFlow

github-actions bot commented Apr 6, 2022 • edited Loading

@check-spelling-bot Report

Unrecognized words, please review:

florelis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

florelis commented Apr 6, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

florelis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yao-msft left a comment

Choose a reason for hiding this comment

JohnMcPMS commented Apr 6, 2022 •

edited by ghost

Loading

github-actions bot commented Apr 6, 2022 •

edited

Loading