[Heartbeat] Fix broken invocation of synth package (backport #26228) (#…

…26250) (#27083)

* [Heartbeat] Fix broken invocation of synth package (#26228)

Fixes invocation of synthetics broken in #26188

Additionally, for dev purposes, this lets accepts a synthetics version of file:/// as valid to help with debugging development versions of the synthetics agent.

Never released, so no changelog needed

Still just sticking with unit tests here since testing more deeply with synthetics (esp. master where this is a problem) is a larger problem than we're equipped to handle ATM.

(cherry picked from commit ad7c19f)

# Conflicts:
#	x-pack/heartbeat/monitors/browser/source/validatepackage.go
#	x-pack/heartbeat/monitors/browser/source/validatepackage_test.go

* Fix license

* Fix notice

Co-authored-by: Andrew Cholakian <[email protected]>
(cherry picked from commit 5f49358)

Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>

x-pack/heartbeat/monitors/browser/source/validatepackage.go

@@ -42,6 +42,10 @@ func parseVersion(version string) string {
 }
 
 func validateVersion(expected string, current string) error {
+	if strings.HasPrefix(current, "file://") {
+		return nil
+	}
+
 	expectedRange, err := semver.NewConstraint(expected)
 	if err != nil {
 		return err
@@ -75,6 +79,7 @@ func validatePackageJSON(path string) error {
 	if synthVersion == "" {
 		synthVersion = pkgJson.DevDependencies.SynthVersion
 	}
+
 	err = validateVersion(ExpectedSynthVersion, synthVersion)
 	if err != nil {
 		return fmt.Errorf("could not validate @elastic/synthetics version: '%s' %w", synthVersion, err)

x-pack/heartbeat/monitors/browser/source/validatepackage_test.go

@@ -70,6 +70,11 @@ func TestValidateVersion(t *testing.T) {
 			current:   "0.0.1-alpha.11",
 			shouldErr: false,
 		},
+		{
+			expected:  "",
+			current:   "file://blahblahblah",
+			shouldErr: false,
+		},
 	}
 
 	for _, tt := range tests {

x-pack/heartbeat/monitors/browser/synthexec/synthexec.go

@@ -30,12 +30,12 @@ const debugSelector = "synthexec"
 func SuiteJob(ctx context.Context, suitePath string, params common.MapStr, extraArgs ...string) (jobs.Job, error) {
 	// Run the command in the given suitePath, use '.' as the first arg since the command runs
 	// in the correct dir
-	newCmd, err := suiteCommandFactory(suitePath, append(extraArgs, ".")...)
+	cmdFactory, err := suiteCommandFactory(suitePath, extraArgs...)
 	if err != nil {
 		return nil, err
 	}
 
-	return startCmdJob(ctx, newCmd, nil, params), nil
+	return startCmdJob(ctx, cmdFactory, nil, params), nil
 }
 
 func suiteCommandFactory(suitePath string, args ...string) (func() *exec.Cmd, error) {
@@ -46,7 +46,11 @@ func suiteCommandFactory(suitePath string, args ...string) (func() *exec.Cmd, er
 
 	newCmd := func() *exec.Cmd {
 		bin := filepath.Join(npmRoot, "node_modules/.bin/elastic-synthetics")
-		cmd := exec.Command(bin, args...)
+		// Always put the suite path first to prevent conflation with variadic args!
+		// See https://github.com/tj/commander.js/blob/master/docs/options-taking-varying-arguments.md
+		// Note, we don't use the -- approach because it's cleaner to always know we can add new options
+		// to the end.
+		cmd := exec.Command(bin, append([]string{suitePath}, args...)...)
 		cmd.Dir = npmRoot
 		return cmd
 	}

x-pack/heartbeat/monitors/browser/synthexec/synthexec_test.go

@@ -157,3 +157,56 @@ Loop:
 		})
 	}
 }
+
+func TestSuiteCommandFactory(t *testing.T) {
+	_, filename, _, _ := runtime.Caller(0)
+	origPath := path.Join(filepath.Dir(filename), "../source/fixtures/todos")
+	suitePath, err := filepath.Abs(origPath)
+	require.NoError(t, err)
+	binPath := path.Join(suitePath, "node_modules/.bin/elastic-synthetics")
+
+	tests := []struct {
+		name      string
+		suitePath string
+		extraArgs []string
+		want      []string
+		wantErr   bool
+	}{
+		{
+			"no args",
+			suitePath,
+			nil,
+			[]string{binPath, suitePath},
+			false,
+		},
+		{
+			"with args",
+			suitePath,
+			[]string{"--capability", "foo", "bar", "--rich-events"},
+			[]string{binPath, suitePath, "--capability", "foo", "bar", "--rich-events"},
+			false,
+		},
+		{
+			"no npm root",
+			"/not/a/path/for/sure",
+			nil,
+			nil,
+			true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			factory, err := suiteCommandFactory(tt.suitePath, tt.extraArgs...)
+
+			if tt.wantErr {
+				require.Error(t, err)
+				return
+			}
+			require.NoError(t, err)
+
+			cmd := factory()
+			got := cmd.Args
+			require.Equal(t, tt.want, got)
+		})
+	}
+}