use script to wait

acceptance/bin/wait_pid

@@ -13,13 +13,13 @@ wait_pid() {
         if [[ "$OSTYPE" == "msys"* || "$OSTYPE" == "cygwin"* ]]; then
             # Windows approach
             if ! tasklist | grep -q $pid; then
-                echo "Process has ended"
+                echo "[wait_pid] process has ended"
                 return 0
             fi
         else
             # Linux/macOS approach
             if ! kill -0 $pid 2>/dev/null; then
-                echo "Process has ended"
+                echo "[wait_pid] process has ended"
                 return 0
             fi
         fi

acceptance/bundle/debug/out.stderr.txt

@@ -72,7 +72,7 @@
 10:07:59 Debug: Apply pid=12345 mutator=initialize mutator=seq mutator=metadata.AnnotatePipelines
 10:07:59 Debug: Apply pid=12345 mutator=initialize mutator=seq mutator=terraform.Initialize
 10:07:59 Debug: Using Terraform from DATABRICKS_TF_EXEC_PATH at [TERRAFORM] pid=12345 mutator=initialize mutator=seq mutator=terraform.Initialize
-10:07:59 Debug: Using Terraform CLI config from DATABRICKS_TF_CLI_CONFIG_FILE at [DATABRICKS_TF_CLI_CONFIG_FILE] pid=12345 mutator=initialize mutator=seq mutator=terraform.Initialize
+10:07:59 Debug: DATABRICKS_TF_PROVIDER_VERSION as 1.62.0 does not match the current version 1.65.1, ignoring DATABRICKS_TF_CLI_CONFIG_FILE pid=12345 mutator=initialize mutator=seq mutator=terraform.Initialize
 10:07:59 Debug: Environment variables for Terraform: ...redacted... pid=12345 mutator=initialize mutator=seq mutator=terraform.Initialize
 10:07:59 Debug: Apply pid=12345 mutator=initialize mutator=seq mutator=scripts.postinit
 10:07:59 Debug: No script defined for postinit, skipping pid=12345 mutator=initialize mutator=seq mutator=scripts.postinit

acceptance/daemon/child-detaches/out.parentchild.txt

@@ -1,13 +1,7 @@
-Parent process has started
+[parent] started child
-Started  the child process
+[parent] input sent to child: Hello from the other side
-Provided input: Hello from the other side
+[parent] exiting
-Released the child process
+[child][wait_pid] process has ended
-Parent process is exiting
+[child] Parent process has exited
-
+[child] input from parent: Hello from the other side
-====================
-
-All output from this point on is from the child process
-Parent process has exited
-Received input from parent process:
-Hello from the other side
 

acceptance/daemon/child-detaches/output.txt

@@ -1,2 +1,2 @@
-waiting for child process to finish
+[script] waiting for child process to end...
-Process has ended
+[script][wait_pid] process has ended

acceptance/daemon/child-detaches/script

@@ -1,8 +1,9 @@
 export DATABRICKS_CLI_SELFTEST_CHILD_OUTPUT_FILE="out.parentchild.txt"
 
-$CLI selftest parent &> out.parentchild.txt
+$CLI selftest parent > "out.parentchild.txt"
 
-echo "waiting for child process to finish"
+echo "[script] waiting for child process to end..."
+echo -n "[script]"
 wait_pid $(cat ./child.pid)
 
 rm ./child.pid

acceptance/daemon/child-discards-output/output.txt

@@ -1,9 +1,6 @@
-Parent process has started
+[parent] started child
-Started  the child process
+[parent] input sent to child: Hello from the other side
-Provided input: Hello from the other side
+[parent] exiting
-Released the child process
+[script] waiting for child process to end...
-Parent process is exiting
+[script][wait_pid] process has ended
-
+[script] child process should not have written any output.
-wait for child process to finish
-Process has ended
-the child process should not have written any output.

acceptance/daemon/child-discards-output/script

@@ -1,9 +1,9 @@
 $CLI selftest parent
 
-echo "
+echo "[script] waiting for child process to end..."
-wait for child process to finish"
+echo -n "[script]"
 wait_pid $(cat ./child.pid)
 
-echo "the child process should not have written any output."
+echo "[script] child process should not have written any output."
 
 rm ./child.pid

cmd/selftest/child.go

@@ -1,14 +1,12 @@
 package selftest
 
 import (
-	"errors"
 	"fmt"
 	"io"
 	"os"
-	"strconv"
+	"os/exec"
 
 	"github.com/databricks/cli/libs/daemon"
-	"github.com/databricks/cli/libs/process"
 	"github.com/spf13/cobra"
 )
 
@@ -17,27 +15,20 @@ func newChildCommand() *cobra.Command {
 	return &cobra.Command{
 		Use: "child",
 		RunE: func(cmd *cobra.Command, args []string) error {
-			parentPid, err := strconv.Atoi(os.Getenv(daemon.DatabricksCliParentPid))
+			waitCmd := exec.Command("bash", "-euo", "pipefail", "wait_pid", os.Getenv(daemon.DatabricksCliParentPid))
+			b, err := waitCmd.Output()
 			if err != nil {
-				return fmt.Errorf("failed to parse parent PID: %w", err)
-			}
-
-			err = process.Wait(parentPid)
-			if err != nil && !errors.As(err, &process.ErrProcessNotFound{}) {
 				return fmt.Errorf("failed to wait for parent process: %w", err)
 			}
-
+			fmt.Print("[child]" + string(b))
-			fmt.Println("\n====================")
+			fmt.Println("[child] Parent process has exited")
-			fmt.Println("\nAll output from this point on is from the child process")
-			fmt.Println("Parent process has exited")
 
 			in, err := io.ReadAll(os.Stdin)
 			if err != nil {
 				return fmt.Errorf("failed to read from stdin: %w", err)
 			}
 
-			fmt.Println("Received input from parent process:")
+			fmt.Println("[child] input from parent: " + string(in))
-			fmt.Println(string(in))
 			return nil
 		},
 	}

cmd/selftest/parent.go

@@ -14,8 +14,6 @@ func newParentCommand() *cobra.Command {
 	return &cobra.Command{
 		Use: "parent",
 		RunE: func(cmd *cobra.Command, args []string) error {
-			fmt.Println("Parent process has started")
-
 			d := daemon.Daemon{
 				Env:         os.Environ(),
 				Args:        []string{"selftest", "child"},
@@ -27,21 +25,19 @@ func newParentCommand() *cobra.Command {
 			if err != nil {
 				return fmt.Errorf("failed to start child process: %w", err)
 			}
-			fmt.Println("Started  the child process")
+			fmt.Println("[parent] started child")
 
 			err = d.WriteInput([]byte("Hello from the other side\n"))
 			if err != nil {
 				return fmt.Errorf("failed to write to child process: %w", err)
 			}
-			fmt.Println("Provided input: Hello from the other side")
+			fmt.Println("[parent] input sent to child: Hello from the other side")
 
 			err = d.Release()
 			if err != nil {
 				return fmt.Errorf("failed to release child process: %w", err)
 			}
-			fmt.Println("Released the child process")
+			fmt.Println("[parent] exiting")
-
-			fmt.Println("Parent process is exiting")
 			return nil
 		},
 	}

libs/process/wait.go

@@ -1,15 +0,0 @@
-package process
-
-import "fmt"
-
-type ErrProcessNotFound struct {
-	Pid int
-}
-
-func (e ErrProcessNotFound) Error() string {
-	return fmt.Sprintf("process with pid %d does not exist", e.Pid)
-}
-
-func Wait(pid int) error {
-	return waitForPid(pid)
-}

libs/process/wait_unix.go

@@ -1,36 +0,0 @@
-//go:build linux || darwin
-
-package process
-
-import (
-	"errors"
-	"os"
-	"syscall"
-	"time"
-)
-
-func waitForPid(pid int) error {
-	p, err := os.FindProcess(pid)
-	if err != nil {
-		return err
-	}
-
-	// Initial existence check.
-	if err := p.Signal(syscall.Signal(0)); err != nil {
-		if errors.Is(err, os.ErrProcessDone) {
-			return ErrProcessNotFound{Pid: pid}
-		}
-		return err
-	}
-
-	// Polling loop until process exits
-	for {
-		if err := p.Signal(syscall.Signal(0)); err != nil {
-			if errors.Is(err, os.ErrProcessDone) {
-				return nil
-			}
-			return err
-		}
-		time.Sleep(100 * time.Millisecond)
-	}
-}

libs/process/wait_unix_test.go

@@ -1,71 +0,0 @@
-package process
-
-import (
-	"os"
-	"os/exec"
-	"path/filepath"
-	"runtime"
-	"strconv"
-	"testing"
-	"time"
-
-	"github.com/databricks/cli/internal/testutil"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
-)
-
-func TestWaitUnix(t *testing.T) {
-	if runtime.GOOS == "windows" {
-		t.Skip("Skipping test on Windows")
-	}
-
-	t.Parallel()
-	tmpDir := t.TempDir()
-
-	pidFile := filepath.Join(tmpDir, "child.pid")
-	outputFile := filepath.Join(tmpDir, "output.txt")
-
-	// For this test, we cannot start the background process in this test itself
-	// and instead have to use parent.sh as an intermediary.
-	//
-	// This is because in Unix if we start the background process in this test itself,
-	// the background process will be a child of this test process and thus would
-	// need to be reaped by this test process (using the Wait function / syscall).
-	// Otherwise waitForPid will forever wait for the background process to finish.
-	//
-	// If we rely on an intermediate script to start the background process, the
-	// background process is reasigned to the init process (PID 1) once the parent
-	// exits and thus we can successfully wait for it in this test using waitForPid function.
-	cmd := exec.Command("./testdata/parent.sh", pidFile, outputFile)
-	err := cmd.Start()
-	require.NoError(t, err)
-
-	// Wait 5 seconds for the parent bash script to write the child's PID to the file.
-	var childPid int
-	require.Eventually(t, func() bool {
-		b, err := os.ReadFile(pidFile)
-		if err != nil {
-			return false
-		}
-
-		childPid, err = strconv.Atoi(string(b))
-		require.NoError(t, err)
-		return true
-	}, 2*time.Second, 100*time.Millisecond)
-
-	// The output file should not exist yet since the background process should
-	// still be running.
-	assert.NoFileExists(t, outputFile)
-
-	// Wait for the background process to finish.
-	err = waitForPid(childPid)
-	assert.NoError(t, err)
-
-	// The output file should exist now since the background process has finished.
-	testutil.AssertFileContents(t, outputFile, "abc\n")
-
-	// Since the background process has finished, waiting for it again should
-	// return an error.
-	err = waitForPid(childPid)
-	assert.Regexp(t, "process with pid .* does not exist", err.Error())
-}

libs/process/wait_windows.go

@@ -1,44 +0,0 @@
-//go:build windows
-
-package process
-
-import (
-	"errors"
-	"fmt"
-	"time"
-
-	"golang.org/x/sys/windows"
-)
-
-func waitForPid(pid int) error {
-	handle, err := windows.OpenProcess(
-		windows.SYNCHRONIZE|windows.PROCESS_QUERY_INFORMATION,
-		false,
-		uint32(pid),
-	)
-	if errors.Is(err, windows.ERROR_INVALID_PARAMETER) {
-		return ErrProcessNotFound{Pid: pid}
-	}
-	if err != nil {
-		return fmt.Errorf("OpenProcess failed: %v", err)
-	}
-	defer windows.CloseHandle(handle)
-
-	// Wait forever for the process to exit. Wait for 5 minutes max.
-	ret, err := windows.WaitForSingleObject(handle, uint32(5*time.Minute.Milliseconds()))
-	if err != nil {
-		return fmt.Errorf("Wait failed: %v", err)
-	}
-
-	switch ret {
-	case windows.WAIT_OBJECT_0:
-		return nil // Process exited
-	case 0x00000102:
-		// Standard library does not have have a constant defined for this
-		// so we use the hex value directly. This is the WAIT_TIMEOUT value.
-		// ref: https://learn.microsoft.com/en-us/windows/win32/api/synchapi/nf-synchapi-waitforsingleobject#return-value
-		return fmt.Errorf("process wait timed out")
-	default:
-		return fmt.Errorf("unexpected process wait return value: %d", ret)
-	}
-}