From 815d93d191a8870286b229adc26b86f9405a9f2f Mon Sep 17 00:00:00 2001 From: Blake Watters Date: Wed, 22 Apr 2020 05:39:05 -0700 Subject: [PATCH 01/22] Snapshot --- command/init_test.go | 52 +++++++++++++++++++++++++++++++----- test/interactive_terminal.go | 7 +++-- 2 files changed, 50 insertions(+), 9 deletions(-) diff --git a/command/init_test.go b/command/init_test.go index 3a4dd52..f65696b 100644 --- a/command/init_test.go +++ b/command/init_test.go @@ -16,13 +16,15 @@ package command_test import ( "fmt" + "io" + "os" "testing" "time" "github.com/AlecAivazis/survey/v2" "github.com/AlecAivazis/survey/v2/core" "github.com/AlecAivazis/survey/v2/terminal" - expect "github.com/Netflix/go-expect" + "github.com/Netflix/go-expect" "github.com/opsani/cli/command" "github.com/opsani/cli/test" "github.com/spf13/viper" @@ -56,10 +58,22 @@ func (s *InitTestSuite) TestRunningInitHelp() { func (s *InitTestSuite) TestTerminalInteraction() { var name string test.RunTestInInteractiveTerminal(s.T(), func(context *test.InteractiveExecutionContext) error { + pipe, _ := expect.NewPassthroughPipe(context.GetStdin()) + pipe.SetReadDeadline(time.Now().Add(100 * time.Millisecond)) + mock := &FakeFileReader{ + PassthroughPipe: pipe, + } + go func(stdin io.Reader) { + _, err := io.Copy(context.GetConsole(), stdin) + if err != nil { + context.GetConsole().Logf("failed to copy stdin: %s", err) + } + }(pipe) return survey.AskOne(&survey.Input{ Message: "What is your name?", - }, &name, survey.WithStdio(context.GetStdin(), context.GetStdout(), context.GetStderr())) + }, &name, survey.WithStdio(mock, context.GetStdout(), context.GetStderr())) }, func(_ *test.InteractiveExecutionContext, c *expect.Console) error { + // s.RequireNoErr2(c.ExpectString("? What is your name?")) c.ExpectString("? What is your name?") c.SendLine("Blake Watters") c.ExpectEOF() @@ -68,19 +82,43 @@ func (s *InitTestSuite) TestTerminalInteraction() { s.Require().Equal(name, "Blake Watters") } +func (s *InitTestSuite) RequireNoErr2(_ interface{}, err error) { + s.Require().NoError(err) +} + +type FakeFileReader struct { + *expect.PassthroughPipe + file *os.File +} + +// TODO: These can alias to temp files +func (s *FakeFileReader) Fd() uintptr { + return s.file.Fd() +} + func (s *InitTestSuite) TestTerminalConfirm() { - var confirmed bool + // s.T().Parallel() + var confirmed bool = true test.RunTestInInteractiveTerminal(s.T(), func(context *test.InteractiveExecutionContext) error { + file := os.NewFile(context.GetStdin().Fd(), "pipe") + pipe, _ := expect.NewPassthroughPipe(context.GetStdin()) + pipe.SetReadDeadline(time.Now().Add(500 * time.Millisecond)) + mock := &FakeFileReader{ + file: file, + PassthroughPipe: pipe, + } return survey.AskOne(&survey.Confirm{ - Message: "Delete file?", - }, &confirmed, survey.WithStdio(context.GetStdin(), context.GetStdout(), context.GetStderr())) + Message: "Delete file?", + }, &confirmed, survey.WithStdio(mock, context.GetStdout(), context.GetStderr())) }, func(_ *test.InteractiveExecutionContext, c *expect.Console) error { + // c.Expect(expect.RegexpPattern("Delete file?")) c.ExpectString("? Delete file?") - c.SendLine("Y") + c.SendLine("N") c.ExpectEOF() return nil }) - s.Require().True(confirmed) + panic("adass") + s.Require().False(confirmed) } func (s *InitTestSuite) TestInitWithExistingConfig() { diff --git a/test/interactive_terminal.go b/test/interactive_terminal.go index 798a8f6..f67c90d 100644 --- a/test/interactive_terminal.go +++ b/test/interactive_terminal.go @@ -16,6 +16,7 @@ package test import ( "bytes" + "fmt" "os" "strings" "testing" @@ -33,7 +34,6 @@ func RunTestInInteractiveTerminal(t *testing.T, codeUnderTestFunc InteractiveProcessFunc, testFunc InteractiveUserFunc, consoleOpts ...expect.ConsoleOpt) (*InteractiveExecutionContext, error) { - context, err := ExecuteInInteractiveTerminal(codeUnderTestFunc, testFunc, consoleOpts...) t.Logf("Raw output: %q", context.GetOutputBuffer().String()) t.Logf("\n\nterminal state: %s", expect.StripTrailingEmptyLines(context.GetTerminalState().String())) @@ -114,7 +114,7 @@ func ExecuteInInteractiveTerminal( console, terminalState, err := vt10x.NewVT10XConsole( append([]expect.ConsoleOpt{ expect.WithStdout(outputBuffer), - expect.WithDefaultTimeout(60 * time.Second), + expect.WithDefaultTimeout(300 * time.Millisecond), }, consoleOpts...)...) if err != nil { return nil, err @@ -137,6 +137,9 @@ func ExecuteInInteractiveTerminal( // Run the process for the user to interact with err = processFunc(executionContext) + if err != nil { + fmt.Println("Process failed", err) + } // Close the slave end of the pty, and read the remaining bytes from the master end. console.Tty().Close() From bddb0d402e89519fdc9fa0e59308f2acb184f10a Mon Sep 17 00:00:00 2001 From: Blake Watters Date: Wed, 22 Apr 2020 05:42:59 -0700 Subject: [PATCH 02/22] Improve snapshot --- command/init_test.go | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/command/init_test.go b/command/init_test.go index f65696b..8a4f8e2 100644 --- a/command/init_test.go +++ b/command/init_test.go @@ -97,7 +97,6 @@ func (s *FakeFileReader) Fd() uintptr { } func (s *InitTestSuite) TestTerminalConfirm() { - // s.T().Parallel() var confirmed bool = true test.RunTestInInteractiveTerminal(s.T(), func(context *test.InteractiveExecutionContext) error { file := os.NewFile(context.GetStdin().Fd(), "pipe") @@ -108,16 +107,15 @@ func (s *InitTestSuite) TestTerminalConfirm() { PassthroughPipe: pipe, } return survey.AskOne(&survey.Confirm{ - Message: "Delete file?", + Message: "Delete file?", }, &confirmed, survey.WithStdio(mock, context.GetStdout(), context.GetStderr())) }, func(_ *test.InteractiveExecutionContext, c *expect.Console) error { - // c.Expect(expect.RegexpPattern("Delete file?")) - c.ExpectString("? Delete file?") + c.Expect(expect.RegexpPattern("Delete file?")) c.SendLine("N") c.ExpectEOF() return nil }) - panic("adass") + // panic("adass") s.Require().False(confirmed) } From c6cb4c7d99c929b60ecaf297c3dfa31379f970cc Mon Sep 17 00:00:00 2001 From: Blake Watters Date: Wed, 22 Apr 2020 05:55:31 -0700 Subject: [PATCH 03/22] regex reqs --- command/init_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/command/init_test.go b/command/init_test.go index 8a4f8e2..c44bb67 100644 --- a/command/init_test.go +++ b/command/init_test.go @@ -73,8 +73,8 @@ func (s *InitTestSuite) TestTerminalInteraction() { Message: "What is your name?", }, &name, survey.WithStdio(mock, context.GetStdout(), context.GetStderr())) }, func(_ *test.InteractiveExecutionContext, c *expect.Console) error { - // s.RequireNoErr2(c.ExpectString("? What is your name?")) - c.ExpectString("? What is your name?") + s.RequireNoErr2(c.ExpectString("What is your name?")) + // c.ExpectString("? What is your name?") c.SendLine("Blake Watters") c.ExpectEOF() return nil From 1488adc1e4ac5499ccd805f48f3e3f0e74b6c9f1 Mon Sep 17 00:00:00 2001 From: Blake Watters Date: Wed, 22 Apr 2020 05:58:29 -0700 Subject: [PATCH 04/22] better --- command/init_test.go | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/command/init_test.go b/command/init_test.go index c44bb67..9cebeee 100644 --- a/command/init_test.go +++ b/command/init_test.go @@ -16,7 +16,6 @@ package command_test import ( "fmt" - "io" "os" "testing" "time" @@ -58,17 +57,24 @@ func (s *InitTestSuite) TestRunningInitHelp() { func (s *InitTestSuite) TestTerminalInteraction() { var name string test.RunTestInInteractiveTerminal(s.T(), func(context *test.InteractiveExecutionContext) error { + // pipe, _ := expect.NewPassthroughPipe(context.GetStdin()) + // pipe.SetReadDeadline(time.Now().Add(100 * time.Millisecond)) + // mock := &FakeFileReader{ + // PassthroughPipe: pipe, + // } + // go func(stdin io.Reader) { + // _, err := io.Copy(context.GetConsole(), stdin) + // if err != nil { + // context.GetConsole().Logf("failed to copy stdin: %s", err) + // } + // }(pipe) + file := os.NewFile(context.GetStdin().Fd(), "pipe") pipe, _ := expect.NewPassthroughPipe(context.GetStdin()) - pipe.SetReadDeadline(time.Now().Add(100 * time.Millisecond)) + pipe.SetReadDeadline(time.Now().Add(500 * time.Millisecond)) mock := &FakeFileReader{ + file: file, PassthroughPipe: pipe, } - go func(stdin io.Reader) { - _, err := io.Copy(context.GetConsole(), stdin) - if err != nil { - context.GetConsole().Logf("failed to copy stdin: %s", err) - } - }(pipe) return survey.AskOne(&survey.Input{ Message: "What is your name?", }, &name, survey.WithStdio(mock, context.GetStdout(), context.GetStderr())) @@ -115,7 +121,6 @@ func (s *InitTestSuite) TestTerminalConfirm() { c.ExpectEOF() return nil }) - // panic("adass") s.Require().False(confirmed) } From b537d655b9dc5ab69c8c6f89c3e36f945b231408 Mon Sep 17 00:00:00 2001 From: Blake Watters Date: Wed, 22 Apr 2020 06:00:00 -0700 Subject: [PATCH 05/22] mo bettah --- command/init_test.go | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/command/init_test.go b/command/init_test.go index 9cebeee..40f578b 100644 --- a/command/init_test.go +++ b/command/init_test.go @@ -57,17 +57,6 @@ func (s *InitTestSuite) TestRunningInitHelp() { func (s *InitTestSuite) TestTerminalInteraction() { var name string test.RunTestInInteractiveTerminal(s.T(), func(context *test.InteractiveExecutionContext) error { - // pipe, _ := expect.NewPassthroughPipe(context.GetStdin()) - // pipe.SetReadDeadline(time.Now().Add(100 * time.Millisecond)) - // mock := &FakeFileReader{ - // PassthroughPipe: pipe, - // } - // go func(stdin io.Reader) { - // _, err := io.Copy(context.GetConsole(), stdin) - // if err != nil { - // context.GetConsole().Logf("failed to copy stdin: %s", err) - // } - // }(pipe) file := os.NewFile(context.GetStdin().Fd(), "pipe") pipe, _ := expect.NewPassthroughPipe(context.GetStdin()) pipe.SetReadDeadline(time.Now().Add(500 * time.Millisecond)) @@ -80,7 +69,6 @@ func (s *InitTestSuite) TestTerminalInteraction() { }, &name, survey.WithStdio(mock, context.GetStdout(), context.GetStderr())) }, func(_ *test.InteractiveExecutionContext, c *expect.Console) error { s.RequireNoErr2(c.ExpectString("What is your name?")) - // c.ExpectString("? What is your name?") c.SendLine("Blake Watters") c.ExpectEOF() return nil From 7b544892062404e9e2f2cf2877a09df556c09765 Mon Sep 17 00:00:00 2001 From: Blake Watters Date: Wed, 22 Apr 2020 06:59:09 -0700 Subject: [PATCH 06/22] Fix timeout problems with survey lib --- command/init_test.go | 36 ++++++++++++++++-------------------- test/interactive_terminal.go | 2 +- 2 files changed, 17 insertions(+), 21 deletions(-) diff --git a/command/init_test.go b/command/init_test.go index 40f578b..f510ef4 100644 --- a/command/init_test.go +++ b/command/init_test.go @@ -57,16 +57,10 @@ func (s *InitTestSuite) TestRunningInitHelp() { func (s *InitTestSuite) TestTerminalInteraction() { var name string test.RunTestInInteractiveTerminal(s.T(), func(context *test.InteractiveExecutionContext) error { - file := os.NewFile(context.GetStdin().Fd(), "pipe") - pipe, _ := expect.NewPassthroughPipe(context.GetStdin()) - pipe.SetReadDeadline(time.Now().Add(500 * time.Millisecond)) - mock := &FakeFileReader{ - file: file, - PassthroughPipe: pipe, - } + proxy := NewPassthroughPipeFile(context.GetStdin()) return survey.AskOne(&survey.Input{ Message: "What is your name?", - }, &name, survey.WithStdio(mock, context.GetStdout(), context.GetStderr())) + }, &name, survey.WithStdio(proxy, context.GetStdout(), context.GetStderr())) }, func(_ *test.InteractiveExecutionContext, c *expect.Console) error { s.RequireNoErr2(c.ExpectString("What is your name?")) c.SendLine("Blake Watters") @@ -80,31 +74,33 @@ func (s *InitTestSuite) RequireNoErr2(_ interface{}, err error) { s.Require().NoError(err) } -type FakeFileReader struct { +type PassthroughPipeFile struct { *expect.PassthroughPipe file *os.File } -// TODO: These can alias to temp files -func (s *FakeFileReader) Fd() uintptr { +func (s *PassthroughPipeFile) Fd() uintptr { return s.file.Fd() } +func NewPassthroughPipeFile(stdin *os.File) *PassthroughPipeFile { + file := os.NewFile(stdin.Fd(), "pipe") + pipe, _ := expect.NewPassthroughPipe(stdin) + pipe.SetReadDeadline(time.Now().Add(500 * time.Millisecond)) + return &PassthroughPipeFile{ + file: file, + PassthroughPipe: pipe, + } +} + func (s *InitTestSuite) TestTerminalConfirm() { var confirmed bool = true test.RunTestInInteractiveTerminal(s.T(), func(context *test.InteractiveExecutionContext) error { - file := os.NewFile(context.GetStdin().Fd(), "pipe") - pipe, _ := expect.NewPassthroughPipe(context.GetStdin()) - pipe.SetReadDeadline(time.Now().Add(500 * time.Millisecond)) - mock := &FakeFileReader{ - file: file, - PassthroughPipe: pipe, - } return survey.AskOne(&survey.Confirm{ Message: "Delete file?", - }, &confirmed, survey.WithStdio(mock, context.GetStdout(), context.GetStderr())) + }, &confirmed, survey.WithStdio(NewPassthroughPipeFile(context.GetStdin()), context.GetStdout(), context.GetStderr())) }, func(_ *test.InteractiveExecutionContext, c *expect.Console) error { - c.Expect(expect.RegexpPattern("Delete file?")) + s.RequireNoErr2(c.Expect(expect.RegexpPattern("Delete file?"))) c.SendLine("N") c.ExpectEOF() return nil diff --git a/test/interactive_terminal.go b/test/interactive_terminal.go index f67c90d..5b91fc5 100644 --- a/test/interactive_terminal.go +++ b/test/interactive_terminal.go @@ -114,7 +114,7 @@ func ExecuteInInteractiveTerminal( console, terminalState, err := vt10x.NewVT10XConsole( append([]expect.ConsoleOpt{ expect.WithStdout(outputBuffer), - expect.WithDefaultTimeout(300 * time.Millisecond), + expect.WithDefaultTimeout(100 * time.Millisecond), }, consoleOpts...)...) if err != nil { return nil, err From dda1a057c815ae07490dd202fb1da60a2734ba2a Mon Sep 17 00:00:00 2001 From: Blake Watters Date: Wed, 22 Apr 2020 07:17:08 -0700 Subject: [PATCH 07/22] Fix test blocking on stdin read --- command/init_test.go | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/command/init_test.go b/command/init_test.go index f510ef4..b550230 100644 --- a/command/init_test.go +++ b/command/init_test.go @@ -119,7 +119,7 @@ func (s *InitTestSuite) TestInitWithExistingConfig() { ice.PreExecutionFunc = func(context *test.InteractiveExecutionContext) error { // Attach the survey library to the console // This is necessary because of type safety fun with modeling around file readers - command.Stdio = terminal.Stdio{context.GetStdin(), context.GetStdout(), context.GetStderr()} + command.Stdio = terminal.Stdio{NewPassthroughPipeFile(context.GetStdin()), context.GetStdout(), context.GetStderr()} return nil } _, err := ice.Execute(test.Args("--config", configFile.Name(), "init"), func(_ *test.InteractiveExecutionContext, console *expect.Console) error { @@ -127,11 +127,11 @@ func (s *InitTestSuite) TestInitWithExistingConfig() { return err } str := fmt.Sprintf("? Existing config found. Overwrite %s?", configFile.Name()) - if _, err := console.ExpectString(str); err != nil { - return err - } - console.SendLine("N") - console.ExpectEOF() + _, err := console.ExpectString(str) + s.NoError(err) + _, err = console.SendLine("N") + s.NoError(err) + _, err = console.ExpectEOF() return nil }) s.Require().Error(err) From 6047d7aeba3c72ccccc9d4f6941e870d7d080414 Mon Sep 17 00:00:00 2001 From: Blake Watters Date: Wed, 22 Apr 2020 08:14:59 -0700 Subject: [PATCH 08/22] Save point --- command/init_test.go | 50 +++++++++++++++++++++++++++++++++++++++++++- go.mod | 1 + 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/command/init_test.go b/command/init_test.go index b550230..53af57b 100644 --- a/command/init_test.go +++ b/command/init_test.go @@ -16,6 +16,7 @@ package command_test import ( "fmt" + "io/ioutil" "os" "testing" "time" @@ -28,6 +29,7 @@ import ( "github.com/opsani/cli/test" "github.com/spf13/viper" "github.com/stretchr/testify/suite" + "gopkg.in/yaml.v2" ) type InitTestSuite struct { @@ -108,7 +110,7 @@ func (s *InitTestSuite) TestTerminalConfirm() { s.Require().False(confirmed) } -func (s *InitTestSuite) TestInitWithExistingConfig() { +func (s *InitTestSuite) TestInitWithExistingConfigDeclined() { configFile := test.TempConfigFileWithObj(map[string]string{ "app": "example.com/app", "token": "123456", @@ -137,3 +139,49 @@ func (s *InitTestSuite) TestInitWithExistingConfig() { s.Require().Error(err) s.Require().EqualError(err, terminal.InterruptErr.Error()) } + +func (s *InitTestSuite) TestInitWithExistingConfigAccepted() { + configFile := test.TempConfigFileWithObj(map[string]string{ + "app": "example.com/app", + "token": "123456", + }) + + rootCmd := command.NewRootCommand() + ice := test.NewInteractiveCommandExecutor(rootCmd, expect.WithDefaultTimeout(10.0*time.Second)) + ice.PreExecutionFunc = func(context *test.InteractiveExecutionContext) error { + // Attach the survey library to the console + // This is necessary because of type safety fun with modeling around file readers + command.Stdio = terminal.Stdio{NewPassthroughPipeFile(context.GetStdin()), context.GetStdout(), context.GetStderr()} + return nil + } + context, err := ice.Execute(test.Args("--config", configFile.Name(), "init"), func(_ *test.InteractiveExecutionContext, console *expect.Console) error { + if _, err := console.ExpectString(fmt.Sprintf("Using config from: %s", configFile.Name())); err != nil { + return err + } + str := fmt.Sprintf("? Existing config found. Overwrite %s?", configFile.Name()) + _, err := console.ExpectString(str) + s.Require().NoError(err) + _, err = console.SendLine("Y") + s.Require().NoError(err) + console.Expect(expect.RegexpPattern("Opsani app")) + _, err = console.SendLine("dev.opsani.com/amazing-app") + console.Expect(expect.RegexpPattern("API Token")) + _, err = console.SendLine("123456") + str = fmt.Sprintf("Write to %s?", configFile.Name()) + console.Expect(expect.RegexpPattern(str)) + console.ExpectEOF() + _, err = console.SendLine("Y") + s.Require().NoError(err) + console.Expect(expect.RegexpPattern("Opsani config initialized")) + console.ExpectEOF() + return nil + }) + s.Require().NoError(err, context.GetOutputBuffer()) + + // Check the config file + var config = &map[string]interface{}{} + body, err := ioutil.ReadFile(configFile.Name()) + err = yaml.Unmarshal(body, &config) + s.Require().EqualValues(&map[string]interface{}{"app": "example.com/app", "token": "123456"}, config) + +} diff --git a/go.mod b/go.mod index 9c37679..83e7e71 100644 --- a/go.mod +++ b/go.mod @@ -51,6 +51,7 @@ require ( google.golang.org/genproto v0.0.0-20200413115906-b5235f65be36 // indirect google.golang.org/grpc v1.28.1 // indirect gopkg.in/ini.v1 v1.55.0 // indirect + gopkg.in/yaml.v2 v2.2.8 gotest.tools v2.2.0+incompatible // indirect k8s.io/apimachinery v0.18.1 k8s.io/client-go v0.18.1 From 7e7de4ab5ec590e53f2521e8c2e6cfb2e90afcce Mon Sep 17 00:00:00 2001 From: Blake Watters Date: Wed, 22 Apr 2020 08:40:12 -0700 Subject: [PATCH 09/22] another savepoint --- command/init_test.go | 29 ++++-------------------- go.mod | 2 +- test/interactive_terminal.go | 43 ++++++++++++++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 26 deletions(-) diff --git a/command/init_test.go b/command/init_test.go index 53af57b..1d391bf 100644 --- a/command/init_test.go +++ b/command/init_test.go @@ -17,7 +17,6 @@ package command_test import ( "fmt" "io/ioutil" - "os" "testing" "time" @@ -59,10 +58,9 @@ func (s *InitTestSuite) TestRunningInitHelp() { func (s *InitTestSuite) TestTerminalInteraction() { var name string test.RunTestInInteractiveTerminal(s.T(), func(context *test.InteractiveExecutionContext) error { - proxy := NewPassthroughPipeFile(context.GetStdin()) return survey.AskOne(&survey.Input{ Message: "What is your name?", - }, &name, survey.WithStdio(proxy, context.GetStdout(), context.GetStderr())) + }, &name, survey.WithStdio(context.GetStdin(), context.GetStdout(), context.GetStderr())) }, func(_ *test.InteractiveExecutionContext, c *expect.Console) error { s.RequireNoErr2(c.ExpectString("What is your name?")) c.SendLine("Blake Watters") @@ -76,31 +74,12 @@ func (s *InitTestSuite) RequireNoErr2(_ interface{}, err error) { s.Require().NoError(err) } -type PassthroughPipeFile struct { - *expect.PassthroughPipe - file *os.File -} - -func (s *PassthroughPipeFile) Fd() uintptr { - return s.file.Fd() -} - -func NewPassthroughPipeFile(stdin *os.File) *PassthroughPipeFile { - file := os.NewFile(stdin.Fd(), "pipe") - pipe, _ := expect.NewPassthroughPipe(stdin) - pipe.SetReadDeadline(time.Now().Add(500 * time.Millisecond)) - return &PassthroughPipeFile{ - file: file, - PassthroughPipe: pipe, - } -} - func (s *InitTestSuite) TestTerminalConfirm() { var confirmed bool = true test.RunTestInInteractiveTerminal(s.T(), func(context *test.InteractiveExecutionContext) error { return survey.AskOne(&survey.Confirm{ Message: "Delete file?", - }, &confirmed, survey.WithStdio(NewPassthroughPipeFile(context.GetStdin()), context.GetStdout(), context.GetStderr())) + }, &confirmed, survey.WithStdio(context.GetStdin(), context.GetStdout(), context.GetStderr())) }, func(_ *test.InteractiveExecutionContext, c *expect.Console) error { s.RequireNoErr2(c.Expect(expect.RegexpPattern("Delete file?"))) c.SendLine("N") @@ -121,7 +100,7 @@ func (s *InitTestSuite) TestInitWithExistingConfigDeclined() { ice.PreExecutionFunc = func(context *test.InteractiveExecutionContext) error { // Attach the survey library to the console // This is necessary because of type safety fun with modeling around file readers - command.Stdio = terminal.Stdio{NewPassthroughPipeFile(context.GetStdin()), context.GetStdout(), context.GetStderr()} + command.Stdio = terminal.Stdio{context.GetStdin(), context.GetStdout(), context.GetStderr()} return nil } _, err := ice.Execute(test.Args("--config", configFile.Name(), "init"), func(_ *test.InteractiveExecutionContext, console *expect.Console) error { @@ -151,7 +130,7 @@ func (s *InitTestSuite) TestInitWithExistingConfigAccepted() { ice.PreExecutionFunc = func(context *test.InteractiveExecutionContext) error { // Attach the survey library to the console // This is necessary because of type safety fun with modeling around file readers - command.Stdio = terminal.Stdio{NewPassthroughPipeFile(context.GetStdin()), context.GetStdout(), context.GetStderr()} + command.Stdio = terminal.Stdio{test.NewPassthroughPipeFile(context.GetStdin()), context.GetStdout(), context.GetStderr()} return nil } context, err := ice.Execute(test.Args("--config", configFile.Name(), "init"), func(_ *test.InteractiveExecutionContext, console *expect.Console) error { diff --git a/go.mod b/go.mod index 83e7e71..da8b12c 100644 --- a/go.mod +++ b/go.mod @@ -25,7 +25,7 @@ require ( github.com/hokaccha/go-prettyjson v0.0.0-20190818114111-108c894c2c0e github.com/imdario/mergo v0.3.9 // indirect github.com/konsorten/go-windows-terminal-sequences v1.0.2 // indirect - github.com/kr/pty v1.1.8 // indirect + github.com/kr/pty v1.1.8 github.com/mattn/go-isatty v0.0.12 github.com/mgutz/ansi v0.0.0-20170206155736-9520e82c474b github.com/mitchellh/go-homedir v1.1.0 diff --git a/test/interactive_terminal.go b/test/interactive_terminal.go index 5b91fc5..9d988f5 100644 --- a/test/interactive_terminal.go +++ b/test/interactive_terminal.go @@ -25,6 +25,7 @@ import ( expect "github.com/Netflix/go-expect" "github.com/hinshun/vt10x" "github.com/spf13/cobra" + "github.com/kr/pty" ) // RunTestInInteractiveTerminal runs a test within an interactive terminal environment @@ -105,6 +106,48 @@ func (ice *InteractiveCommandExecutor) SetTimeout(timeout time.Duration) { ice.consoleOpts = append(ice.consoleOpts, expect.WithDefaultTimeout(timeout)) } +type PassthroughPipeFile struct { + *expect.PassthroughPipe + file *os.File +} + +func (s *PassthroughPipeFile) Fd() uintptr { + return s.file.Fd() +} + +func NewPassthroughPipeFile(stdin *os.File) *PassthroughPipeFile { + file := os.NewFile(stdin.Fd(), "pipe") + pipe, _ := expect.NewPassthroughPipe(stdin) + pipe.SetReadDeadline(time.Now().Add(500 * time.Millisecond)) + return &PassthroughPipeFile{ + file: file, + PassthroughPipe: pipe, + } +} + +func buildVT10X(consoleOpts ...expect.ConsoleOpt) (*expect.Console, *vt10x.State, error) { + ptm, pts, err := pty.Open() + if err != nil { + return nil, nil, err + } + + var state vt10x.State + term, err := vt10x.Create(&state, pts) + if err != nil { + return nil, nil, err + } + + c, err := expect.NewConsole(append(consoleOpts, + expect.WithStdin(NewPassthroughPipeFile(ptm)), + expect.WithStdout(term), + expect.WithCloser(pts, ptm, term))...) + if err != nil { + return nil, nil, err + } + + return c, &state, nil +} + // ExecuteInInteractiveTerminal runs a pair of functions connected in an interactive virtual terminal environment func ExecuteInInteractiveTerminal( processFunc InteractiveProcessFunc, // Represents the process that the user is interacting with via the terminal From 81a88f2872ac6654040e90358fce9d17f3f43468 Mon Sep 17 00:00:00 2001 From: Blake Watters Date: Wed, 22 Apr 2020 18:56:44 -0700 Subject: [PATCH 10/22] Fix hanging test problem --- command/init_test.go | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/command/init_test.go b/command/init_test.go index 1d391bf..576af5c 100644 --- a/command/init_test.go +++ b/command/init_test.go @@ -18,7 +18,6 @@ import ( "fmt" "io/ioutil" "testing" - "time" "github.com/AlecAivazis/survey/v2" "github.com/AlecAivazis/survey/v2/core" @@ -96,11 +95,11 @@ func (s *InitTestSuite) TestInitWithExistingConfigDeclined() { }) rootCmd := command.NewRootCommand() - ice := test.NewInteractiveCommandExecutor(rootCmd, expect.WithDefaultTimeout(1.0*time.Second)) + ice := test.NewInteractiveCommandExecutor(rootCmd) ice.PreExecutionFunc = func(context *test.InteractiveExecutionContext) error { // Attach the survey library to the console // This is necessary because of type safety fun with modeling around file readers - command.Stdio = terminal.Stdio{context.GetStdin(), context.GetStdout(), context.GetStderr()} + command.Stdio = terminal.Stdio{In: test.NewPassthroughPipeFile(context.GetStdin()), Out: context.GetStdout(), Err: context.GetStderr()} return nil } _, err := ice.Execute(test.Args("--config", configFile.Name(), "init"), func(_ *test.InteractiveExecutionContext, console *expect.Console) error { @@ -109,9 +108,9 @@ func (s *InitTestSuite) TestInitWithExistingConfigDeclined() { } str := fmt.Sprintf("? Existing config found. Overwrite %s?", configFile.Name()) _, err := console.ExpectString(str) - s.NoError(err) + s.Require().NoErrorf(err, "Failed reading %q: %v", str, err) _, err = console.SendLine("N") - s.NoError(err) + s.Require().NoError(err) _, err = console.ExpectEOF() return nil }) @@ -126,11 +125,11 @@ func (s *InitTestSuite) TestInitWithExistingConfigAccepted() { }) rootCmd := command.NewRootCommand() - ice := test.NewInteractiveCommandExecutor(rootCmd, expect.WithDefaultTimeout(10.0*time.Second)) + ice := test.NewInteractiveCommandExecutor(rootCmd) ice.PreExecutionFunc = func(context *test.InteractiveExecutionContext) error { // Attach the survey library to the console // This is necessary because of type safety fun with modeling around file readers - command.Stdio = terminal.Stdio{test.NewPassthroughPipeFile(context.GetStdin()), context.GetStdout(), context.GetStderr()} + command.Stdio = terminal.Stdio{In: test.NewPassthroughPipeFile(context.GetStdin()), Out: context.GetStdout(), Err: context.GetStderr()} return nil } context, err := ice.Execute(test.Args("--config", configFile.Name(), "init"), func(_ *test.InteractiveExecutionContext, console *expect.Console) error { @@ -139,28 +138,28 @@ func (s *InitTestSuite) TestInitWithExistingConfigAccepted() { } str := fmt.Sprintf("? Existing config found. Overwrite %s?", configFile.Name()) _, err := console.ExpectString(str) - s.Require().NoError(err) + s.Require().NoErrorf(err, "Failed reading %q: %v", str, err) _, err = console.SendLine("Y") s.Require().NoError(err) - console.Expect(expect.RegexpPattern("Opsani app")) + _, err = console.Expect(expect.RegexpPattern("Opsani app")) _, err = console.SendLine("dev.opsani.com/amazing-app") console.Expect(expect.RegexpPattern("API Token")) _, err = console.SendLine("123456") str = fmt.Sprintf("Write to %s?", configFile.Name()) console.Expect(expect.RegexpPattern(str)) - console.ExpectEOF() + _, err = console.SendLine("Y") s.Require().NoError(err) console.Expect(expect.RegexpPattern("Opsani config initialized")) console.ExpectEOF() return nil }) - s.Require().NoError(err, context.GetOutputBuffer()) + s.Require().NoError(err, context.GetOutputBuffer().String()) // Check the config file - var config = &map[string]interface{}{} + var config = map[string]interface{}{} body, err := ioutil.ReadFile(configFile.Name()) - err = yaml.Unmarshal(body, &config) - s.Require().EqualValues(&map[string]interface{}{"app": "example.com/app", "token": "123456"}, config) - + yaml.Unmarshal(body, &config) + s.Require().Equal("dev.opsani.com/amazing-app", config["app"]) + s.Require().Equal("123456", config["token"]) } From 3a4a60e499fa4fa5e1b30f9c28b70ddf61f81d12 Mon Sep 17 00:00:00 2001 From: Blake Watters Date: Wed, 22 Apr 2020 21:53:14 -0700 Subject: [PATCH 11/22] Snapshot functional fast path --- command/init_test.go | 15 ++-- test/interactive_terminal.go | 150 ++++++++++++++++++++++------------- 2 files changed, 102 insertions(+), 63 deletions(-) diff --git a/command/init_test.go b/command/init_test.go index 576af5c..032c844 100644 --- a/command/init_test.go +++ b/command/init_test.go @@ -57,10 +57,12 @@ func (s *InitTestSuite) TestRunningInitHelp() { func (s *InitTestSuite) TestTerminalInteraction() { var name string test.RunTestInInteractiveTerminal(s.T(), func(context *test.InteractiveExecutionContext) error { + fmt.Printf("%v\n", context) return survey.AskOne(&survey.Input{ Message: "What is your name?", - }, &name, survey.WithStdio(context.GetStdin(), context.GetStdout(), context.GetStderr())) + }, &name, survey.WithStdio(context.Tty(), context.Tty(), context.Tty())) }, func(_ *test.InteractiveExecutionContext, c *expect.Console) error { + // panic("sdasd") s.RequireNoErr2(c.ExpectString("What is your name?")) c.SendLine("Blake Watters") c.ExpectEOF() @@ -78,7 +80,7 @@ func (s *InitTestSuite) TestTerminalConfirm() { test.RunTestInInteractiveTerminal(s.T(), func(context *test.InteractiveExecutionContext) error { return survey.AskOne(&survey.Confirm{ Message: "Delete file?", - }, &confirmed, survey.WithStdio(context.GetStdin(), context.GetStdout(), context.GetStderr())) + }, &confirmed, survey.WithStdio(context.Tty(), context.Tty(), context.Tty())) }, func(_ *test.InteractiveExecutionContext, c *expect.Console) error { s.RequireNoErr2(c.Expect(expect.RegexpPattern("Delete file?"))) c.SendLine("N") @@ -99,14 +101,15 @@ func (s *InitTestSuite) TestInitWithExistingConfigDeclined() { ice.PreExecutionFunc = func(context *test.InteractiveExecutionContext) error { // Attach the survey library to the console // This is necessary because of type safety fun with modeling around file readers - command.Stdio = terminal.Stdio{In: test.NewPassthroughPipeFile(context.GetStdin()), Out: context.GetStdout(), Err: context.GetStderr()} + command.Stdio = terminal.Stdio{In: context.PassthroughTty(), Out: context.PassthroughTty(), Err: context.PassthroughTty()} + // command.Stdio = terminal.Stdio{In: test.NewPassthroughPipeFile(context.GetStdin()), Out: context.GetStdout(), Err: context.GetStderr()} return nil } _, err := ice.Execute(test.Args("--config", configFile.Name(), "init"), func(_ *test.InteractiveExecutionContext, console *expect.Console) error { if _, err := console.ExpectString(fmt.Sprintf("Using config from: %s", configFile.Name())); err != nil { return err } - str := fmt.Sprintf("? Existing config found. Overwrite %s?", configFile.Name()) + str := fmt.Sprintf("? Ex isting config found. Overwrite %s?", configFile.Name()) _, err := console.ExpectString(str) s.Require().NoErrorf(err, "Failed reading %q: %v", str, err) _, err = console.SendLine("N") @@ -129,7 +132,7 @@ func (s *InitTestSuite) TestInitWithExistingConfigAccepted() { ice.PreExecutionFunc = func(context *test.InteractiveExecutionContext) error { // Attach the survey library to the console // This is necessary because of type safety fun with modeling around file readers - command.Stdio = terminal.Stdio{In: test.NewPassthroughPipeFile(context.GetStdin()), Out: context.GetStdout(), Err: context.GetStderr()} + command.Stdio = terminal.Stdio{In: context.PassthroughTty(), Out: context.PassthroughTty(), Err: context.PassthroughTty()} return nil } context, err := ice.Execute(test.Args("--config", configFile.Name(), "init"), func(_ *test.InteractiveExecutionContext, console *expect.Console) error { @@ -154,7 +157,7 @@ func (s *InitTestSuite) TestInitWithExistingConfigAccepted() { console.ExpectEOF() return nil }) - s.Require().NoError(err, context.GetOutputBuffer().String()) + s.Require().NoError(err, context.OutputBuffer().String()) // Check the config file var config = map[string]interface{}{} diff --git a/test/interactive_terminal.go b/test/interactive_terminal.go index 9d988f5..372af0b 100644 --- a/test/interactive_terminal.go +++ b/test/interactive_terminal.go @@ -17,6 +17,7 @@ package test import ( "bytes" "fmt" + "io" "os" "strings" "testing" @@ -25,9 +26,45 @@ import ( expect "github.com/Netflix/go-expect" "github.com/hinshun/vt10x" "github.com/spf13/cobra" - "github.com/kr/pty" ) +// TODO: Rename to PassthroughTty +// PassthroughPipeFile wraps a file with a PassthroughPipe to add read deadline support +type PassthroughPipeFile struct { + *expect.PassthroughPipe + file *os.File +} + +// Write proxies the write operation to the underlying file +func (s *PassthroughPipeFile) Write(p []byte) (n int, err error) { + return s.file.Write(p) +} + +// Fd proxies the file descriptor of the underyling file +// This is necessary because the survey library treats the stdio descriptors as +// concrete os.File instances instead of reader/writer interfaces +func (s *PassthroughPipeFile) Fd() uintptr { + return s.file.Fd() +} + +// NewPassthroughPipeFile returns a new PassthroughPipeFile that wraps the input file to enable read deadline support +func NewPassthroughPipeFile(inFile *os.File) (*PassthroughPipeFile, error) { + file := os.NewFile(inFile.Fd(), "pipe") // TODO: Do I need this?? + if file == nil { + panic("l;kg;lfdkg;fldkgd") + } + fmt.Printf("The inputFile is %v and the new file is %v", inFile, file) + pipe, err := expect.NewPassthroughPipe(inFile) + if err != nil { + panic(err) + return nil, err + } + return &PassthroughPipeFile{ + file: file, + PassthroughPipe: pipe, + }, nil +} + // RunTestInInteractiveTerminal runs a test within an interactive terminal environment // Executin requires a standard test instance, a pair of functions that execute the code // under test and the test code, and any desired options for configuring the virtual terminal environment @@ -36,41 +73,79 @@ func RunTestInInteractiveTerminal(t *testing.T, testFunc InteractiveUserFunc, consoleOpts ...expect.ConsoleOpt) (*InteractiveExecutionContext, error) { context, err := ExecuteInInteractiveTerminal(codeUnderTestFunc, testFunc, consoleOpts...) - t.Logf("Raw output: %q", context.GetOutputBuffer().String()) - t.Logf("\n\nterminal state: %s", expect.StripTrailingEmptyLines(context.GetTerminalState().String())) + t.Logf("Raw output: %q", context.OutputBuffer().String()) + t.Logf("\n\nterminal state: %s", expect.StripTrailingEmptyLines(context.TerminalState().String())) return context, err } +// InteractiveExecutionContext describes the state of an interactive terminal execution type InteractiveExecutionContext struct { - outputBuffer *bytes.Buffer - terminalState *vt10x.State - console *expect.Console + outputBuffer *bytes.Buffer + terminalState *vt10x.State + console *expect.Console + passthroughTty *PassthroughPipeFile +} + +// Tty returns the io.Reader to be used as stdin during execution +// func (ice *InteractiveExecutionContext) Tty() *PassthroughPipeFile { +// return ice.passthroughTty +// } +func (ice *InteractiveExecutionContext) Tty() *os.File { + // return ice.passthroughTty + return ice.console.Tty() } -func (ice *InteractiveExecutionContext) GetStdin() *os.File { +// Stdin returns the io.Reader to be used as stdin during execution +func (ice *InteractiveExecutionContext) Stdin() io.Reader { + // if ice.passthroughTty == nil { + + // } + // return ice.passthroughTty + // // return ice.passthroughTty return ice.console.Tty() } -func (ice *InteractiveExecutionContext) GetStdout() *os.File { +// Stdout returns the io.Writer to be used as stdout during execution +func (ice *InteractiveExecutionContext) Stdout() io.Writer { + // return ice.passthroughTty return ice.console.Tty() } -func (ice *InteractiveExecutionContext) GetStderr() *os.File { +// Stderr returns the io.Writer to be used as stdout during execution +func (ice *InteractiveExecutionContext) Stderr() io.Writer { + // return ice.passthroughTty return ice.console.Tty() } -func (ice *InteractiveExecutionContext) GetOutputBuffer() *bytes.Buffer { +// OutputBuffer returns the output buffer read from the tty +func (ice *InteractiveExecutionContext) OutputBuffer() *bytes.Buffer { return ice.outputBuffer } -func (ice *InteractiveExecutionContext) GetTerminalState() *vt10x.State { +// TerminalState returns the state if the terminal +func (ice *InteractiveExecutionContext) TerminalState() *vt10x.State { return ice.terminalState } -func (ice *InteractiveExecutionContext) GetConsole() *expect.Console { +// Console returns the console for interacting with the terminal +func (ice *InteractiveExecutionContext) Console() *expect.Console { return ice.console } +func (ice *InteractiveExecutionContext) PassthroughTty() *PassthroughPipeFile { + // Wrap the Tty into a PassthroughPipeFile to enable deadline support (NewPassthroughPipeFileReader??) + if ice.passthroughTty == nil { + passthroughTty, err := NewPassthroughPipeFile(ice.Tty()) + // TODO: SETTING THE DEADLINE IS CRITICAL + passthroughTty.SetReadDeadline(time.Now().Add(500 * time.Millisecond)) + if err != nil { + panic(err) + } + ice.passthroughTty = passthroughTty + } + return ice.passthroughTty +} + // Args is a convenience function that converts a variadic list of strings into an array func Args(args ...string) []string { return args @@ -106,48 +181,6 @@ func (ice *InteractiveCommandExecutor) SetTimeout(timeout time.Duration) { ice.consoleOpts = append(ice.consoleOpts, expect.WithDefaultTimeout(timeout)) } -type PassthroughPipeFile struct { - *expect.PassthroughPipe - file *os.File -} - -func (s *PassthroughPipeFile) Fd() uintptr { - return s.file.Fd() -} - -func NewPassthroughPipeFile(stdin *os.File) *PassthroughPipeFile { - file := os.NewFile(stdin.Fd(), "pipe") - pipe, _ := expect.NewPassthroughPipe(stdin) - pipe.SetReadDeadline(time.Now().Add(500 * time.Millisecond)) - return &PassthroughPipeFile{ - file: file, - PassthroughPipe: pipe, - } -} - -func buildVT10X(consoleOpts ...expect.ConsoleOpt) (*expect.Console, *vt10x.State, error) { - ptm, pts, err := pty.Open() - if err != nil { - return nil, nil, err - } - - var state vt10x.State - term, err := vt10x.Create(&state, pts) - if err != nil { - return nil, nil, err - } - - c, err := expect.NewConsole(append(consoleOpts, - expect.WithStdin(NewPassthroughPipeFile(ptm)), - expect.WithStdout(term), - expect.WithCloser(pts, ptm, term))...) - if err != nil { - return nil, nil, err - } - - return c, &state, nil -} - // ExecuteInInteractiveTerminal runs a pair of functions connected in an interactive virtual terminal environment func ExecuteInInteractiveTerminal( processFunc InteractiveProcessFunc, // Represents the process that the user is interacting with via the terminal @@ -169,6 +202,7 @@ func ExecuteInInteractiveTerminal( outputBuffer: outputBuffer, console: console, terminalState: terminalState, + // passthroughTty: passthroughTty, } // Execute our function within a channel and wait for exit @@ -195,11 +229,13 @@ func ExecuteInInteractiveTerminal( func (ice *InteractiveCommandExecutor) Execute(args []string, interactionFunc InteractiveUserFunc) (*InteractiveExecutionContext, error) { // Wrap our execution func with setup for Command execution commandExecutionFunc := func(context *InteractiveExecutionContext) error { - ice.command.SetIn(context.GetStdin()) - ice.command.SetOut(context.GetStdout()) - ice.command.SetErr(context.GetStderr()) + ice.command.SetIn(context.Stdin()) + ice.command.SetOut(context.Stdout()) + ice.command.SetErr(context.Stderr()) ice.command.SetArgs(args) + // TODO: Add the hook for survey + if ice.PreExecutionFunc != nil { ice.PreExecutionFunc(context) } From de9cd5bcc4d43b1b0111f7052bb07db72e2f879e Mon Sep 17 00:00:00 2001 From: Blake Watters Date: Wed, 22 Apr 2020 23:51:06 -0700 Subject: [PATCH 12/22] Cleanup --- command/init_test.go | 3 +-- test/interactive_terminal.go | 6 ++---- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/command/init_test.go b/command/init_test.go index 032c844..11715bb 100644 --- a/command/init_test.go +++ b/command/init_test.go @@ -62,7 +62,6 @@ func (s *InitTestSuite) TestTerminalInteraction() { Message: "What is your name?", }, &name, survey.WithStdio(context.Tty(), context.Tty(), context.Tty())) }, func(_ *test.InteractiveExecutionContext, c *expect.Console) error { - // panic("sdasd") s.RequireNoErr2(c.ExpectString("What is your name?")) c.SendLine("Blake Watters") c.ExpectEOF() @@ -109,7 +108,7 @@ func (s *InitTestSuite) TestInitWithExistingConfigDeclined() { if _, err := console.ExpectString(fmt.Sprintf("Using config from: %s", configFile.Name())); err != nil { return err } - str := fmt.Sprintf("? Ex isting config found. Overwrite %s?", configFile.Name()) + str := fmt.Sprintf("? Existing config found. Overwrite %s?", configFile.Name()) _, err := console.ExpectString(str) s.Require().NoErrorf(err, "Failed reading %q: %v", str, err) _, err = console.SendLine("N") diff --git a/test/interactive_terminal.go b/test/interactive_terminal.go index 372af0b..7ac13eb 100644 --- a/test/interactive_terminal.go +++ b/test/interactive_terminal.go @@ -49,14 +49,12 @@ func (s *PassthroughPipeFile) Fd() uintptr { // NewPassthroughPipeFile returns a new PassthroughPipeFile that wraps the input file to enable read deadline support func NewPassthroughPipeFile(inFile *os.File) (*PassthroughPipeFile, error) { - file := os.NewFile(inFile.Fd(), "pipe") // TODO: Do I need this?? + file := os.NewFile(inFile.Fd(), "pipe") if file == nil { - panic("l;kg;lfdkg;fldkgd") + fmt.Errorf("os.NewFile failed: is your file descriptor valid?") } - fmt.Printf("The inputFile is %v and the new file is %v", inFile, file) pipe, err := expect.NewPassthroughPipe(inFile) if err != nil { - panic(err) return nil, err } return &PassthroughPipeFile{ From 6dced342d365d838a50928437b2ad463eb281709 Mon Sep 17 00:00:00 2001 From: Blake Watters Date: Wed, 22 Apr 2020 23:53:15 -0700 Subject: [PATCH 13/22] Factor stdio state up --- command/init_test.go | 13 ------------- test/interactive_terminal.go | 4 +++- 2 files changed, 3 insertions(+), 14 deletions(-) diff --git a/command/init_test.go b/command/init_test.go index 11715bb..edba142 100644 --- a/command/init_test.go +++ b/command/init_test.go @@ -97,13 +97,6 @@ func (s *InitTestSuite) TestInitWithExistingConfigDeclined() { rootCmd := command.NewRootCommand() ice := test.NewInteractiveCommandExecutor(rootCmd) - ice.PreExecutionFunc = func(context *test.InteractiveExecutionContext) error { - // Attach the survey library to the console - // This is necessary because of type safety fun with modeling around file readers - command.Stdio = terminal.Stdio{In: context.PassthroughTty(), Out: context.PassthroughTty(), Err: context.PassthroughTty()} - // command.Stdio = terminal.Stdio{In: test.NewPassthroughPipeFile(context.GetStdin()), Out: context.GetStdout(), Err: context.GetStderr()} - return nil - } _, err := ice.Execute(test.Args("--config", configFile.Name(), "init"), func(_ *test.InteractiveExecutionContext, console *expect.Console) error { if _, err := console.ExpectString(fmt.Sprintf("Using config from: %s", configFile.Name())); err != nil { return err @@ -128,12 +121,6 @@ func (s *InitTestSuite) TestInitWithExistingConfigAccepted() { rootCmd := command.NewRootCommand() ice := test.NewInteractiveCommandExecutor(rootCmd) - ice.PreExecutionFunc = func(context *test.InteractiveExecutionContext) error { - // Attach the survey library to the console - // This is necessary because of type safety fun with modeling around file readers - command.Stdio = terminal.Stdio{In: context.PassthroughTty(), Out: context.PassthroughTty(), Err: context.PassthroughTty()} - return nil - } context, err := ice.Execute(test.Args("--config", configFile.Name(), "init"), func(_ *test.InteractiveExecutionContext, console *expect.Console) error { if _, err := console.ExpectString(fmt.Sprintf("Using config from: %s", configFile.Name())); err != nil { return err diff --git a/test/interactive_terminal.go b/test/interactive_terminal.go index 7ac13eb..d6757fa 100644 --- a/test/interactive_terminal.go +++ b/test/interactive_terminal.go @@ -23,8 +23,10 @@ import ( "testing" "time" + "github.com/AlecAivazis/survey/v2/terminal" expect "github.com/Netflix/go-expect" "github.com/hinshun/vt10x" + "github.com/opsani/cli/command" "github.com/spf13/cobra" ) @@ -232,7 +234,7 @@ func (ice *InteractiveCommandExecutor) Execute(args []string, interactionFunc In ice.command.SetErr(context.Stderr()) ice.command.SetArgs(args) - // TODO: Add the hook for survey + command.Stdio = terminal.Stdio{In: context.PassthroughTty(), Out: context.PassthroughTty(), Err: context.PassthroughTty()} if ice.PreExecutionFunc != nil { ice.PreExecutionFunc(context) From a1bc31728d954a53096c67abe27fe208b4b97c45 Mon Sep 17 00:00:00 2001 From: Blake Watters Date: Wed, 22 Apr 2020 23:58:48 -0700 Subject: [PATCH 14/22] Only use a package global --- command/command.go | 28 ++++++++++------------------ test/interactive_terminal.go | 2 +- 2 files changed, 11 insertions(+), 19 deletions(-) diff --git a/command/command.go b/command/command.go index 25308f7..79ce69b 100644 --- a/command/command.go +++ b/command/command.go @@ -29,7 +29,6 @@ import ( // Command is a wrapper around cobra.Command that adds Opsani functionality type Command struct { *cobra.Command - stdio terminal.Stdio // Shadow all Cobra functions with Opsani equivalents PersistentPreRun func(cmd *Command, args []string) @@ -56,24 +55,15 @@ type Command struct { // Survey method wrappers // NOTE: These are necessary because of how the Survey library models in, out, and err -func (cmd *Command) SetStdio(stdio terminal.Stdio) { - Stdio = stdio - cmd.stdio = stdio +var globalStdio terminal.Stdio - // When stdio is set, cascade to Cobra - cmd.SetIn(stdio.In) - cmd.SetOut(stdio.Out) - cmd.SetErr(stdio.Err) +func SetStdio(stdio terminal.Stdio) { + globalStdio = stdio } -// TODO: Temporary global because of type issues -var Stdio terminal.Stdio - -func (cmd *Command) GetStdio() terminal.Stdio { - if Stdio != (terminal.Stdio{}) { - return Stdio - } else if cmd.stdio != (terminal.Stdio{}) { - return cmd.stdio +func (cmd *Command) Stdio() terminal.Stdio { + if globalStdio != (terminal.Stdio{}) { + return globalStdio } else { return terminal.Stdio{ In: os.Stdin, @@ -83,13 +73,15 @@ func (cmd *Command) GetStdio() terminal.Stdio { } } +// Ask is a wrapper for survey.AskOne that executes with the command's stdio func (cmd *Command) Ask(qs []*survey.Question, response interface{}, opts ...survey.AskOpt) error { - stdio := cmd.GetStdio() + stdio := cmd.Stdio() return survey.Ask(qs, response, append(opts, survey.WithStdio(stdio.In, stdio.Out, stdio.Err))...) } +// AskOne is a wrapper for survey.AskOne that executes with the command's stdio func (cmd *Command) AskOne(p survey.Prompt, response interface{}, opts ...survey.AskOpt) error { - stdio := cmd.GetStdio() + stdio := cmd.Stdio() return survey.AskOne(p, response, append(opts, survey.WithStdio(stdio.In, stdio.Out, stdio.Err))...) } diff --git a/test/interactive_terminal.go b/test/interactive_terminal.go index d6757fa..6690d9f 100644 --- a/test/interactive_terminal.go +++ b/test/interactive_terminal.go @@ -234,7 +234,7 @@ func (ice *InteractiveCommandExecutor) Execute(args []string, interactionFunc In ice.command.SetErr(context.Stderr()) ice.command.SetArgs(args) - command.Stdio = terminal.Stdio{In: context.PassthroughTty(), Out: context.PassthroughTty(), Err: context.PassthroughTty()} + command.SetStdio(terminal.Stdio{In: context.PassthroughTty(), Out: context.PassthroughTty(), Err: context.PassthroughTty()}) if ice.PreExecutionFunc != nil { ice.PreExecutionFunc(context) From ba008b316abbb757fcc673d49d122f830cdb4dbc Mon Sep 17 00:00:00 2001 From: Blake Watters Date: Thu, 23 Apr 2020 00:42:47 -0700 Subject: [PATCH 15/22] Finalize test helpers --- command/command.go | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/command/command.go b/command/command.go index 79ce69b..057d043 100644 --- a/command/command.go +++ b/command/command.go @@ -53,15 +53,18 @@ type Command struct { } // Survey method wrappers -// NOTE: These are necessary because of how the Survey library models in, out, and err - +// Survey needs access to a file descriptor for configuring the terminal but Cobra wants to model +// stdio as streams. var globalStdio terminal.Stdio +// SetStdio is global package helper for testing where access to a file +// descriptor for the TTY is required func SetStdio(stdio terminal.Stdio) { globalStdio = stdio } -func (cmd *Command) Stdio() terminal.Stdio { +// stdio is a test helper for returning terminal file descriptors usable by Survey +func (cmd *Command) stdio() terminal.Stdio { if globalStdio != (terminal.Stdio{}) { return globalStdio } else { @@ -75,13 +78,13 @@ func (cmd *Command) Stdio() terminal.Stdio { // Ask is a wrapper for survey.AskOne that executes with the command's stdio func (cmd *Command) Ask(qs []*survey.Question, response interface{}, opts ...survey.AskOpt) error { - stdio := cmd.Stdio() + stdio := cmd.stdio() return survey.Ask(qs, response, append(opts, survey.WithStdio(stdio.In, stdio.Out, stdio.Err))...) } // AskOne is a wrapper for survey.AskOne that executes with the command's stdio func (cmd *Command) AskOne(p survey.Prompt, response interface{}, opts ...survey.AskOpt) error { - stdio := cmd.Stdio() + stdio := cmd.stdio() return survey.AskOne(p, response, append(opts, survey.WithStdio(stdio.In, stdio.Out, stdio.Err))...) } From b269cc98b04a639c2e6fd5255a00cc5b5986b825 Mon Sep 17 00:00:00 2001 From: Blake Watters Date: Thu, 23 Apr 2020 00:52:07 -0700 Subject: [PATCH 16/22] Cleanup commented out code --- test/interactive_terminal.go | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/test/interactive_terminal.go b/test/interactive_terminal.go index 6690d9f..7228099 100644 --- a/test/interactive_terminal.go +++ b/test/interactive_terminal.go @@ -86,35 +86,25 @@ type InteractiveExecutionContext struct { passthroughTty *PassthroughPipeFile } -// Tty returns the io.Reader to be used as stdin during execution -// func (ice *InteractiveExecutionContext) Tty() *PassthroughPipeFile { -// return ice.passthroughTty -// } +// Tty returns the Tty of the underlying expect.Console instance +// You probably want to interact with the PassthroughTty which supports deadline based timeouts func (ice *InteractiveExecutionContext) Tty() *os.File { - // return ice.passthroughTty return ice.console.Tty() } // Stdin returns the io.Reader to be used as stdin during execution func (ice *InteractiveExecutionContext) Stdin() io.Reader { - // if ice.passthroughTty == nil { - - // } - // return ice.passthroughTty - // // return ice.passthroughTty - return ice.console.Tty() + return ice.PassthroughTty() } // Stdout returns the io.Writer to be used as stdout during execution func (ice *InteractiveExecutionContext) Stdout() io.Writer { - // return ice.passthroughTty - return ice.console.Tty() + return ice.PassthroughTty() } // Stderr returns the io.Writer to be used as stdout during execution func (ice *InteractiveExecutionContext) Stderr() io.Writer { - // return ice.passthroughTty - return ice.console.Tty() + return ice.PassthroughTty() } // OutputBuffer returns the output buffer read from the tty @@ -132,6 +122,8 @@ func (ice *InteractiveExecutionContext) Console() *expect.Console { return ice.console } +// PassthroughTty returns a wrapper for the Tty that supports deadline based timeouts +// The Std* family of methods are all aliases for the passthrough tty func (ice *InteractiveExecutionContext) PassthroughTty() *PassthroughPipeFile { // Wrap the Tty into a PassthroughPipeFile to enable deadline support (NewPassthroughPipeFileReader??) if ice.passthroughTty == nil { From 842fc519a918b86721cb973bb2d80a8941cf272a Mon Sep 17 00:00:00 2001 From: Blake Watters Date: Thu, 23 Apr 2020 01:07:05 -0700 Subject: [PATCH 17/22] Scrub dangling comments --- test/interactive_terminal.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/interactive_terminal.go b/test/interactive_terminal.go index 7228099..48d99df 100644 --- a/test/interactive_terminal.go +++ b/test/interactive_terminal.go @@ -30,7 +30,6 @@ import ( "github.com/spf13/cobra" ) -// TODO: Rename to PassthroughTty // PassthroughPipeFile wraps a file with a PassthroughPipe to add read deadline support type PassthroughPipeFile struct { *expect.PassthroughPipe @@ -194,7 +193,6 @@ func ExecuteInInteractiveTerminal( outputBuffer: outputBuffer, console: console, terminalState: terminalState, - // passthroughTty: passthroughTty, } // Execute our function within a channel and wait for exit From f06fea196c203ef040926e2485b8e7d542762fdd Mon Sep 17 00:00:00 2001 From: Blake Watters Date: Thu, 23 Apr 2020 01:29:52 -0700 Subject: [PATCH 18/22] Add closer proxy to speed up tests even further --- test/interactive_terminal.go | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/test/interactive_terminal.go b/test/interactive_terminal.go index 48d99df..4946323 100644 --- a/test/interactive_terminal.go +++ b/test/interactive_terminal.go @@ -52,7 +52,7 @@ func (s *PassthroughPipeFile) Fd() uintptr { func NewPassthroughPipeFile(inFile *os.File) (*PassthroughPipeFile, error) { file := os.NewFile(inFile.Fd(), "pipe") if file == nil { - fmt.Errorf("os.NewFile failed: is your file descriptor valid?") + return nil, fmt.Errorf("os.NewFile failed: is your file descriptor valid?") } pipe, err := expect.NewPassthroughPipe(inFile) if err != nil { @@ -83,6 +83,7 @@ type InteractiveExecutionContext struct { terminalState *vt10x.State console *expect.Console passthroughTty *PassthroughPipeFile + closerProxy *closerProxy } // Tty returns the Tty of the underlying expect.Console instance @@ -132,6 +133,7 @@ func (ice *InteractiveExecutionContext) PassthroughTty() *PassthroughPipeFile { if err != nil { panic(err) } + ice.closerProxy.target = passthroughTty ice.passthroughTty = passthroughTty } return ice.passthroughTty @@ -172,15 +174,28 @@ func (ice *InteractiveCommandExecutor) SetTimeout(timeout time.Duration) { ice.consoleOpts = append(ice.consoleOpts, expect.WithDefaultTimeout(timeout)) } +type closerProxy struct { + target io.Closer +} + +func (cp *closerProxy) Close() error { + if cp.target != nil { + return cp.target.Close() + } + return nil +} + // ExecuteInInteractiveTerminal runs a pair of functions connected in an interactive virtual terminal environment func ExecuteInInteractiveTerminal( processFunc InteractiveProcessFunc, // Represents the process that the user is interacting with via the terminal userFunc InteractiveUserFunc, // Represents the user interacting with the process consoleOpts ...expect.ConsoleOpt) (*InteractiveExecutionContext, error) { + closerProxy := new(closerProxy) // Create a proxy object to close our Tty proxy later outputBuffer := new(bytes.Buffer) console, terminalState, err := vt10x.NewVT10XConsole( append([]expect.ConsoleOpt{ expect.WithStdout(outputBuffer), + expect.WithCloser(closerProxy), expect.WithDefaultTimeout(100 * time.Millisecond), }, consoleOpts...)...) if err != nil { @@ -193,6 +208,7 @@ func ExecuteInInteractiveTerminal( outputBuffer: outputBuffer, console: console, terminalState: terminalState, + closerProxy: closerProxy, } // Execute our function within a channel and wait for exit From 5a52aab4e95205de7f5ffdd7a17e91247208fb1b Mon Sep 17 00:00:00 2001 From: Blake Watters Date: Thu, 23 Apr 2020 02:42:56 -0700 Subject: [PATCH 19/22] Add an expectation observer to extend the read deadline in response to successful expectations --- test/interactive_terminal.go | 77 +++++++++++++++++++++++++++--------- 1 file changed, 59 insertions(+), 18 deletions(-) diff --git a/test/interactive_terminal.go b/test/interactive_terminal.go index 4946323..9afdb37 100644 --- a/test/interactive_terminal.go +++ b/test/interactive_terminal.go @@ -79,11 +79,20 @@ func RunTestInInteractiveTerminal(t *testing.T, // InteractiveExecutionContext describes the state of an interactive terminal execution type InteractiveExecutionContext struct { - outputBuffer *bytes.Buffer - terminalState *vt10x.State - console *expect.Console - passthroughTty *PassthroughPipeFile - closerProxy *closerProxy + outputBuffer *bytes.Buffer + terminalState *vt10x.State + console *expect.Console + passthroughTty *PassthroughPipeFile + closerProxy *closerProxy + expectDeadlineObserver *expectDeadlineObserver +} + +// ReadTimeout returns the read time for the process side of an interactive execution +// expect.Console takes care of establishing a proxy pipe on the master side of the Tty +// but in a unit testing situation we have read failures on the slave side where the process +// may be waiting for input from the user +func (ice *InteractiveExecutionContext) ReadTimeout() time.Duration { + return *ice.expectDeadlineObserver.readTimeout } // Tty returns the Tty of the underlying expect.Console instance @@ -128,13 +137,15 @@ func (ice *InteractiveExecutionContext) PassthroughTty() *PassthroughPipeFile { // Wrap the Tty into a PassthroughPipeFile to enable deadline support (NewPassthroughPipeFileReader??) if ice.passthroughTty == nil { passthroughTty, err := NewPassthroughPipeFile(ice.Tty()) - // TODO: SETTING THE DEADLINE IS CRITICAL - passthroughTty.SetReadDeadline(time.Now().Add(500 * time.Millisecond)) if err != nil { panic(err) } - ice.closerProxy.target = passthroughTty ice.passthroughTty = passthroughTty + ice.closerProxy.target = passthroughTty + ice.expectDeadlineObserver.passthroughPipe = passthroughTty.PassthroughPipe + if readTimeout := ice.ReadTimeout(); readTimeout != 0 { + passthroughTty.SetReadDeadline(time.Now().Add(readTimeout)) + } } return ice.passthroughTty } @@ -174,6 +185,7 @@ func (ice *InteractiveCommandExecutor) SetTimeout(timeout time.Duration) { ice.consoleOpts = append(ice.consoleOpts, expect.WithDefaultTimeout(timeout)) } +// Closes out the passthrough proxy type closerProxy struct { target io.Closer } @@ -185,30 +197,59 @@ func (cp *closerProxy) Close() error { return nil } +// Extends the read deadline on the passthrough pipe after expect operations +type expectDeadlineObserver struct { + passthroughPipe *expect.PassthroughPipe + readTimeout *time.Duration +} + +func (eo *expectDeadlineObserver) observeExpect(matchers []expect.Matcher, buf string, err error) { + if eo.passthroughPipe == nil || eo.readTimeout == nil || err != nil { + return + } + + err = eo.passthroughPipe.SetReadDeadline(time.Now().Add(*eo.readTimeout)) + if err != nil { + panic(err) + } +} + // ExecuteInInteractiveTerminal runs a pair of functions connected in an interactive virtual terminal environment func ExecuteInInteractiveTerminal( processFunc InteractiveProcessFunc, // Represents the process that the user is interacting with via the terminal userFunc InteractiveUserFunc, // Represents the user interacting with the process consoleOpts ...expect.ConsoleOpt) (*InteractiveExecutionContext, error) { + expectDeadlineObserver := new(expectDeadlineObserver) closerProxy := new(closerProxy) // Create a proxy object to close our Tty proxy later outputBuffer := new(bytes.Buffer) - console, terminalState, err := vt10x.NewVT10XConsole( - append([]expect.ConsoleOpt{ - expect.WithStdout(outputBuffer), - expect.WithCloser(closerProxy), - expect.WithDefaultTimeout(100 * time.Millisecond), - }, consoleOpts...)...) + consoleOpts = append([]expect.ConsoleOpt{ + expect.WithStdout(outputBuffer), + expect.WithExpectObserver(expectDeadlineObserver.observeExpect), + expect.WithCloser(closerProxy), + expect.WithDefaultTimeout(100 * time.Millisecond), + }, consoleOpts...) + console, terminalState, err := vt10x.NewVT10XConsole(consoleOpts...) if err != nil { return nil, err } defer console.Close() + // Use the same timeout in effect on the slave (user) side on the master (process) side pf the PTY + timeoutOpts := expect.ConsoleOpts{} + for _, opt := range consoleOpts { + if err := opt(&timeoutOpts); err != nil { + return nil, err + } + } + expectDeadlineObserver.readTimeout = timeoutOpts.ReadTimeout + // Create the execution context executionContext := &InteractiveExecutionContext{ - outputBuffer: outputBuffer, - console: console, - terminalState: terminalState, - closerProxy: closerProxy, + outputBuffer: outputBuffer, + console: console, + terminalState: terminalState, + closerProxy: closerProxy, + expectDeadlineObserver: expectDeadlineObserver, } // Execute our function within a channel and wait for exit From 80d0231aaaf1a3e2912f542b7a37d1ec13dbe610 Mon Sep 17 00:00:00 2001 From: Blake Watters Date: Thu, 23 Apr 2020 07:32:26 -0700 Subject: [PATCH 20/22] land foundations --- command/init_test.go | 75 +++++++++++++++++++++++++++++++----- test/interactive_terminal.go | 61 +++++++++++++++++------------ 2 files changed, 101 insertions(+), 35 deletions(-) diff --git a/command/init_test.go b/command/init_test.go index edba142..1fe0ea2 100644 --- a/command/init_test.go +++ b/command/init_test.go @@ -26,6 +26,7 @@ import ( "github.com/opsani/cli/command" "github.com/opsani/cli/test" "github.com/spf13/viper" + "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" "gopkg.in/yaml.v2" ) @@ -89,6 +90,63 @@ func (s *InitTestSuite) TestTerminalConfirm() { s.Require().False(confirmed) } +type InteractiveCommandTest struct { + console *expect.Console + context *test.InteractiveExecutionContext + s *InitTestSuite +} + +func (ict *InteractiveCommandTest) Require() *require.Assertions { + return ict.s.Require() +} + +func (ict *InteractiveCommandTest) SendLine(s string) (int, error) { + l, err := ict.console.SendLine(s) + ict.Require().NoError(err) + return l, err +} + +func (ict *InteractiveCommandTest) ExpectEOF() (string, error) { + return ict.console.ExpectEOF() +} + +func (ict *InteractiveCommandTest) ExpectString(s string) (string, error) { + return ict.console.ExpectString(s) +} + +func (ict *InteractiveCommandTest) ExpectStringf(format string, args ...interface{}) (string, error) { + return ict.ExpectString(fmt.Sprintf(format, args...)) +} + +func (ict *InteractiveCommandTest) RequireEOF() (string, error) { + l, err := ict.console.ExpectEOF() + ict.Require().NoErrorf(err, "Unexpected error %q: - ", err, ict.context.OutputBuffer().String()) + return l, err +} + +func (ict *InteractiveCommandTest) RequireString(s string) (string, error) { + l, err := ict.console.ExpectString(s) + ict.Require().NoErrorf(err, "Failed while attempting to read %q: %v", s, err) + return l, err +} + +func (ict *InteractiveCommandTest) RequireStringf(format string, args ...interface{}) (string, error) { + return ict.RequireString(fmt.Sprintf(format, args...)) +} + +func (s *InitTestSuite) RunTestCommandE( + ice *test.InteractiveCommandExecutor, + args []string, + testFunc func(*InteractiveCommandTest) error) (*test.InteractiveExecutionContext, error) { + return ice.Execute(args, func(context *test.InteractiveExecutionContext, console *expect.Console) error { + return testFunc(&InteractiveCommandTest{ + console: console, + context: context, + s: s, + }) + }) +} + func (s *InitTestSuite) TestInitWithExistingConfigDeclined() { configFile := test.TempConfigFileWithObj(map[string]string{ "app": "example.com/app", @@ -97,18 +155,15 @@ func (s *InitTestSuite) TestInitWithExistingConfigDeclined() { rootCmd := command.NewRootCommand() ice := test.NewInteractiveCommandExecutor(rootCmd) - _, err := ice.Execute(test.Args("--config", configFile.Name(), "init"), func(_ *test.InteractiveExecutionContext, console *expect.Console) error { - if _, err := console.ExpectString(fmt.Sprintf("Using config from: %s", configFile.Name())); err != nil { - return err - } - str := fmt.Sprintf("? Existing config found. Overwrite %s?", configFile.Name()) - _, err := console.ExpectString(str) - s.Require().NoErrorf(err, "Failed reading %q: %v", str, err) - _, err = console.SendLine("N") - s.Require().NoError(err) - _, err = console.ExpectEOF() + context, err := s.RunTestCommandE(ice, test.Args("--config", configFile.Name(), "init"), func(t *InteractiveCommandTest) error { + fmt.Printf("? Console = %v, Context = %v, t = %v, require = %v", t.console, t.context, t.s, t.s) + t.RequireStringf("Using config from: %s", configFile.Name()) + t.RequireStringf("? Existing config found. Overwrite %s?", configFile.Name()) + t.SendLine("N") + t.console.ExpectEOF() return nil }) + s.T().Logf("%v", context.OutputBuffer().String()) s.Require().Error(err) s.Require().EqualError(err, terminal.InterruptErr.Error()) } diff --git a/test/interactive_terminal.go b/test/interactive_terminal.go index 9afdb37..545166d 100644 --- a/test/interactive_terminal.go +++ b/test/interactive_terminal.go @@ -79,12 +79,12 @@ func RunTestInInteractiveTerminal(t *testing.T, // InteractiveExecutionContext describes the state of an interactive terminal execution type InteractiveExecutionContext struct { - outputBuffer *bytes.Buffer - terminalState *vt10x.State - console *expect.Console - passthroughTty *PassthroughPipeFile - closerProxy *closerProxy - expectDeadlineObserver *expectDeadlineObserver + outputBuffer *bytes.Buffer + terminalState *vt10x.State + console *expect.Console + passthroughTty *PassthroughPipeFile + closerProxy *closerProxy + consoleObserver *consoleObserver } // ReadTimeout returns the read time for the process side of an interactive execution @@ -92,7 +92,7 @@ type InteractiveExecutionContext struct { // but in a unit testing situation we have read failures on the slave side where the process // may be waiting for input from the user func (ice *InteractiveExecutionContext) ReadTimeout() time.Duration { - return *ice.expectDeadlineObserver.readTimeout + return *ice.consoleObserver.readTimeout } // Tty returns the Tty of the underlying expect.Console instance @@ -142,10 +142,8 @@ func (ice *InteractiveExecutionContext) PassthroughTty() *PassthroughPipeFile { } ice.passthroughTty = passthroughTty ice.closerProxy.target = passthroughTty - ice.expectDeadlineObserver.passthroughPipe = passthroughTty.PassthroughPipe - if readTimeout := ice.ReadTimeout(); readTimeout != 0 { - passthroughTty.SetReadDeadline(time.Now().Add(readTimeout)) - } + ice.consoleObserver.passthroughPipe = passthroughTty.PassthroughPipe + ice.consoleObserver.extendDeadline() } return ice.passthroughTty } @@ -197,20 +195,32 @@ func (cp *closerProxy) Close() error { return nil } -// Extends the read deadline on the passthrough pipe after expect operations -type expectDeadlineObserver struct { +// Extends the read deadline on the passthrough pipe after expect/send operations +type consoleObserver struct { passthroughPipe *expect.PassthroughPipe readTimeout *time.Duration } -func (eo *expectDeadlineObserver) observeExpect(matchers []expect.Matcher, buf string, err error) { +func (eo *consoleObserver) observeExpect(matchers []expect.Matcher, buf string, err error) { if eo.passthroughPipe == nil || eo.readTimeout == nil || err != nil { return } + eo.extendDeadline() +} - err = eo.passthroughPipe.SetReadDeadline(time.Now().Add(*eo.readTimeout)) +func (eo *consoleObserver) observeSend(msg string, num int, err error) { if err != nil { - panic(err) + eo.extendDeadline() + } +} + +func (eo *consoleObserver) extendDeadline() { + if readTimeout := eo.readTimeout; readTimeout != nil { + fmt.Printf("Read timeout is %v", *readTimeout) + err := eo.passthroughPipe.SetReadDeadline(time.Now().Add(*readTimeout)) + if err != nil { + panic(err) + } } } @@ -219,14 +229,15 @@ func ExecuteInInteractiveTerminal( processFunc InteractiveProcessFunc, // Represents the process that the user is interacting with via the terminal userFunc InteractiveUserFunc, // Represents the user interacting with the process consoleOpts ...expect.ConsoleOpt) (*InteractiveExecutionContext, error) { - expectDeadlineObserver := new(expectDeadlineObserver) + consoleObserver := new(consoleObserver) closerProxy := new(closerProxy) // Create a proxy object to close our Tty proxy later outputBuffer := new(bytes.Buffer) consoleOpts = append([]expect.ConsoleOpt{ expect.WithStdout(outputBuffer), - expect.WithExpectObserver(expectDeadlineObserver.observeExpect), + expect.WithExpectObserver(consoleObserver.observeExpect), + expect.WithSendObserver(consoleObserver.observeSend), expect.WithCloser(closerProxy), - expect.WithDefaultTimeout(100 * time.Millisecond), + expect.WithDefaultTimeout(250 * time.Millisecond), }, consoleOpts...) console, terminalState, err := vt10x.NewVT10XConsole(consoleOpts...) if err != nil { @@ -241,15 +252,15 @@ func ExecuteInInteractiveTerminal( return nil, err } } - expectDeadlineObserver.readTimeout = timeoutOpts.ReadTimeout + consoleObserver.readTimeout = timeoutOpts.ReadTimeout // Create the execution context executionContext := &InteractiveExecutionContext{ - outputBuffer: outputBuffer, - console: console, - terminalState: terminalState, - closerProxy: closerProxy, - expectDeadlineObserver: expectDeadlineObserver, + outputBuffer: outputBuffer, + console: console, + terminalState: terminalState, + closerProxy: closerProxy, + consoleObserver: consoleObserver, } // Execute our function within a channel and wait for exit From b048a645cfe92a63cc5f060d3618533dab4cdcaf Mon Sep 17 00:00:00 2001 From: Blake Watters Date: Thu, 23 Apr 2020 07:50:20 -0700 Subject: [PATCH 21/22] Refactor names for clarity and consistency --- command/init_test.go | 21 +++++++++++++++++---- test/interactive_terminal.go | 12 ++++++------ 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/command/init_test.go b/command/init_test.go index 1fe0ea2..a387554 100644 --- a/command/init_test.go +++ b/command/init_test.go @@ -34,6 +34,7 @@ import ( type InitTestSuite struct { suite.Suite *test.OpsaniCommandExecutor + *test.InteractiveCommandExecutor } func TestInitTestSuite(t *testing.T) { @@ -134,11 +135,23 @@ func (ict *InteractiveCommandTest) RequireStringf(format string, args ...interfa return ict.RequireString(fmt.Sprintf(format, args...)) } -func (s *InitTestSuite) RunTestCommandE( +func (s *InitTestSuite) ExecuteCommandInteractivelyE( ice *test.InteractiveCommandExecutor, args []string, testFunc func(*InteractiveCommandTest) error) (*test.InteractiveExecutionContext, error) { - return ice.Execute(args, func(context *test.InteractiveExecutionContext, console *expect.Console) error { + return ice.ExecuteInteractively(args, func(context *test.InteractiveExecutionContext, console *expect.Console) error { + return testFunc(&InteractiveCommandTest{ + console: console, + context: context, + s: s, + }) + }) +} + +func (s *InitTestSuite) ExecuteCommandInteractively( + args []string, + testFunc func(*InteractiveCommandTest) error) (*test.InteractiveExecutionContext, error) { + return s.ExecuteInteractively(args, func(context *test.InteractiveExecutionContext, console *expect.Console) error { return testFunc(&InteractiveCommandTest{ console: console, context: context, @@ -155,7 +168,7 @@ func (s *InitTestSuite) TestInitWithExistingConfigDeclined() { rootCmd := command.NewRootCommand() ice := test.NewInteractiveCommandExecutor(rootCmd) - context, err := s.RunTestCommandE(ice, test.Args("--config", configFile.Name(), "init"), func(t *InteractiveCommandTest) error { + context, err := s.ExecuteCommandInteractivelyE(ice, test.Args("--config", configFile.Name(), "init"), func(t *InteractiveCommandTest) error { fmt.Printf("? Console = %v, Context = %v, t = %v, require = %v", t.console, t.context, t.s, t.s) t.RequireStringf("Using config from: %s", configFile.Name()) t.RequireStringf("? Existing config found. Overwrite %s?", configFile.Name()) @@ -176,7 +189,7 @@ func (s *InitTestSuite) TestInitWithExistingConfigAccepted() { rootCmd := command.NewRootCommand() ice := test.NewInteractiveCommandExecutor(rootCmd) - context, err := ice.Execute(test.Args("--config", configFile.Name(), "init"), func(_ *test.InteractiveExecutionContext, console *expect.Console) error { + context, err := ice.ExecuteInteractively(test.Args("--config", configFile.Name(), "init"), func(_ *test.InteractiveExecutionContext, console *expect.Console) error { if _, err := console.ExpectString(fmt.Sprintf("Using config from: %s", configFile.Name())); err != nil { return err } diff --git a/test/interactive_terminal.go b/test/interactive_terminal.go index 545166d..0b042c1 100644 --- a/test/interactive_terminal.go +++ b/test/interactive_terminal.go @@ -65,7 +65,7 @@ func NewPassthroughPipeFile(inFile *os.File) (*PassthroughPipeFile, error) { } // RunTestInInteractiveTerminal runs a test within an interactive terminal environment -// Executin requires a standard test instance, a pair of functions that execute the code +// Execution requires a standard test instance, a pair of functions that execute the code // under test and the test code, and any desired options for configuring the virtual terminal environment func RunTestInInteractiveTerminal(t *testing.T, codeUnderTestFunc InteractiveProcessFunc, @@ -283,8 +283,8 @@ func ExecuteInInteractiveTerminal( return executionContext, err } -// Execute runs the specified command interactively and returns an execution context object upon completion -func (ice *InteractiveCommandExecutor) Execute(args []string, interactionFunc InteractiveUserFunc) (*InteractiveExecutionContext, error) { +// ExecuteInteractively runs the specified command interactively and returns an execution context object upon completion +func (ice *InteractiveCommandExecutor) ExecuteInteractively(args []string, interactionFunc InteractiveUserFunc) (*InteractiveExecutionContext, error) { // Wrap our execution func with setup for Command execution commandExecutionFunc := func(context *InteractiveExecutionContext) error { ice.command.SetIn(context.Stdin()) @@ -307,9 +307,9 @@ func (ice *InteractiveCommandExecutor) Execute(args []string, interactionFunc In return ExecuteInInteractiveTerminal(commandExecutionFunc, interactionFunc, ice.consoleOpts...) } -// ExecuteString executes the target command by splitting the args string at space boundaries +// ExecuteStringInteractively executes the target command by splitting the args string at space boundaries // This is a convenience interface suitable only for simple arguments that do not contain quoted values or literals // If you need something more advanced please use the Execute() and Args() method to compose from a variadic list of arguments -func (ice *InteractiveCommandExecutor) ExecuteString(argsStr string, interactionFunc InteractiveUserFunc) (*InteractiveExecutionContext, error) { - return ice.Execute(strings.Split(argsStr, " "), interactionFunc) +func (ice *InteractiveCommandExecutor) ExecuteStringInteractively(argsStr string, interactionFunc InteractiveUserFunc) (*InteractiveExecutionContext, error) { + return ice.ExecuteInteractively(strings.Split(argsStr, " "), interactionFunc) } From 440526985f9b157540f122450300a13ea282c467 Mon Sep 17 00:00:00 2001 From: Blake Watters Date: Thu, 23 Apr 2020 08:36:30 -0700 Subject: [PATCH 22/22] More polish --- command/init_test.go | 79 ++++++++++++++++++++++++------------ test/interactive_terminal.go | 1 - 2 files changed, 53 insertions(+), 27 deletions(-) diff --git a/command/init_test.go b/command/init_test.go index a387554..de1104d 100644 --- a/command/init_test.go +++ b/command/init_test.go @@ -48,6 +48,7 @@ func (s *InitTestSuite) SetupTest() { rootCmd := command.NewRootCommand() s.OpsaniCommandExecutor = test.NewOpsaniCommandExecutor(rootCmd) + s.InteractiveCommandExecutor = test.NewInteractiveCommandExecutor(rootCmd) } func (s *InitTestSuite) TestRunningInitHelp() { @@ -119,6 +120,14 @@ func (ict *InteractiveCommandTest) ExpectStringf(format string, args ...interfac return ict.ExpectString(fmt.Sprintf(format, args...)) } +func (ict *InteractiveCommandTest) ExpectMatch(opts ...expect.ExpectOpt) (string, error) { + return ict.console.Expect(opts...) +} + +func (ict *InteractiveCommandTest) ExpectMatches(opts ...expect.ExpectOpt) (string, error) { + return ict.console.Expect(opts...) +} + func (ict *InteractiveCommandTest) RequireEOF() (string, error) { l, err := ict.console.ExpectEOF() ict.Require().NoErrorf(err, "Unexpected error %q: - ", err, ict.context.OutputBuffer().String()) @@ -135,6 +144,18 @@ func (ict *InteractiveCommandTest) RequireStringf(format string, args ...interfa return ict.RequireString(fmt.Sprintf(format, args...)) } +func (ict *InteractiveCommandTest) RequireMatch(opts ...expect.ExpectOpt) (string, error) { + l, err := ict.console.Expect(opts...) + ict.Require().NoErrorf(err, "Failed while attempting to find a matcher for %q: %v", l, err) + return l, err +} + +func (ict *InteractiveCommandTest) RequireMatches(opts ...expect.ExpectOpt) (string, error) { + l, err := ict.console.Expect(opts...) + ict.Require().NoErrorf(err, "Failed while attempting to find a matcher for %q: %v", l, err) + return l, err +} + func (s *InitTestSuite) ExecuteCommandInteractivelyE( ice *test.InteractiveCommandExecutor, args []string, @@ -160,15 +181,13 @@ func (s *InitTestSuite) ExecuteCommandInteractively( }) } -func (s *InitTestSuite) TestInitWithExistingConfigDeclined() { +func (s *InitTestSuite) TestInitWithExistingConfigDeclinedL() { configFile := test.TempConfigFileWithObj(map[string]string{ "app": "example.com/app", "token": "123456", }) - rootCmd := command.NewRootCommand() - ice := test.NewInteractiveCommandExecutor(rootCmd) - context, err := s.ExecuteCommandInteractivelyE(ice, test.Args("--config", configFile.Name(), "init"), func(t *InteractiveCommandTest) error { + context, err := s.ExecuteCommandInteractively(test.Args("--config", configFile.Name(), "init"), func(t *InteractiveCommandTest) error { fmt.Printf("? Console = %v, Context = %v, t = %v, require = %v", t.console, t.context, t.s, t.s) t.RequireStringf("Using config from: %s", configFile.Name()) t.RequireStringf("? Existing config found. Overwrite %s?", configFile.Name()) @@ -181,34 +200,42 @@ func (s *InitTestSuite) TestInitWithExistingConfigDeclined() { s.Require().EqualError(err, terminal.InterruptErr.Error()) } +func (s *InitTestSuite) TestInitWithExistingConfigDeclined() { + configFile := test.TempConfigFileWithObj(map[string]string{ + "app": "example.com/app", + "token": "123456", + }) + + context, err := s.ExecuteCommandInteractively(test.Args("--config", configFile.Name(), "init"), func(t *InteractiveCommandTest) error { + t.RequireStringf("Using config from: %s", configFile.Name()) + t.RequireStringf("? Existing config found. Overwrite %s?", configFile.Name()) + t.SendLine("N") + t.ExpectEOF() + return nil + }) + s.T().Logf("%v", context.OutputBuffer().String()) + s.Require().Error(err) + s.Require().EqualError(err, terminal.InterruptErr.Error()) +} + func (s *InitTestSuite) TestInitWithExistingConfigAccepted() { configFile := test.TempConfigFileWithObj(map[string]string{ "app": "example.com/app", "token": "123456", }) - rootCmd := command.NewRootCommand() - ice := test.NewInteractiveCommandExecutor(rootCmd) - context, err := ice.ExecuteInteractively(test.Args("--config", configFile.Name(), "init"), func(_ *test.InteractiveExecutionContext, console *expect.Console) error { - if _, err := console.ExpectString(fmt.Sprintf("Using config from: %s", configFile.Name())); err != nil { - return err - } - str := fmt.Sprintf("? Existing config found. Overwrite %s?", configFile.Name()) - _, err := console.ExpectString(str) - s.Require().NoErrorf(err, "Failed reading %q: %v", str, err) - _, err = console.SendLine("Y") - s.Require().NoError(err) - _, err = console.Expect(expect.RegexpPattern("Opsani app")) - _, err = console.SendLine("dev.opsani.com/amazing-app") - console.Expect(expect.RegexpPattern("API Token")) - _, err = console.SendLine("123456") - str = fmt.Sprintf("Write to %s?", configFile.Name()) - console.Expect(expect.RegexpPattern(str)) - - _, err = console.SendLine("Y") - s.Require().NoError(err) - console.Expect(expect.RegexpPattern("Opsani config initialized")) - console.ExpectEOF() + context, err := s.ExecuteCommandInteractively(test.Args("--config", configFile.Name(), "init"), func(t *InteractiveCommandTest) error { + t.RequireStringf("Using config from: %s", configFile.Name()) + t.RequireStringf("? Existing config found. Overwrite %s?", configFile.Name()) + t.SendLine("Y") + t.ExpectMatch(expect.RegexpPattern("Opsani app")) + t.SendLine("dev.opsani.com/amazing-app") + t.RequireMatch(expect.RegexpPattern("API Token")) + t.SendLine("123456") + t.RequireMatch(expect.RegexpPattern(fmt.Sprintf("Write to %s?", configFile.Name()))) + + t.SendLine("Y") + t.RequireMatch(expect.RegexpPattern("Opsani CLI initialized")) return nil }) s.Require().NoError(err, context.OutputBuffer().String()) diff --git a/test/interactive_terminal.go b/test/interactive_terminal.go index 0b042c1..39f8235 100644 --- a/test/interactive_terminal.go +++ b/test/interactive_terminal.go @@ -216,7 +216,6 @@ func (eo *consoleObserver) observeSend(msg string, num int, err error) { func (eo *consoleObserver) extendDeadline() { if readTimeout := eo.readTimeout; readTimeout != nil { - fmt.Printf("Read timeout is %v", *readTimeout) err := eo.passthroughPipe.SetReadDeadline(time.Now().Add(*readTimeout)) if err != nil { panic(err)