ExpectProcess's Stop method uses 'strings.Contains' to check
the returned err, however, this can be avoided. os.ErrProcessDone's
error message is the same as the hardcoded string. So I think
this explicit error is what this method wants to compare.
Signed-off-by: Jes Cok <xigua67damn@gmail.com>
Let's say there is command which outputs one line and exit with error.
There are three goroutines to acquire the lock:
1. ep.read()
2. ep.waitSaveExitErr()
3. ep.Expect()
When ep.read goroutine reads the log but it doesn't acquire the lock in
time, the ep.waitSaveExitErr acquires the lock and updates the
`exitErr`. And then ep.Expect acquires lock but it doesn't see any log
yet and then returns err.
It's hard to reproduce it in local. Add the extra sleep can reproduce it.
```diff
diff --git a/pkg/expect/expect.go b/pkg/expect/expect.go
index a512a3ce4..602bea73f 100644
--- a/pkg/expect/expect.go
+++ b/pkg/expect/expect.go
@@ -128,6 +128,7 @@ func (ep *ExpectProcess) tryReadNextLine(r *bufio.Reader) error {
printDebugLines := os.Getenv("EXPECT_DEBUG") != ""
l, err := r.ReadString('\n')
+ time.Sleep(10 * time.Millisecond)
ep.mu.Lock()
defer ep.mu.Unlock()
```
See it once in Github Action [1]. In order to fix it, the patch introduces
`readCloseCh` to wait for ep.read to get all the data and retry it.
[1]: https://github.com/etcd-io/etcd/pull/16137#issuecomment-1605838518
Signed-off-by: Wei Fu <fuweid89@gmail.com>
For the pkg/expect package, if the process has been stopped but there is
no `Close()` call, the `ExitCode()` won't return exit code correctly.
The `ExitCode()` should check `exitErr` and return exit code if cmd isn't nil.
And introduces `exitCode` to return correct exit code based on the
process is signaled or exited.
Signed-off-by: Wei Fu <fuweid89@gmail.com>
ExpectProcess and ExpectFunc now take the exit code of the process into
account, not just the matching of the tty output.
This also refactors the many tests that were previously succeeding on
matching an output from a failing cmd execution.
Signed-off-by: Thomas Jungblut <tjungblu@redhat.com>
This should aid in debugging test flakes, especially in tests where the process is restarted very often and thus changes its pid.
Now it's a lot easier to grep for different members, also when different tests fail at the same time.
The test TestDowngradeUpgradeClusterOf3 as mentioned in #13167 is a good example for that.
Signed-off-by: Thomas Jungblut <tjungblu@redhat.com>
The fix is needed to mitigate consequences of
https://github.com/golang/go/issues/29458 "golang breaking change" that
causes following test failures on etcd end:
--- FAIL: TestCtlV2Set (0.00s)
ctl_v2_test.go:552: could not start etcd process cluster (fork/exec ../../bin/etcd: Setctty set but Ctty not valid in child)
--- FAIL: TestCtlV2SetQuorum (0.00s)
ctl_v2_test.go:552: could not start etcd process cluster (fork/exec ../../bin/etcd: Setctty set but Ctty not valid in child)
--- FAIL: TestCtlV2SetClientTLS (0.00s)
ctl_v2_test.go:552: could not start etcd process cluster (fork/exec ../../bin/etcd: Setctty set but Ctty not valid in child)
bufio.NewReader.ReadString blocks even
when the process received syscall.SIGKILL.
Remove ptyMu mutex and make ReadString return
when *os.File is closed.
Signed-off-by: Gyuho Lee <gyuhox@gmail.com>
add and expose StopSignal to ExpectProcess allows user
to define what signal to send on ExpectProcess.close()
coverage testing code sets StopSignal to SIGTERM allowing
the test binary to shutdown gracefully so that it can generate
a coverage report.
