Repository: exercism/cli
Pull request #503 (2017-11-30 03:49:40)

[nextercism] Interactive confirmation when submitting directory (#485)
When a user submits a directory, show them a list of (pre-filtered) files and allow them to confirm/pass on the whole selection.

Changes that support the interactive prompt:
 - made `Transmission.ArgDirs` public in order to check it in `submit.go`.

Otherwise, the interaction/prompts would need to take place inside code for a Transmission, which seemed like a bad separation of concerns. In our case, as long as `ArgDirs` == 1, then the user gave a whole directory, and we don't need to figure out anything else.

For testing:
 - added an In variable, like the Out, into `cmd/root`
 - added MockInput to `cmd_test`

This makes it possible to mock interactive responses in tests. There are several Issues talking about interactive submission/confirmation etc (#485, #480, #463, #462, #461 ), so I figured having the ability to test them would be nice. If the testing changes should be a separate PR, happy to split it out!

aldraco: Thank you @kytrinyx for the review -- did not know about `go fmt`, handy tool. I think I have addressed all of your review comments.

Patch:
diff --git a/cli/asset.go b/cli/asset.go
index 19e3a16..246895f 100644
--- a/cli/asset.go
+++ b/cli/asset.go
@@ -15,7 +15,7 @@ type Asset struct {
 }
 
 func (a *Asset) download() (*bytes.Reader, error) {
-	downloadURL := fmt.Sprintf("https://api.github.com/repos/exercism/cli/releases/assets/%d", a.ID)
+	downloadURL := fmt.Sprintf("%s/assets/%d", ReleaseURL, a.ID)
 	req, err := http.NewRequest("GET", downloadURL, nil)
 	if err != nil {
 		return nil, err
diff --git a/cli/cli.go b/cli/cli.go
index 304acbb..c374fd0 100644
--- a/cli/cli.go
+++ b/cli/cli.go
@@ -45,8 +45,8 @@ var (
 var (
 	// HTTPClient is the client used to make HTTP calls in the cli package.
 	HTTPClient = &http.Client{Timeout: 10 * time.Second}
-	// LatestReleaseURL is the endpoint that provides information about the latest release.
-	LatestReleaseURL = "https://api.github.com/repos/exercism/cli/releases/latest"
+	// ReleaseURL is the endpoint that provides information about cli releases.
+	ReleaseURL = "https://api.github.com/repos/exercism/cli/releases"
 )
 
 // Updater is a simple upgradable file interface.
@@ -133,7 +133,8 @@ func (c *CLI) Upgrade() error {
 }
 
 func (c *CLI) fetchLatestRelease() error {
-	resp, err := HTTPClient.Get(LatestReleaseURL)
+	latestReleaseURL := fmt.Sprintf("%s/%s", ReleaseURL, "latest")
+	resp, err := HTTPClient.Get(latestReleaseURL)
 	if err != nil {
 		return err
 	}
diff --git a/cmd/root.go b/cmd/root.go
index c5eda1f..1e71dce 100644
--- a/cmd/root.go
+++ b/cmd/root.go
@@ -19,6 +19,8 @@ var (
 	BinaryName string
 	// Out is used to write to the required writer.
 	Out io.Writer
+	// In is used to provide mocked test input (i.e. for prompts).
+	In io.Reader
 )
 
 // RootCmd represents the base command when called without any subcommands.
@@ -41,5 +43,6 @@ func Execute() {
 func init() {
 	BinaryName = os.Args[0]
 	Out = os.Stdout
+	In = os.Stdin
 	api.UserAgent = fmt.Sprintf("github.com/exercism/cli v%s (%s/%s)", Version, runtime.GOOS, runtime.GOARCH)
 }
diff --git a/cmd/submit.go b/cmd/submit.go
index c3708bb..850b107 100644
--- a/cmd/submit.go
+++ b/cmd/submit.go
@@ -96,12 +96,11 @@ figuring things out if necessary.
 			> `
 			option, err := selection.Pick(prompt)
 			if err != nil {
-				fmt.Println(err)
-				continue
+				return err
 			}
 			s, ok := option.(*workspace.Solution)
 			if !ok {
-				fmt.Println("something went wrong trying to pick that solution, not sure what happened")
+				fmt.Fprintf(Out, "something went wrong trying to pick that solution, not sure what happened")
 				continue
 			}
 			solution = s
@@ -144,6 +143,31 @@ figuring things out if necessary.
 			return errors.New("no files found to submit")
 		}
 
+		// If the user submits a directory, confirm the list of files.
+		if len(tx.ArgDirs) > 0 {
+			prompt := "You specified a directory, which contains these files:\n"
+			for i, path := range paths {
+				prompt += fmt.Sprintf(" [%d]  %s\n", i+1, path)
+			}
+			prompt += "\nPress ENTER to submit, or control + c to cancel: "
+
+			confirmQuestion := &comms.Question{
+				Prompt:       prompt,
+				DefaultValue: "y",
+				Reader:       In,
+				Writer:       Out,
+			}
+			answer, err := confirmQuestion.Ask()
+			if err != nil {
+				return err
+			}
+			if strings.ToLower(answer) != "y" {
+				fmt.Fprintf(Out, "Submit cancelled.\nTry submitting individually instead.")
+				return nil
+			}
+			fmt.Fprintf(Out, "Submitting files now...")
+		}
+
 		for _, path := range paths {
 			// Don't submit empty files
 			info, err := os.Stat(path)
diff --git a/workspace/transmission.go b/workspace/transmission.go
index f277779..0f1f63e 100644
--- a/workspace/transmission.go
+++ b/workspace/transmission.go
@@ -9,7 +9,7 @@ import (
 type Transmission struct {
 	Files   []string
 	Dir     string
-	argDirs []string
+	ArgDirs []string
 }
 
 // NewTransmission processes the arguments to the submit command to prep a submission.
@@ -29,12 +29,12 @@ func NewTransmission(root string, args []string) (*Transmission, error) {
 			continue
 		}
 		// For our purposes, if it's not a file then it's a directory.
-		tx.argDirs = append(tx.argDirs, arg)
+		tx.ArgDirs = append(tx.ArgDirs, arg)
 	}
-	if len(tx.argDirs) > 1 {
+	if len(tx.ArgDirs) > 1 {
 		return nil, errors.New("more than one dir")
 	}
-	if len(tx.argDirs) > 0 && len(tx.Files) > 0 {
+	if len(tx.ArgDirs) > 0 && len(tx.Files) > 0 {
 		return nil, errors.New("mixing files and dirs")
 	}
 	if len(tx.Files) > 0 {
@@ -52,8 +52,8 @@ func NewTransmission(root string, args []string) (*Transmission, error) {
 			return nil, errors.New("files are from more than one solution")
 		}
 	}
-	if len(tx.argDirs) == 1 {
-		tx.Dir = tx.argDirs[0]
+	if len(tx.ArgDirs) == 1 {
+		tx.Dir = tx.ArgDirs[0]
 	}
 	return tx, nil
 }


Test patch:
diff --git a/cli/cli_test.go b/cli/cli_test.go
index a525974..b7968a0 100644
--- a/cli/cli_test.go
+++ b/cli/cli_test.go
@@ -55,11 +55,16 @@ func TestIsUpToDate(t *testing.T) {
 
 func TestIsUpToDateWithoutRelease(t *testing.T) {
 	fakeEndpoint := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		// Checking for the latest release should call latestReleaseURL endpoint.
+		// if the code below fails to return the proper response then the URL generation logic in pkg cli has changed.
+		if r.URL.Path != "/latest" {
+			fmt.Fprintln(w, "")
+		}
 		fmt.Fprintln(w, `{"tag_name": "v2.0.0"}`)
 	})
 	ts := httptest.NewServer(fakeEndpoint)
 	defer ts.Close()
-	LatestReleaseURL = ts.URL
+	ReleaseURL = ts.URL
 
 	c := &CLI{
 		Version: "1.0.0",
diff --git a/cmd/cmd_test.go b/cmd/cmd_test.go
index da1e92b..1a4d7eb 100644
--- a/cmd/cmd_test.go
+++ b/cmd/cmd_test.go
@@ -18,6 +18,8 @@ const cfgHomeKey = "EXERCISM_CONFIG_HOME"
 // The args are the faked out os.Args. The first two arguments
 // in the Args will be ignored. These represent the command (e.g. exercism)
 // and the subcommand (e.g. download).
+// Pass any interactive responses needed for the test in a single
+// String in MockInput, delimited by newlines.
 //
 // Finally, when you have done whatever other setup you need in your
 // test, call the command by calling Execute on the App.
@@ -27,18 +29,20 @@ const cfgHomeKey = "EXERCISM_CONFIG_HOME"
 // 	Cmd:    myCmd,
 // 	InitFn: initMyCmd,
 // 	Args:   []string{"fakeapp", "mycommand", "arg1", "--flag", "value"},
+// 	MockInteractiveResponse: "first-input\nsecond\n",
 // }
 // cmdTest.Setup(t)
 // defer cmdTest.Teardown(t)
 // ...
 // cmdTest.App.Execute()
 type CommandTest struct {
-	App            *cobra.Command
-	Cmd            *cobra.Command
-	InitFn         func()
-	TmpDir         string
-	Args           []string
-	OriginalValues struct {
+	App                     *cobra.Command
+	Cmd                     *cobra.Command
+	InitFn                  func()
+	TmpDir                  string
+	Args                    []string
+	MockInteractiveResponse string
+	OriginalValues          struct {
 		ConfigHome string
 		Args       []string
 	}
diff --git a/cmd/submit_test.go b/cmd/submit_test.go
index d72f261..180ed1b 100644
--- a/cmd/submit_test.go
+++ b/cmd/submit_test.go
@@ -6,6 +6,7 @@ import (
 	"net/http/httptest"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 
 	"github.com/exercism/cli/config"
@@ -15,8 +16,11 @@ import (
 
 func TestSubmit(t *testing.T) {
 	oldOut := Out
+	oldIn := In
 	Out = ioutil.Discard
+
 	defer func() { Out = oldOut }()
+	defer func() { In = oldIn }()
 
 	type file struct {
 		relativePath string
@@ -37,9 +41,10 @@ func TestSubmit(t *testing.T) {
 	}
 
 	cmdTest := &CommandTest{
-		Cmd:    submitCmd,
-		InitFn: initSubmitCmd,
-		Args:   []string{"fakeapp", "submit", "bogus-exercise"},
+		Cmd:                     submitCmd,
+		InitFn:                  initSubmitCmd,
+		MockInteractiveResponse: "\n",
+		Args: []string{"fakeapp", "submit", "bogus-exercise"},
 	}
 	cmdTest.Setup(t)
 	defer cmdTest.Teardown(t)
@@ -111,6 +116,9 @@ func TestSubmit(t *testing.T) {
 	err = apiCfg.Write()
 	assert.NoError(t, err)
 
+	// Write mock interactive input to In for the CLI command.
+	In = strings.NewReader(cmdTest.MockInteractiveResponse)
+
 	// Execute the command!
 	cmdTest.App.Execute()
 
diff --git a/cmd/version_test.go b/cmd/version_test.go
index d023428..e2a55a1 100644
--- a/cmd/version_test.go
+++ b/cmd/version_test.go
@@ -23,7 +23,7 @@ func TestVersionUpdateCheck(t *testing.T) {
 	})
 	ts := httptest.NewServer(fakeEndpoint)
 	defer ts.Close()
-	cli.LatestReleaseURL = ts.URL
+	cli.ReleaseURL = ts.URL
 
 	tests := []struct {
 		version  string