Simpler and safer spawn of dapr comands in tests

Signed-off-by: Albert Callarisa <albert@diagrid.io>

Author: acroca

tests/e2e/spawn/spawn.go

@@ -80,6 +80,8 @@ func Command(command string, arguments ...string) (string, error) {
 // CommandExecWithContext runs a command with its arguments, kills the command after context is done
 // and returns the combined stdout, stderr or the error.
 func CommandExecWithContext(ctx context.Context, command string, arguments ...string) (string, error) {
+	ctx, cancel := context.WithTimeout(ctx, 30*time.Second)
+	defer cancel()
 	cmd := exec.CommandContext(ctx, command, arguments...)
 	var b bytes.Buffer
 	cmd.Stdout = &b
@@ -90,27 +92,21 @@ func CommandExecWithContext(ctx context.Context, command string, arguments ...st
 		return "", fmt.Errorf("error starting command : %w", err)
 	}
 
-	waitErrChan := make(chan error, 1)
-	go func(errChan chan error) {
-		waitErr := cmd.Wait()
-		if waitErr != nil {
-			fmt.Printf("error waiting for command : %s\n", waitErr)
-		}
-		waitErrChan <- waitErr
-	}(waitErrChan)
-	ctx, cancel := context.WithTimeout(ctx, time.Second*20)
-	defer cancel()
-	<-ctx.Done()
-	if cmd.ProcessState == nil || !cmd.ProcessState.Exited() {
-		cmd.Process.Signal(syscall.SIGTERM)
-		time.Sleep(10 * time.Second)
-		if cmd.ProcessState == nil || !cmd.ProcessState.Exited() {
-			err = cmd.Process.Kill()
-			if err != nil {
-				return b.String(), fmt.Errorf("error killing command : %w", err)
+	// Send SIGTERM after 20 seconds for graceful shutdown
+	go func() {
+		select {
+		case <-ctx.Done():
+			// Do nothing, it's already stopped
+		case <-time.After(20 * time.Second):
+			if cmd.ProcessState == nil || !cmd.ProcessState.Exited() {
+				cmd.Process.Signal(syscall.SIGTERM)
 			}
 		}
-	}
+	}()
 
-	return b.String(), <-waitErrChan
+	err = cmd.Wait()
+	if err != nil {
+		return b.String(), fmt.Errorf("error waiting for command : %w", err)
+	}
+	return b.String(), nil
 }

tests/e2e/standalone/run_template_test.go

@@ -23,7 +23,6 @@ import (
 	"path/filepath"
 	"strings"
 	"testing"
-	"time"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -52,7 +51,6 @@ func TestRunWithTemplateFile(t *testing.T) {
 		t.Cleanup(func() {
 			// assumption in the test is that there is only one set of app and daprd logs in the logs directory.
 			cleanUpLogs()
-			waitAppsToBeStopped()
 		})
 		args := []string{
 			"-f", runFilePath,
@@ -99,7 +97,6 @@ func TestRunWithTemplateFile(t *testing.T) {
 		t.Cleanup(func() {
 			// assumption in the test is that there is only one set of app and daprd logs in the logs directory.
 			cleanUpLogs()
-			waitAppsToBeStopped()
 		})
 		args := []string{
 			"-f", runFilePath,
@@ -153,7 +150,6 @@ func TestRunWithTemplateFile(t *testing.T) {
 		t.Cleanup(func() {
 			// assumption in the test is that there is only one set of app and daprd logs in the logs directory.
 			cleanUpLogs()
-			waitAppsToBeStopped()
 		})
 		args := []string{
 			"-f", runFilePath,
@@ -201,7 +197,6 @@ func TestRunWithTemplateFile(t *testing.T) {
 		t.Cleanup(func() {
 			// assumption in the test is that there is only one set of app and daprd logs in the logs directory.
 			cleanUpLogs()
-			waitAppsToBeStopped()
 		})
 		args := []string{
 			"-f", runFilePath,
@@ -224,7 +219,7 @@ func TestRunWithTemplateFile(t *testing.T) {
 			appID:          "processor",
 			baseLogDirPath: "../../apps/processor/.dapr/logs",
 			appLogContents: []string{
-				"Starting server in port 9086...",
+				"Starting server in port 9081...",
 				"termination signal received: shutting down",
 			},
 			daprdLogContent: []string{
@@ -250,7 +245,6 @@ func TestRunWithTemplateFile(t *testing.T) {
 		t.Cleanup(func() {
 			// assumption in the test is that there is only one set of app and daprd logs in the logs directory.
 			cleanUpLogs()
-			waitAppsToBeStopped()
 		})
 		args := []string{
 			"-f", runFilePath,
@@ -269,7 +263,7 @@ func TestRunWithTemplateFile(t *testing.T) {
 			appID:          "processor",
 			baseLogDirPath: "../../apps/processor/.dapr/logs",
 			appLogContents: []string{
-				"Starting server in port 9084...",
+				"Starting server in port 9081...",
 				"termination signal received: shutting down",
 			},
 			daprdLogContent: []string{
@@ -296,7 +290,6 @@ func TestRunWithTemplateFile(t *testing.T) {
 		t.Cleanup(func() {
 			// assumption in the test is that there is only one set of app and daprd logs in the logs directory.
 			cleanUpLogs()
-			waitAppsToBeStopped()
 		})
 		args := []string{
 			"-f", runFilePath,
@@ -408,7 +401,3 @@ func lookUpFileFullName(dirPath, partialFilename string) (string, error) {
 	}
 	return "", fmt.Errorf("failed to find file with partial name %s in directory %s", partialFilename, dirPath)
 }
-
-func waitAppsToBeStopped() {
-	time.Sleep(15 * time.Second)
-}

tests/e2e/testdata/run-template-files/app_output_to_only_console.yaml

@@ -1,12 +1,12 @@
 version: 1
 apps:
   - appDirPath: ../../../apps/processor/
-    appPort: 9082
+    appPort: 9081
     daprHTTPPort: 3510
     command: ["go","run", "app.go"]
     appLogDestination: console
     env:
-      APP_PORT: 9082
+      APP_PORT: 9081
   - appID: emit-metrics
     appDirPath: ../../../apps/emit-metrics/
     daprHTTPPort: 3511

tests/e2e/testdata/run-template-files/dapr.yaml

@@ -2,12 +2,12 @@ version: 1
 name: test_dapr_template
 apps:
   - appDirPath: ../../../apps/processor/
-    appPort: 9083
+    appPort: 9081
     daprHTTPPort: 3510
     command: ["go","run", "app.go"]
     appLogDestination: file
     env:
-      APP_PORT: 9083
+      APP_PORT: 9081
   - appID: emit-metrics
     appDirPath: ../../../apps/emit-metrics/
     daprHTTPPort: 3511

tests/e2e/testdata/run-template-files/empty_app_command.yaml

@@ -1,12 +1,12 @@
 version: 1
 apps:
   - appDirPath: ../../../apps/processor/
-    appPort: 9084
+    appPort: 9081
     daprHTTPPort: 3510
     command: ["go","run", "app.go"]
     appLogDestination: file
     env:
-      APP_PORT: 9084
+      APP_PORT: 9081
   - appID: emit-metrics
     appDirPath: ../../../apps/emit-metrics/
     daprHTTPPort: 3511

tests/e2e/testdata/run-template-files/env_var_not_set_dapr.yaml

@@ -1,12 +1,12 @@
 version: 1
 apps:
   - appDirPath: ../../../apps/processor/
-    appPort: 9085
+    appPort: 9081
     daprHTTPPort: 3510
     appLogDestination: file
     command: ["go","run", "app.go"]
     env:
-      APP_PORT: 9085
+      APP_PORT: 9081
   - appID: emit-metrics
     appDirPath: ../../../apps/emit-metrics/
     daprHTTPPort: 3511  # required env var DAPR_HOST_ADD not set in the yaml file

tests/e2e/testdata/run-template-files/no_app_command.yaml

@@ -6,12 +6,12 @@ common:
   configFilePath: appconfig.yaml
 apps:
   - appDirPath: ../../../apps/processor/
-    appPort: 9086
+    appPort: 9081
     daprHTTPPort: 3510
     command: ["go","run", "app.go"]
     appLogDestination: file
     env:
-      APP_PORT: 9086
+      APP_PORT: 9081
   - appID: emit-metrics
     appDirPath: ../../../apps/emit-metrics/
     daprHTTPPort: 3511

tests/e2e/testdata/run-template-files/wrong_emit_metrics_app_dapr.yaml

@@ -1,12 +1,12 @@
 version: 1
 apps:
   - appDirPath: ../../../apps/processor/
-    appPort: 9087
+    appPort: 9081
     daprHTTPPort: 3510
     command: ["go","run", "app.go"]
     appLogDestination: file
     env:
-      APP_PORT: 9087
+      APP_PORT: 9081
   - appID: emit-metrics
     appDirPath: ../../../apps/emit-metrics/
     daprHTTPPort: 3511