From 2f3ff6ced824296d903fd342b14e1a192ff0b986 Mon Sep 17 00:00:00 2001
From: Marcin Tojek <mtojek@users.noreply.github.com>
Date: Wed, 7 Dec 2022 14:18:09 +0100
Subject: [PATCH] fix: improve pty and ptytest (#5327)

* Fix: improve ptytest

* Disable skip

* WIP

* WIP

* WIP

* WIP

* WIP

* WIP

* WIP

* WIP

* Fix

* WIP

* Fix

* fix: pty close on Windows

* Revert changes around workflow

* fix
---
 pty/pty_windows.go     | 8 ++++----
 pty/ptytest/ptytest.go | 9 ++++++++-
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/pty/pty_windows.go b/pty/pty_windows.go
index 7cdbf0316d..be2de2159f 100644
--- a/pty/pty_windows.go
+++ b/pty/pty_windows.go
@@ -130,16 +130,16 @@ func (p *ptyWindows) Close() error {
 		return nil
 	}
 	p.closed = true
-	_ = p.outputWrite.Close()
-	_ = p.outputRead.Close()
-	_ = p.inputWrite.Close()
-	_ = p.inputRead.Close()
 
 	ret, _, err := procClosePseudoConsole.Call(uintptr(p.console))
 	if ret < 0 {
 		return xerrors.Errorf("close pseudo console: %w", err)
 	}
 
+	_ = p.outputWrite.Close()
+	_ = p.outputRead.Close()
+	_ = p.inputWrite.Close()
+	_ = p.inputRead.Close()
 	return nil
 }
 
diff --git a/pty/ptytest/ptytest.go b/pty/ptytest/ptytest.go
index bc26585f29..1bca67fe71 100644
--- a/pty/ptytest/ptytest.go
+++ b/pty/ptytest/ptytest.go
@@ -50,8 +50,14 @@ func create(t *testing.T, ptty pty.PTY, name string) *PTY {
 		ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort)
 		defer cancel()
 
+		logf(t, name, "close logw on cleanup")
 		_ = logw.Close()
+
+		logf(t, name, "close logr on cleanup")
 		_ = logr.Close()
+
+		logf(t, name, "logr and logw closed")
+
 		select {
 		case <-ctx.Done():
 			fatalf(t, name, "cleanup", "log pipe did not close in time")
@@ -74,6 +80,7 @@ func create(t *testing.T, ptty pty.PTY, name string) *PTY {
 
 		// Close pty only so that the copy goroutine can consume the
 		// remainder of it's buffer and then exit.
+		logf(t, name, "close pty on cleanup")
 		err := ptty.Close()
 		// Pty may already be closed, so don't fail the test, but log
 		// the error in case it's significant.
@@ -152,7 +159,7 @@ func (p *PTY) ExpectMatch(str string) string {
 		p.logf("matched %q = %q", str, buffer.String())
 		return buffer.String()
 	case <-timeout.Done():
-		// Ensure gorouine is cleaned up before test exit.
+		// Ensure goroutine is cleaned up before test exit.
 		_ = p.out.closeErr(p.Close())
 		<-match