Skip to content

fix: improve pty and ptytest #5327

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 17 commits into from
Dec 7, 2022
Merged

fix: improve pty and ptytest #5327

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
f823b02
Fix: improve ptytest
mtojek Dec 6, 2022
8ab62d4
Merge branch 'main' into fix-pty-1
mtojek Dec 6, 2022
a97a7ad
Disable skip
mtojek Dec 6, 2022
593a891
WIP
mtojek Dec 7, 2022
8a3f6e2
WIP
mtojek Dec 7, 2022
1a7070f
WIP
mtojek Dec 7, 2022
3afeca9
WIP
mtojek Dec 7, 2022
7ea26b6
WIP
mtojek Dec 7, 2022
14a79d1
WIP
mtojek Dec 7, 2022
04fce69
WIP
mtojek Dec 7, 2022
dd2cfd0
WIP
mtojek Dec 7, 2022
ea3b170
Fix
mtojek Dec 7, 2022
a042bfc
WIP
mtojek Dec 7, 2022
dc86c9a
Fix
mtojek Dec 7, 2022
e48d4ab
fix: pty close on Windows
mtojek Dec 7, 2022
eff5966
Revert changes around workflow
mtojek Dec 7, 2022
d88b552
fix
mtojek Dec 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
WIP
  • Loading branch information
@mtojek
mtojek committed Dec 7, 2022
commit a042bfc1adcfa872a7c19f1480edde0a54172f54
2 changes: 2 additions & 0 deletions .github/workflows/coder.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,7 @@ jobs:
gotestsum --junitfile="gotests.xml" --jsonfile="gotestsum.json" --packages="./..." --debug -- -parallel=8 -timeout=3m -short -failfast $COVERAGE_FLAGS

- uses: actions/upload-artifact@v3
if: success() || failure()
with:
name: gotestsum-debug-${{ matrix.os }}.json
path: ./gotestsum.json
Expand Down Expand Up @@ -406,6 +407,7 @@ jobs:
run: make test-postgres

- uses: actions/upload-artifact@v3
if: success() || failure()
with:
name: gotestsum-debug-postgres.json
path: ./gotestsum.json
Expand Down
8 changes: 0 additions & 8 deletions pty/pty_other.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,17 +114,9 @@ func (p *otherPty) Resize(height uint16, width uint16) error {
}

func (p *otherPty) Close() error {
if p.opts.logger != nil {
p.opts.logger.Println("Will close...")
}

p.mutex.Lock()
defer p.mutex.Unlock()

if p.opts.logger != nil {
p.opts.logger.Println("Closing...")
}

if p.closed {
return p.err
}
Expand Down
13 changes: 10 additions & 3 deletions pty/ptytest/ptytest.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,17 @@ func create(t *testing.T, ptty pty.PTY, name string) *PTY {
logDone := make(chan struct{})
logr, logw := io.Pipe()
t.Cleanup(func() {
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitMedium)
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")
Expand All @@ -69,11 +75,12 @@ func create(t *testing.T, ptty pty.PTY, name string) *PTY {
_ = out.closeErr(err)
}()
t.Cleanup(func() {
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitMedium)
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort)
defer cancel()

// 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.
Expand Down Expand Up @@ -119,7 +126,7 @@ type PTY struct {
func (p *PTY) ExpectMatch(str string) string {
p.t.Helper()

timeout, cancel := context.WithTimeout(context.Background(), testutil.WaitMedium)
timeout, cancel := context.WithTimeout(context.Background(), testutil.WaitShort)
defer cancel()

var buffer bytes.Buffer
Expand Down