-
-
Notifications
You must be signed in to change notification settings - Fork 361
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add test
command to run any unit test
#1092
Changes from 18 commits
2abd3dd
0f8f7a8
3c2c3b8
92b9252
345a627
f0e69d0
7844519
7613a46
5545bc7
7169a7c
c4bc591
540bef5
fc6b22c
dcdf168
ffe0f04
8d18c39
238b572
cb92807
0dc184e
ad33864
33cf452
9da7a31
d790992
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,84 @@ | ||||||
package cmd | ||||||
|
||||||
import ( | ||||||
"fmt" | ||||||
"log" | ||||||
"os" | ||||||
"os/exec" | ||||||
"strings" | ||||||
|
||||||
"github.com/exercism/cli/workspace" | ||||||
"github.com/spf13/cobra" | ||||||
) | ||||||
|
||||||
var testCmd = &cobra.Command{ | ||||||
Use: "test", | ||||||
Aliases: []string{"t"}, | ||||||
Short: "Run the exercise's tests.", | ||||||
Long: `Run the exercise's tests. | ||||||
|
||||||
Run this command in an exercise's root directory.`, | ||||||
RunE: func(cmd *cobra.Command, args []string) error { | ||||||
return runTest(args) | ||||||
}, | ||||||
} | ||||||
|
||||||
func runTest(args []string) error { | ||||||
track, err := getTrack() | ||||||
if err != nil { | ||||||
return err | ||||||
} | ||||||
|
||||||
testConf, ok := workspace.TestConfigurations[track] | ||||||
|
||||||
if !ok { | ||||||
return fmt.Errorf("test handler for the `%s` track not yet implemented. Please see HELP.md for testing instructions", track) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice to point to the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I also noticed that every track's testing instructions are at There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You could, but I think There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wonder if we should maybe no use "test handler", which is somewhat technical, but use a more functional description. Maybe:
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah, great call. I went with |
||||||
} | ||||||
|
||||||
command, err := testConf.GetTestCommand() | ||||||
if err != nil { | ||||||
return err | ||||||
} | ||||||
cmdParts := strings.Split(command, " ") | ||||||
|
||||||
// pass args/flags to this command down to the test handler | ||||||
if len(args) > 0 { | ||||||
cmdParts = append(cmdParts, args...) | ||||||
} | ||||||
|
||||||
fmt.Printf("--> %s\n", strings.Join(cmdParts, " ")) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I really like the logging! My only concern is that it might be too subtle for the student to notice. Maybe we could make it more explicit? To instead of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure! updated to:
which makes it clear exactly what's running and gives it some space to breathe |
||||||
exerciseTestCmd := exec.Command(cmdParts[0], cmdParts[1:]...) | ||||||
|
||||||
// pipe output directly out, preserving any color | ||||||
exerciseTestCmd.Stdout = os.Stdout | ||||||
exerciseTestCmd.Stderr = os.Stderr | ||||||
|
||||||
err = exerciseTestCmd.Run() | ||||||
if err != nil { | ||||||
// unclear what other errors would pop up here, but it pays to be defensive | ||||||
if exitErr, ok := err.(*exec.ExitError); ok { | ||||||
exitCode := exitErr.ExitCode() | ||||||
// if subcommand returned a non-zero exit code, exit with the same | ||||||
os.Exit(exitCode) | ||||||
} else { | ||||||
log.Fatalf("Failed to get error from failed subcommand: %v", err) | ||||||
} | ||||||
} | ||||||
return nil | ||||||
} | ||||||
|
||||||
func getTrack() (string, error) { | ||||||
metadata, err := workspace.NewExerciseMetadata(".") | ||||||
if err != nil { | ||||||
return "", err | ||||||
} | ||||||
if metadata.Track == "" { | ||||||
return "", fmt.Errorf("no track found in exercise metadata") | ||||||
} | ||||||
|
||||||
return metadata.Track, nil | ||||||
} | ||||||
|
||||||
func init() { | ||||||
RootCmd.AddCommand(testCmd) | ||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
package workspace | ||
|
||
import ( | ||
"encoding/json" | ||
"errors" | ||
"io/ioutil" | ||
"path/filepath" | ||
) | ||
|
||
const configFilename = "config.json" | ||
|
||
var configFilepath = filepath.Join(ignoreSubdir, configFilename) | ||
|
||
// ExerciseConfig contains exercise metadata. | ||
// Note: we only use a subset of its fields | ||
type ExerciseConfig struct { | ||
Files struct { | ||
Solution []string `json:"solution"` | ||
Test []string `json:"test"` | ||
} `json:"files"` | ||
} | ||
|
||
// NewExerciseConfig reads exercise metadata from a file in the given directory. | ||
func NewExerciseConfig(dir string) (*ExerciseConfig, error) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I like how this is now "a thing". Maybe the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah! that would make sense. |
||
b, err := ioutil.ReadFile(filepath.Join(dir, configFilepath)) | ||
if err != nil { | ||
return nil, err | ||
} | ||
var config ExerciseConfig | ||
if err := json.Unmarshal(b, &config); err != nil { | ||
return nil, err | ||
} | ||
|
||
return &config, nil | ||
} | ||
|
||
// GetTestFiles finds returns the names of the file(s) that hold unit tests for this exercise, if any | ||
func (c *ExerciseConfig) GetSolutionFiles() ([]string, error) { | ||
result := c.Files.Solution | ||
if result == nil { | ||
// solution file(s) key was missing in config json, which is an error when calling this fuction | ||
return []string{}, errors.New("no `files.solution` key in your `config.json`. Was it removed by mistake?") | ||
} | ||
|
||
return result, nil | ||
} | ||
|
||
// GetTestFiles finds returns the names of the file(s) that hold unit tests for this exercise, if any | ||
func (c *ExerciseConfig) GetTestFiles() ([]string, error) { | ||
result := c.Files.Test | ||
if result == nil { | ||
// test file(s) key was missing in config json, which is an error when calling this fuction | ||
return []string{}, errors.New("no `files.test` key in your `config.json`. Was it removed by mistake?") | ||
} | ||
|
||
return result, nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,98 @@ | ||
package workspace | ||
|
||
import ( | ||
"io/ioutil" | ||
"os" | ||
"path/filepath" | ||
"strings" | ||
"testing" | ||
|
||
"github.com/stretchr/testify/assert" | ||
) | ||
|
||
func TestExerciseConfig(t *testing.T) { | ||
dir, err := ioutil.TempDir("", "exercise_config") | ||
assert.NoError(t, err) | ||
defer os.RemoveAll(dir) | ||
|
||
err = os.Mkdir(filepath.Join(dir, ".exercism"), os.ModePerm) | ||
assert.NoError(t, err) | ||
|
||
f, err := os.Create(filepath.Join(dir, ".exercism", "config.json")) | ||
assert.NoError(t, err) | ||
defer f.Close() | ||
|
||
_, err = f.WriteString(`{ "blurb": "Learn about the basics of Ruby by following a lasagna recipe.", "authors": ["iHiD", "pvcarrera"], "files": { "solution": ["lasagna.rb"], "test": ["lasagna_test.rb"], "exemplar": [".meta/exemplar.rb"] } } `) | ||
assert.NoError(t, err) | ||
|
||
ec, err := NewExerciseConfig(dir) | ||
assert.NoError(t, err) | ||
|
||
assert.Equal(t, ec.Files.Solution, []string{"lasagna.rb"}) | ||
solutionFiles, err := ec.GetSolutionFiles() | ||
assert.NoError(t, err) | ||
assert.Equal(t, solutionFiles, []string{"lasagna.rb"}) | ||
|
||
assert.Equal(t, ec.Files.Test, []string{"lasagna_test.rb"}) | ||
testFiles, err := ec.GetTestFiles() | ||
assert.NoError(t, err) | ||
assert.Equal(t, testFiles, []string{"lasagna_test.rb"}) | ||
} | ||
|
||
func TestExerciseConfigNoTestKey(t *testing.T) { | ||
dir, err := ioutil.TempDir("", "exercise_config") | ||
assert.NoError(t, err) | ||
defer os.RemoveAll(dir) | ||
|
||
err = os.Mkdir(filepath.Join(dir, ".exercism"), os.ModePerm) | ||
assert.NoError(t, err) | ||
|
||
f, err := os.Create(filepath.Join(dir, ".exercism", "config.json")) | ||
assert.NoError(t, err) | ||
defer f.Close() | ||
|
||
_, err = f.WriteString(`{ "blurb": "Learn about the basics of Ruby by following a lasagna recipe.", "authors": ["iHiD", "pvcarrera"], "files": { "exemplar": [".meta/exemplar.rb"] } } `) | ||
assert.NoError(t, err) | ||
|
||
ec, err := NewExerciseConfig(dir) | ||
assert.NoError(t, err) | ||
|
||
_, err = ec.GetSolutionFiles() | ||
assert.Error(t, err, "no `files.solution` key in your `config.json`") | ||
_, err = ec.GetTestFiles() | ||
assert.Error(t, err, "no `files.test` key in your `config.json`") | ||
} | ||
|
||
func TestMissingExerciseConfig(t *testing.T) { | ||
dir, err := ioutil.TempDir("", "exercise_config") | ||
assert.NoError(t, err) | ||
defer os.RemoveAll(dir) | ||
|
||
_, err = NewExerciseConfig(dir) | ||
assert.Error(t, err) | ||
// any assertions about this error message have to work across all platforms, so be vague | ||
// unix: ".exercism/config.json: no such file or directory" | ||
// windows: "open .exercism\config.json: The system cannot find the path specified." | ||
assert.Contains(t, err.Error(), filepath.Join(".exercism", "config.json:")) | ||
} | ||
|
||
func TestInvalidExerciseConfig(t *testing.T) { | ||
dir, err := ioutil.TempDir("", "exercise_config") | ||
assert.NoError(t, err) | ||
defer os.RemoveAll(dir) | ||
|
||
err = os.Mkdir(filepath.Join(dir, ".exercism"), os.ModePerm) | ||
assert.NoError(t, err) | ||
|
||
f, err := os.Create(filepath.Join(dir, ".exercism", "config.json")) | ||
assert.NoError(t, err) | ||
defer f.Close() | ||
|
||
// invalid JSON | ||
_, err = f.WriteString(`{ "blurb": "Learn about the basics of Ruby by following a lasagna recipe.", "authors": ["iHiD", "pvcarr `) | ||
assert.NoError(t, err) | ||
|
||
_, err = NewExerciseConfig(dir) | ||
assert.Error(t, err) | ||
assert.True(t, strings.Contains(err.Error(), "unexpected end of JSON input")) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI was complaining about this block, so I ran
go fmt ./...
and this was the change. I realize you want to keep the PR focused, but I also assume you want to keep the CI green