Merge pull request sequelize#594 from aldraco/submit_cmd_standardize

Standardize submit command (sequelize#550, sequelize#551, sequelize#547, sequelize#549)

cmd/submit.go

@@ -36,23 +36,56 @@ If called with the name of an exercise, it will work out which
 track it is on and submit it. The command will ask for help
 figuring things out if necessary.
 `,
+	Args: cobra.MaximumNArgs(1),
 	RunE: func(cmd *cobra.Command, args []string) error {
-		usrCfg, err := config.NewUserConfig()
+		// Validate input before doing any other work
+		exercise, err := cmd.Flags().GetString("exercise")
 		if err != nil {
 			return err
 		}
 
-		cliCfg, err := config.NewCLIConfig()
+		trackID, err := cmd.Flags().GetString("track")
 		if err != nil {
 			return err
 		}
 
-		if len(args) == 0 {
-			cwd, err := os.Getwd()
-			if err != nil {
-				return err
+		files, err := cmd.Flags().GetStringSlice("files")
+		if err != nil {
+			return err
+		}
+
+		// Verify that both --track and --exercise are used together
+		if len(args) == 0 && len(files) == 0 && !(exercise != "" && trackID != "") {
+			// Are they both missing?
+			if exercise == "" && trackID == "" {
+				return errors.New("Please use the --exercise/--trackID flags to submit without an explicit directory or files.")
 			}
-			args = []string{cwd}
+			// Guess that --trackID is missing, unless it's not
+			present, missing := "--exercise", "--track"
+			if trackID != "" {
+				present, missing = missing, present
+			}
+			// Help user correct CLI command
+			missingFlagMessage := fmt.Sprintf("You specified %s, please also include %s.", present, missing)
+			return errors.New(missingFlagMessage)
+		}
+
+		if len(args) > 0 && (exercise != "" || trackID != "") {
+			return errors.New("You are submitting a directory. We will infer the track and exercise from that. Please re-run the submit command without the flags.")
+		}
+
+		if len(files) > 0 && len(args) > 0 {
+			return errors.New("You can submit either a list of files, or a directory, but not both.")
+		}
+
+		usrCfg, err := config.NewUserConfig()
+		if err != nil {
+			return err
+		}
+
+		cliCfg, err := config.NewCLIConfig()
+		if err != nil {
+			return err
 		}
 
 		// TODO: make sure we get the workspace configured.
@@ -65,6 +98,14 @@ figuring things out if necessary.
 		}
 
 		ws := workspace.New(usrCfg.Workspace)
+
+		// Create directory from track and exercise slugs if needed
+		if trackID != "" && exercise != "" {
+			args = []string{filepath.Join(ws.Dir, trackID, exercise)}
+		} else if len(files) > 0 {
+			args = files
+		}
+
 		tx, err := workspace.NewTransmission(ws.Dir, args)
 		if err != nil {
 			return err
@@ -110,6 +151,7 @@ figuring things out if necessary.
 		if !solution.IsRequester {
 			return errors.New("not your solution")
 		}
+
 		track := cliCfg.Tracks[solution.Track]
 		if track == nil {
 			err := prepareTrack(solution.Track)
@@ -242,7 +284,9 @@ You can complete the exercise and unlock the next core exercise at:
 }
 
 func initSubmitCmd() {
-	// TODO
+	submitCmd.Flags().StringP("track", "t", "", "the track ID")
+	submitCmd.Flags().StringP("exercise", "e", "", "the exercise ID")
+	submitCmd.Flags().StringSliceP("files", "f", make([]string, 0), "files to submit")
 }
 
 func init() {

cmd/submit_test.go

@@ -1,6 +1,7 @@
 package cmd
 
 import (
+	"fmt"
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
@@ -43,86 +44,116 @@ func TestSubmit(t *testing.T) {
 		relativePath: "README.md",
 		contents:     "The readme.",
 	}
-
-	cmdTest := &CommandTest{
+	// Make a list of test commands
+	cmdTestFlags := &CommandTest{
 		Cmd:                     submitCmd,
 		InitFn:                  initSubmitCmd,
 		MockInteractiveResponse: "\n",
-		Args: []string{"fakeapp", "submit", "bogus-exercise"},
+		Args: []string{"fakeapp", "submit", "-e", "bogus-exercise", "-t", "bogus-track"},
 	}
-	cmdTest.Setup(t)
-	defer cmdTest.Teardown(t)
-
-	// Create a temp dir for the config and the exercise files.
-	dir := filepath.Join(cmdTest.TmpDir, "bogus-track", "bogus-exercise")
-	os.MkdirAll(filepath.Join(dir, "subdir"), os.FileMode(0755))
-
-	solution := &workspace.Solution{
-		ID:          "bogus-solution-uuid",
-		Track:       "bogus-track",
-		Exercise:    "bogus-exercise",
-		IsRequester: true,
+	cmdTestRelativeDir := &CommandTest{
+		Cmd:                     submitCmd,
+		InitFn:                  initSubmitCmd,
+		MockInteractiveResponse: "\n",
+		Args: []string{"fakeapp", "submit", filepath.Join("bogus-track", "bogus-exercise")},
 	}
-	err := solution.Write(dir)
-	assert.NoError(t, err)
-
-	for _, file := range []file{file1, file2, file3} {
-		err := ioutil.WriteFile(filepath.Join(dir, file.relativePath), []byte(file.contents), os.FileMode(0755))
-		assert.NoError(t, err)
+	cmdTestFilesFlag := &CommandTest{
+		Cmd:                     submitCmd,
+		InitFn:                  initSubmitCmd,
+		MockInteractiveResponse: "\n",
+		Args: []string{"fakeapp", "submit", "--files"},
 	}
+	tests := []*CommandTest{
+		cmdTestFlags,
+		cmdTestRelativeDir,
+		cmdTestFilesFlag,
+	}
+	for _, cmdTest := range tests {
+		cmdTest.Setup(t)
+		defer cmdTest.Teardown(t)
+
+		// Prefix submitted filenames with correct temporary directory
+		if cmdTest.Args[2] == "--files" {
+			filenames := make([]string, 2)
+			for i, file := range []file{file1, file2} {
+				filenames[i] = filepath.Join(cmdTest.TmpDir, "bogus-track", "bogus-exercise", file.relativePath)
+			}
+			filenameString := strings.Join(filenames, ",")
+			cmdTest.Args[2] = fmt.Sprintf("-f=%s", filenameString)
+		} else if len(cmdTest.Args) == 3 {
+			cmdTest.Args[2] = filepath.Join(cmdTest.TmpDir, cmdTest.Args[2])
+		}
+		// Create a temp dir for the config and the exercise files.
+		dir := filepath.Join(cmdTest.TmpDir, "bogus-track", "bogus-exercise")
+		os.MkdirAll(filepath.Join(dir, "subdir"), os.FileMode(0755))
+
+		solution := &workspace.Solution{
+			ID:          "bogus-solution-uuid",
+			Track:       "bogus-track",
+			Exercise:    "bogus-exercise",
+			IsRequester: true,
+		}
+		err := solution.Write(dir)
+		assert.NoError(t, err)
 
-	// The fake endpoint will populate this when it receives the call from the command.
-	submittedFiles := map[string]string{}
-
-	// Set up the test server.
-	fakeEndpoint := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		err := r.ParseMultipartForm(2 << 10)
-		if err != nil {
-			t.Fatal(err)
+		for _, file := range []file{file1, file2, file3} {
+			err := ioutil.WriteFile(filepath.Join(dir, file.relativePath), []byte(file.contents), os.FileMode(0755))
+			assert.NoError(t, err)
 		}
-		mf := r.MultipartForm
 
-		files := mf.File["files[]"]
-		for _, fileHeader := range files {
-			file, err := fileHeader.Open()
+		// The fake endpoint will populate this when it receives the call from the command.
+		submittedFiles := map[string]string{}
+
+		// Set up the test server.
+		fakeEndpoint := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			err := r.ParseMultipartForm(2 << 10)
 			if err != nil {
 				t.Fatal(err)
 			}
-			defer file.Close()
-			body, err := ioutil.ReadAll(file)
-			if err != nil {
-				t.Fatal(err)
+			mf := r.MultipartForm
+
+			files := mf.File["files[]"]
+			for _, fileHeader := range files {
+				file, err := fileHeader.Open()
+				if err != nil {
+					t.Fatal(err)
+				}
+				defer file.Close()
+				body, err := ioutil.ReadAll(file)
+				if err != nil {
+					t.Fatal(err)
+				}
+				submittedFiles[fileHeader.Filename] = string(body)
 			}
-			submittedFiles[fileHeader.Filename] = string(body)
+		})
+		ts := httptest.NewServer(fakeEndpoint)
+		defer ts.Close()
+
+		// Create a fake user config.
+		usrCfg := config.NewEmptyUserConfig()
+		usrCfg.Workspace = cmdTest.TmpDir
+		usrCfg.APIBaseURL = ts.URL
+		err = usrCfg.Write()
+		assert.NoError(t, err)
+
+		// Create a fake CLI config.
+		cliCfg, err := config.NewCLIConfig()
+		assert.NoError(t, err)
+		cliCfg.Tracks["bogus-track"] = config.NewTrack("bogus-track")
+		err = cliCfg.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()
+
+		// We got only the file we expected.
+		assert.Equal(t, 2, len(submittedFiles))
+		for _, file := range []file{file1, file2} {
+			path := string(os.PathSeparator) + file.relativePath
+			assert.Equal(t, file.contents, submittedFiles[path])
 		}
-	})
-	ts := httptest.NewServer(fakeEndpoint)
-	defer ts.Close()
-
-	// Create a fake user config.
-	usrCfg := config.NewEmptyUserConfig()
-	usrCfg.Workspace = cmdTest.TmpDir
-	usrCfg.APIBaseURL = ts.URL
-	err = usrCfg.Write()
-	assert.NoError(t, err)
-
-	// Create a fake CLI config.
-	cliCfg, err := config.NewCLIConfig()
-	assert.NoError(t, err)
-	cliCfg.Tracks["bogus-track"] = config.NewTrack("bogus-track")
-	err = cliCfg.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()
-
-	// We got only the file we expected.
-	assert.Equal(t, 2, len(submittedFiles))
-	for _, file := range []file{file1, file2} {
-		path := string(os.PathSeparator) + file.relativePath
-		assert.Equal(t, file.contents, submittedFiles[path])
 	}
 }