feat(testing): Adds support for composer api package detection

Author: mfulb

daemon/internal/newrelic/integration/php_packages.go

@@ -31,12 +31,13 @@ type PhpPackagesCollection struct {
 // PHP packages config describes how to collect the JSON for the packages installed
 // for the current test case
 type PhpPackagesConfiguration struct {
-	path                 string
-	command              string
-	supportedListFile    string
-	overrideVersionsFile string
-	expectedPackages     []string
-	packageNameOnly      []string
+	path                 string   //
+	command              string   // command to run to detect packages
+	supportedListFile    string   // JSON file containing list of packages we expect agent to detect
+	overrideVersionsFile string   // JSON file containing overrides for expected package versions
+	expectedPackages     []string // manual override of packages we expect to detect
+	packageNameOnly      []string // list of packages which only have a name because agent cannot determine the version
+	expectAllDetected    bool     // flag to indicate we expect all packages detected by the command "command"
 }
 
 // composer package JSON
@@ -159,16 +160,22 @@ func NewPhpPackagesCollection(path string, config []byte) (*PhpPackagesCollectio
 		}
 	}
 
-	if supportedOK && expectedOK {
-		return nil, fmt.Errorf("Improper EXPECT_PHP_PACKAGES config - cannot specify 'supported_packages' and 'expected packages' - got %+v", params)
+	// or "expect_all" which means we expect the agent to detect all the packages that the "command" option would detect
+	_, expectAllOK := params["expect_all"]
+
+	if (supportedOK && expectedOK) || (supportedOK && expectAllOK) || (expectedOK && expectAllOK) {
+		return nil, fmt.Errorf("Improper EXPECT_PHP_PACKAGES config - must specify one of 'supported_packages', "+
+			"'expected packages' or 'expect_all' - got %+v", params)
 	}
 
-	if !supportedOK && !expectedOK {
-		return nil, fmt.Errorf("Improper EXPECT_PHP_PACKAGES config - must specify 'supported_packages' or 'expected packages' - got %+v", params)
+	if !supportedOK && !expectedOK && !expectAllOK {
+		return nil, fmt.Errorf("Improper EXPECT_PHP_PACKAGES config - must specify 'supported_packages' or 'expected packages' "+
+			"or 'expect_all' - got %+v", params)
 	}
 
-	if supportedOK && !commandOK {
-		return nil, fmt.Errorf("Improper EXPECT_PHP_PACKAGES config - must specify 'command' option with `supported_packages` - got %+v", params)
+	if (supportedOK || expectAllOK) && !commandOK {
+		return nil, fmt.Errorf("Improper EXPECT_PHP_PACKAGES config - must specify 'command' option with `supported_packages` / "+
+			"'expect_all' - got %+v", params)
 	}
 
 	// optional option to specify which packages will only have a name because agent cannot determine the version
@@ -209,7 +216,8 @@ func NewPhpPackagesCollection(path string, config []byte) (*PhpPackagesCollectio
 			supportedListFile:    supportedListFile,
 			overrideVersionsFile: overrideVersionsFile,
 			expectedPackages:     expectedPackagesArr,
-			packageNameOnly:      packageNameOnlyArr},
+			packageNameOnly:      packageNameOnlyArr,
+			expectAllDetected:    expectAllOK},
 	}
 
 	return p, nil
@@ -296,18 +304,27 @@ func (pkgs *PhpPackagesCollection) GatherInstalledPackages() ([]PhpPackage, erro
 	var supported []string
 
 	// get list of packages we expected the agent to detect
-	// this can be one of 2 scenarios:
+	// this can be one of 3 scenarios:
 	//  1) test case used the "supported_packages" option which gives a JSON file which
 	//     lists all the packages the agent can detect
 	//  2) test case used the "expected_packages" options which provides a comma separated
 	//     list of packages we expect the agent to detect
+	//  3) test case used the "expect_all" option which means we expect the agent to
+	//     detect all the packages that the "command" option would detect
+	//
+	//  Options #1 and #2 are mutually exclusive, and are intended for testing the legacy VM detection
+	//  mechanism where the agent looks for "magic" files of a package and examinew internals of the
+	//  package to determine its version.
 	//
-	//  Option #1 is preferable as it provides the most comprehensive view of what the agent can do.
+	//  Option #1 is preferable when it available as it provides the most comprehensive view of what the agent can do.
 	//
 	//  Option #2 is needed because some test cases do not exercise all the packages which are
 	//  installed and so the agent will not detect everything for that test case run which it could
 	//  theorectically detect if the test case used all the available packages installed.
 	//
+	//  Option #3 is used when testing the agent's ability to detect packages using the Composer API.  In
+	//  this case we expect the agent to detect the exact same packages as composer would detect.
+	//
 	//  Once the list of packages the agent is expected to detect is created it is used to filter
 	//  down the package list returned by running the "command" (usually composer) option for the
 	//  test case provided.
@@ -318,8 +335,9 @@ func (pkgs *PhpPackagesCollection) GatherInstalledPackages() ([]PhpPackage, erro
 		}
 	} else if 0 < len(pkgs.config.expectedPackages) {
 		supported = pkgs.config.expectedPackages
-	} else {
-		return nil, fmt.Errorf("Error determining expected packages - supported_packages and expected_packages are both empty")
+	} else if !pkgs.config.expectAllDetected {
+		return nil, fmt.Errorf("Error determining expected packages - supported_packages and expected_packages are both empty " +
+			"and expect_all is false")
 	}
 
 	splitCmd := strings.Split(pkgs.config.command, " ")
@@ -338,8 +356,8 @@ func (pkgs *PhpPackagesCollection) GatherInstalledPackages() ([]PhpPackage, erro
 		detected := ComposerJSON{}
 		json.Unmarshal([]byte(out), &detected)
 		for _, v := range detected.Installed {
-			//fmt.Printf("composer detected %s %s\n", v.Name, v.Version)
-			if StringSliceContains(supported, v.Name) {
+			fmt.Printf("composer detected %s %s\n", v.Name, v.Version)
+			if pkgs.config.expectAllDetected || StringSliceContains(supported, v.Name) {
 				var version string
 
 				// remove any 'v' from front of version string
@@ -349,9 +367,9 @@ func (pkgs *PhpPackagesCollection) GatherInstalledPackages() ([]PhpPackage, erro
 					version = v.Version
 				}
 				pkgs.packages = append(pkgs.packages, PhpPackage{v.Name, version})
-				//fmt.Printf("   -> %s in supported!\n", v.Name)
+				fmt.Printf("   -> %s in supported!\n", v.Name)
 			} else {
-				//fmt.Printf("   -> %s NOT in supported!\n", v.Name)
+				fmt.Printf("   -> %s NOT in supported!\n", v.Name)
 			}
 		}
 	} else if 1 < len(splitCmd) && "wp-cli.phar" == splitCmd[1] {

daemon/internal/newrelic/integration/test.go

@@ -637,6 +637,7 @@ func (t *Test) comparePhpPackages(harvest *newrelic.Harvest) {
 	var expectedPkgsCollection *PhpPackagesCollection
 	var expectNullPkgs bool
 	var version_overrides map[string]interface{}
+	var subsetMatch bool = false
 
 	if nil != t.phpPackagesConfig {
 		var err error
@@ -656,6 +657,19 @@ func (t *Test) comparePhpPackages(harvest *newrelic.Harvest) {
 				t.Fatal(err)
 				return
 			}
+
+			// Determine if we expect an exact match between expected and actual packages
+			// when using the composer API in the agent to detect packages it is possible
+			// it will return packages which the "composer show" command does not.  These
+			// are usually virtual or other types of packages which "composer show"
+			// decides to not show.
+			//
+			// Currently the test handles this by listing all the packages that the agent
+			// detected but "composer show" did not as "Notes" on the test results.
+			//
+			// Tests for the legacy package detection mechanism still expect an exact match.
+			//
+			subsetMatch = expectedPkgsCollection.config.expectAllDetected
 		}
 	} else {
 		// no configuration given for package (no EXPECT_PHP_PACKAGES in test case) so don't run test
@@ -690,22 +704,44 @@ func (t *Test) comparePhpPackages(harvest *newrelic.Harvest) {
 		if nil == expectedPackages {
 			t.Fail(fmt.Errorf("No expected PHP packages, harvest contains %+v\n", actualPackages))
 		}
-		// compare expected and actual lists of packages
+		// Compare expected and actual lists of packages
 		// since package names should be identical, iterate over
 		// expected list and compare element by element with same
 		// position in actual list.  Name and version should match.
 		// this works because the functions which generate these
 		// lists sort them by package name for us
-		if len(expectedPackages) != len(actualPackages) {
+		//
+		// As explained above - if testing results from the Composer API
+		// agent detection mechanism, we may not get an exact match so
+		// we relax the test that the lengths of the two lists are the same
+		if !subsetMatch && (len(expectedPackages) != len(actualPackages)) {
 			t.Fail(fmt.Errorf("Expected and actual php packages differ in length %d vs %d: expected %+v actual %+v",
 				len(expectedPackages), len(actualPackages), expectedPackages, actualPackages))
 			return
 		}
 		for i, _ := range expectedPackages {
-			if expectedPackages[i].Name == actualPackages[i].Name {
+			var matchingIdx int = -1
+			for j, pkg := range actualPackages {
+				fmt.Printf("Comparing %s to %s\n", pkg.Name, expectedPackages[i].Name)
+				if pkg.Name == expectedPackages[i].Name {
+					fmt.Printf("Match - index = %d\n", j)
+					matchingIdx = j
+					break
+				}
+			}
+
+			fmt.Printf("MatchingIdx: %d\n", matchingIdx)
+			fmt.Printf("expectedPatckages[%d]: %+v\n", i, expectedPackages[i])
+			if -1 != matchingIdx {
+				fmt.Printf("actualPackages[%d]: %+v\n", matchingIdx, actualPackages[matchingIdx])
+			} else {
+				fmt.Printf("no match in actualPackages!\n")
+			}
+
+			if -1 != matchingIdx {
 				testPackageNameOnly := false
 				if nil != expectedPkgsCollection.config.packageNameOnly {
-					testPackageNameOnly = StringSliceContains(expectedPkgsCollection.config.packageNameOnly, actualPackages[i].Name)
+					testPackageNameOnly = StringSliceContains(expectedPkgsCollection.config.packageNameOnly, actualPackages[matchingIdx].Name)
 					if testPackageNameOnly {
 						t.AddNote(fmt.Sprintf("Tested package name only for packages: %+v", expectedPkgsCollection.config.packageNameOnly))
 					}
@@ -720,23 +756,41 @@ func (t *Test) comparePhpPackages(harvest *newrelic.Harvest) {
 				}
 
 				if testPackageNameOnly {
-					if " " != actualPackages[i].Version {
+					if " " != actualPackages[matchingIdx].Version {
 						t.Fail(fmt.Errorf("Expected no package version and a package version was detected - expected \" \" actual %+v. ",
-							actualPackages[i].Version))
+							actualPackages[matchingIdx].Version))
 						return
 					} else {
 						continue
 					}
 				}
 
-				if expected_version == actualPackages[i].Version {
+				if expected_version == actualPackages[matchingIdx].Version {
 					continue
+				} else {
+					t.Fail(fmt.Errorf("Expected version %s does not match actual version %s for package %s",
+						expected_version, actualPackages[matchingIdx].Version, expectedPackages[i].Name))
+					return
 				}
 			}
-			t.Fail(fmt.Errorf("Expected and actual Php packages do not match: expected %+v actual %+v. Complete expected %v actual %v.",
-				expectedPackages[i], actualPackages[i], expectedPackages, actualPackages))
+			t.Fail(fmt.Errorf("Expected Php packages do not match any actual packages: expected %+v\nComplete:\nexpected %v\nactual %v.\n",
+				expectedPackages[i], expectedPackages, actualPackages))
 			return
 		}
+
+		// create notes for all packages in the actual list not in the expected list
+		for ii, _ := range actualPackages {
+			var found bool = false
+			for _, pkg := range expectedPackages {
+				if pkg.Name == actualPackages[ii].Name {
+					found = true
+					break
+				}
+			}
+			if !found {
+				t.AddNote(fmt.Sprintf("Detected package not in expected: %+v", actualPackages[ii]))
+			}
+		}
 	} else {
 		if nil != expectedPackages {
 			t.Fail(fmt.Errorf("Expected PHP packages %+v, harvest contains none\n", expectedPackages))