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>
Not all systems include binaries in the same location. On my (NixOS, so
albeit a little weird) system these binaries exist in very different
locations.
This test switches to looking up the paths from the users PATH or skips
the test if they do not exist to improve the `make test` experience on
such systems.
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)
Update to Go 1.12.5 testing. Remove deprecated unused and gosimple
pacakges, and mask staticcheck 1006. Also, fix unconvert errors related
to unnecessary type conversions and following staticcheck errors:
- remove redundant return statements
- use for range instead of for select
- use time.Since instead of time.Now().Sub
- omit comparison to bool constant
- replace T.Fatal and T.Fatalf in tests with T.Error and T.Fatalf respectively because the goroutine calls T.Fatal must be called in the same goroutine as the test
- fix error strings that should not be capitalized
- use sort.Strings(...) instead of sort.Sort(sort.StringSlice(...))
- use he status code of Canceled instead of grpc.ErrClientConnClosing which is deprecated
- use use status.Errorf instead of grpc.Errorf which is deprecated
Related #10528#10438
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.
