From 08582ac4a8e68694414b02d4489af6fe621e49ce Mon Sep 17 00:00:00 2001 From: Daisuke Maki Date: Sat, 17 Dec 2016 06:48:16 +0900 Subject: [PATCH 1/3] fix #376 --- peco.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/peco.go b/peco.go index 8fa57c2..3966a66 100644 --- a/peco.go +++ b/peco.go @@ -219,6 +219,10 @@ func (p *Peco) Err() error { } func (p *Peco) Exit(err error) { + if pdebug.Enabled { + g := pdebug.Marker("Peco.Exit (err = %s)", err) + defer g.End() + } p.err = err if cf := p.cancelFunc; cf != nil { cf() @@ -348,7 +352,7 @@ func (p *Peco) Run(ctx context.Context) (err error) { if b := p.CurrentLineBuffer(); b.Size() == 1 { if l, err := b.LineAt(0); err == nil { p.resultCh = make(chan line.Line) - p.Exit(nil) + p.Exit(errCollectResults{}) p.resultCh <- l close(p.resultCh) } From d8c7102efde338b29deeb32d29888129de70e34a Mon Sep 17 00:00:00 2001 From: Daisuke Maki Date: Sat, 17 Dec 2016 07:13:09 +0900 Subject: [PATCH 2/3] fix the test that should have caught the problem --- peco_test.go | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/peco_test.go b/peco_test.go index b6f7c75..2d645c7 100644 --- a/peco_test.go +++ b/peco_test.go @@ -264,6 +264,9 @@ func TestApplyConfig(t *testing.T) { } } +// While this issue is labeled for Issue363, it tests against 376 as well. +// The test should have caught the bug for 376, but the premise of the test +// itself was wrong func TestGHIssue363(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), time.Second) defer cancel() @@ -273,15 +276,25 @@ func TestGHIssue363(t *testing.T) { p.Stdin = bytes.NewBufferString("foo\n") var out bytes.Buffer p.Stdout = &out - if !assert.NoError(t, p.Run(ctx), "p.Run should succeed") { - return - } + + resultCh := make(chan error) + go func() { + defer close(resultCh) + select { + case <-ctx.Done(): + return + case resultCh <- p.Run(ctx): + return + } + }() select { case <-ctx.Done(): - t.Errorf("we should get here before being canceled") - return - default: + t.Errorf("timeout reached") + case err := <-resultCh: + if !assert.True(t, util.IsCollectResultsError(err), "isCollectResultsError") { + return + } } if !assert.NotEqual(t, "foo\n", out.String(), "output should match") { From 9f1366c9344ab1026451c82166e7e2dbae178984 Mon Sep 17 00:00:00 2001 From: Daisuke Maki Date: Sat, 17 Dec 2016 07:17:41 +0900 Subject: [PATCH 3/3] early return --- peco_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/peco_test.go b/peco_test.go index 2d645c7..465c0e4 100644 --- a/peco_test.go +++ b/peco_test.go @@ -291,6 +291,7 @@ func TestGHIssue363(t *testing.T) { select { case <-ctx.Done(): t.Errorf("timeout reached") + return case err := <-resultCh: if !assert.True(t, util.IsCollectResultsError(err), "isCollectResultsError") { return