Merge pull request #16137 from fuweid/deflake-failpoint-cases

*: should return exitCode even if cmd isn't nil

pkg/expect/expect.go

@@ -229,6 +229,22 @@ func (ep *ExpectProcess) ExitCode() (int, error) {
 		return ep.exitCode, nil
 	}
 
+	if ep.exitErr != nil {
+		// If the child process panics or is killed, for instance, the
+		// goFailpoint triggers the exit event, the ep.cmd isn't nil and
+		// the exitCode will describe the case.
+		if ep.exitCode != 0 {
+			return ep.exitCode, nil
+		}
+
+		// If the wait4(2) in waitProcess returns error, the child
+		// process might be reaped if the process handles the SIGCHILD
+		// in other goroutine. It's unlikely in this repo. But we
+		// should return the error for log even if the child process
+		// is still running.
+		return 0, ep.exitErr
+	}
+
 	return 0, ErrProcessRunning
 }
 
@@ -274,7 +290,7 @@ func (ep *ExpectProcess) waitProcess() error {
 
 	ep.mu.Lock()
 	defer ep.mu.Unlock()
-	ep.exitCode = state.ExitCode()
+	ep.exitCode = exitCode(state)
 
 	if !state.Success() {
 		return fmt.Errorf("unexpected exit code [%d] after running [%s]", ep.exitCode, ep.cmd.String())
@@ -283,6 +299,16 @@ func (ep *ExpectProcess) waitProcess() error {
 	return nil
 }
 
+// exitCode returns correct exit code for a process based on signaled or exited.
+func exitCode(state *os.ProcessState) int {
+	status := state.Sys().(syscall.WaitStatus)
+
+	if status.Signaled() {
+		return 128 + int(status.Signal())
+	}
+	return status.ExitStatus()
+}
+
 // Wait waits for the process to finish.
 func (ep *ExpectProcess) Wait() {
 	ep.wg.Wait()

pkg/expect/expect_test.go

@@ -72,8 +72,8 @@ func TestExpectFuncTimeout(t *testing.T) {
 	}
 
 	err = ep.Close()
-	require.ErrorContains(t, err, "unexpected exit code [-1] after running [/usr/bin/tail -f /dev/null]")
-	require.Equal(t, -1, ep.exitCode)
+	require.ErrorContains(t, err, "unexpected exit code [143] after running [/usr/bin/tail -f /dev/null]")
+	require.Equal(t, 143, ep.exitCode)
 }
 
 func TestExpectFuncExitFailure(t *testing.T) {
@@ -108,8 +108,9 @@ func TestExpectFuncExitFailureStop(t *testing.T) {
 	})
 	require.ErrorContains(t, err, "unexpected exit code [1] after running [/usr/bin/tail -x]")
 	exitCode, err := ep.ExitCode()
-	require.Equal(t, 0, exitCode)
-	require.Equal(t, err, ErrProcessRunning)
+	require.Equal(t, 1, exitCode)
+	require.NoError(t, err)
+
 	if err := ep.Stop(); err != nil {
 		t.Fatal(err)
 	}
@@ -189,7 +190,7 @@ func TestSignal(t *testing.T) {
 	go func() {
 		defer close(donec)
 		err = ep.Close()
-		assert.ErrorContains(t, err, "unexpected exit code [-1]")
+		assert.ErrorContains(t, err, "unexpected exit code [130]")
 		assert.ErrorContains(t, err, "sleep 100")
 	}()
 	select {
@@ -198,3 +199,14 @@ func TestSignal(t *testing.T) {
 	case <-donec:
 	}
 }
+
+func TestExitCodeAfterKill(t *testing.T) {
+	ep, err := NewExpect("sleep", "100")
+	require.NoError(t, err)
+
+	ep.Signal(os.Kill)
+	ep.Wait()
+	code, err := ep.ExitCode()
+	assert.Equal(t, 137, code)
+	assert.NoError(t, err)
+}

tests/framework/e2e/etcd_process.go

@@ -246,7 +246,14 @@ func (ep *EtcdServerProcess) Wait(ctx context.Context) error {
 		defer close(ch)
 		if ep.proc != nil {
 			ep.proc.Wait()
-			ep.cfg.lg.Info("server exited", zap.String("name", ep.cfg.Name))
+
+			exitCode, exitErr := ep.proc.ExitCode()
+
+			ep.cfg.lg.Info("server exited",
+				zap.String("name", ep.cfg.Name),
+				zap.Int("code", exitCode),
+				zap.Error(exitErr),
+			)
 		}
 	}()
 	select {
@@ -262,11 +269,16 @@ func (ep *EtcdServerProcess) IsRunning() bool {
 	if ep.proc == nil {
 		return false
 	}
-	_, err := ep.proc.ExitCode()
+
+	exitCode, err := ep.proc.ExitCode()
 	if err == expect.ErrProcessRunning {
 		return true
 	}
-	ep.cfg.lg.Info("server exited", zap.String("name", ep.cfg.Name))
+
+	ep.cfg.lg.Info("server exited",
+		zap.String("name", ep.cfg.Name),
+		zap.Int("code", exitCode),
+		zap.Error(err))
 	ep.proc = nil
 	return false
 }