From 0fc801021774623149758331428c6fe3702c20ab Mon Sep 17 00:00:00 2001 From: June <12543047+junedev@users.noreply.github.com> Date: Tue, 22 Mar 2022 18:34:47 +0100 Subject: [PATCH] process testing flags from .meta/config.json (#70) --- Dockerfile | 3 + README.md | 23 ++++++- testrunner/execute.go | 60 +++++++++++++++++- testrunner/execute_test.go | 17 +++++ .../practice/passing/.meta/config.json | 20 ++++++ .../testdata/practice/race/.meta/config.json | 25 ++++++++ .../testdata/practice/race/.meta/example.go | 55 ++++++++++++++++ .../testdata/practice/race/bank_account.go | 54 ++++++++++++++++ .../practice/race/bank_account_test.go | 62 +++++++++++++++++++ testrunner/testdata/practice/race/go.mod | 3 + 10 files changed, 319 insertions(+), 3 deletions(-) create mode 100644 testrunner/testdata/practice/passing/.meta/config.json create mode 100644 testrunner/testdata/practice/race/.meta/config.json create mode 100644 testrunner/testdata/practice/race/.meta/example.go create mode 100644 testrunner/testdata/practice/race/bank_account.go create mode 100644 testrunner/testdata/practice/race/bank_account_test.go create mode 100644 testrunner/testdata/practice/race/go.mod diff --git a/Dockerfile b/Dockerfile index d82a82f..5a4dba5 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,5 +1,8 @@ FROM golang:1.17-alpine3.14 +# add addtional packages needed for the race detector to work +RUN apk add --update build-base make + # add a non-root user to run our code as RUN adduser --disabled-password --gecos "" appuser diff --git a/README.md b/README.md index b16459e..06af2e8 100644 --- a/README.md +++ b/README.md @@ -51,13 +51,13 @@ docker build -t exercism/go-test-runner . Run the test runner in the container by passing in the slug name, and absolute paths to the exercise (solution) and a writeable tmp directory. These directories should be mounted as volumes: ```bash -docker run --network none --read-only -v $(pwd)/testdata/practice/gigasecond:/solution -v /tmp:/tmp exercism/go-test-runner gigasecond /solution /tmp +docker run --network none --read-only -v $(pwd)/testrunner/testdata/practice/gigasecond:/solution -v /tmp:/tmp exercism/go-test-runner gigasecond /solution /tmp ``` For troubleshooting / debug you can name the container, run it in interactive mode, and detach from it using: ```bash -docker run --name exercism-go-test-runner -d -i --network none --read-only -v $(pwd)/testdata/practice/gigasecond:/solution -v /tmp:/tmp exercism/go-test-runner gigasecond /solution /tmp +docker run --name exercism-go-test-runner -d -i --network none --read-only -v $(pwd)/testrunner/testdata/practice/gigasecond:/solution -v /tmp:/tmp exercism/go-test-runner gigasecond /solution /tmp # You can then access the container as follows: docker exec -it --user appuser $(docker ps -q --filter name=exercism-go-test-runner) /bin/sh ``` @@ -136,3 +136,22 @@ if got := ParseCard(tt.card); got != tt.want { t.Errorf("ParseCard(%s) = %d, want %d", tt.card, got, tt.want) } ``` + +## Providing Additional Testing Flags + +Exercises can supply additional flags that will be included when the test runner executes the `go test` command. +This is done via the `.meta/config.json` file of the exercise. See example below. + +```json +{ + // ... + "custom": { + "testingFlags": [ + "-race" + ] + } +} +``` + +Currently, only the flag `-race` is supported. +If more flags should be allowed in the future, they first need to be added to the `allowedTestingFlags` list in `testrunner/execute.go`. \ No newline at end of file diff --git a/testrunner/execute.go b/testrunner/execute.go index e810dd0..584b4b1 100644 --- a/testrunner/execute.go +++ b/testrunner/execute.go @@ -7,7 +7,9 @@ import ( "errors" "fmt" "log" + "os" "os/exec" + "path/filepath" "time" ) @@ -18,6 +20,9 @@ const ( statErr = "error" ) +// For security reasons, only testing flags that are included in the list below are processed. +var allowedTestingFlags = []string{"-race"} + type testResult struct { Name string `json:"name"` Status string `json:"status"` @@ -272,11 +277,16 @@ func runTests(input_dir string) (bytes.Buffer, bool) { log.Fatal("failed to find go executable: ", err) } + additionalTestFlags := findAdditionalTestFlags(input_dir) + testCommand := []string{goExe, "test", "--short", "--json"} + testCommand = append(testCommand, additionalTestFlags...) + testCommand = append(testCommand, ".") + var stdout, stderr bytes.Buffer testCmd := &exec.Cmd{ Dir: input_dir, Path: goExe, - Args: []string{goExe, "test", "--short", "--json", "."}, + Args: testCommand, Stdout: &stdout, Stderr: &stderr, } @@ -317,3 +327,51 @@ func runTests(input_dir string) (bytes.Buffer, bool) { } return stdout, false } + +type config struct { + Custom struct { + TestingFlags []string `json:"testingFlags"` + } `json:"custom"` +} + +func findAdditionalTestFlags(input_dir string) []string { + configContent, err := os.ReadFile(filepath.Join(input_dir, ".meta", "config.json")) + if err != nil { + log.Printf("failed to read config.json: %v", err) + return nil + } + + cfg := &config{} + err = json.Unmarshal(configContent, cfg) + if err != nil { + log.Printf("failed to parse config.json: %v", err) + return nil + } + + if len(cfg.Custom.TestingFlags) == 0 { + return nil + } + + return validateTestingFlags(cfg.Custom.TestingFlags) +} + +func validateTestingFlags(flags []string) []string { + validFlags := []string{} + for _, flag := range flags { + if contains(allowedTestingFlags, flag) { + validFlags = append(validFlags, flag) + } else { + log.Printf("invalid testing flag found in config.json: %s", flag) + } + } + return validFlags +} + +func contains(list []string, target string) bool { + for _, item := range list { + if item == target { + return true + } + } + return false +} diff --git a/testrunner/execute_test.go b/testrunner/execute_test.go index 94def1e..7a82aa7 100644 --- a/testrunner/execute_test.go +++ b/testrunner/execute_test.go @@ -116,6 +116,23 @@ func TestRunTests_RuntimeError(t *testing.T) { } } +func TestRunTests_RaceDetector(t *testing.T) { + input_dir := "./testdata/practice/race" + cmdres, ok := runTests(input_dir) + if !ok { + fmt.Printf("race detector test expected to return ok: %s", cmdres.String()) + } + + output := getStructure(cmdres, input_dir, version) + if output.Status != "fail" { + t.Errorf("wrong status for race detector test: got %q, want %q", output.Status, "fail") + } + + if !strings.Contains(output.Tests[0].Message, "WARNING: DATA RACE") { + t.Errorf("no data race error included in message: %s", output.Tests[0].Message) + } +} + func TestRunTests_passing(t *testing.T) { input_dir := "./testdata/practice/passing" diff --git a/testrunner/testdata/practice/passing/.meta/config.json b/testrunner/testdata/practice/passing/.meta/config.json new file mode 100644 index 0000000..070b923 --- /dev/null +++ b/testrunner/testdata/practice/passing/.meta/config.json @@ -0,0 +1,20 @@ +{ + "blurb": "...", + "authors": [ + "..." + ], + "contributors": [ + "..." + ], + "files": { + "solution": [ + "passing.go" + ], + "test": [ + "passing_test.go" + ], + "example": [ + ".meta/example.go" + ] + } +} diff --git a/testrunner/testdata/practice/race/.meta/config.json b/testrunner/testdata/practice/race/.meta/config.json new file mode 100644 index 0000000..bc57556 --- /dev/null +++ b/testrunner/testdata/practice/race/.meta/config.json @@ -0,0 +1,25 @@ +{ + "blurb": "Simulate a bank account supporting opening/closing, withdraws, and deposits of money. Watch out for concurrent transactions!", + "authors": [ + "soniakeys" + ], + "contributors": [ + "..." + ], + "files": { + "solution": [ + "bank_account.go" + ], + "test": [ + "bank_account_test.go" + ], + "example": [ + ".meta/example.go" + ] + }, + "custom": { + "testingFlags": [ + "-race" + ] + } +} diff --git a/testrunner/testdata/practice/race/.meta/example.go b/testrunner/testdata/practice/race/.meta/example.go new file mode 100644 index 0000000..f3e71d6 --- /dev/null +++ b/testrunner/testdata/practice/race/.meta/example.go @@ -0,0 +1,55 @@ +package account + +import "sync" + +type Account struct { + mu *sync.RWMutex + open bool + balance int64 +} + +func Open(amt int64) *Account { + if amt < 0 { + return nil + } + return &Account{ + open: true, + balance: amt, + mu: new(sync.RWMutex), + } +} + +func (a *Account) Balance() (bal int64, ok bool) { + a.mu.RLock() + bal, ok = a.balance, a.open + a.mu.RUnlock() + return +} + +func (a *Account) Deposit(amt int64) (newBal int64, ok bool) { + a.mu.Lock() + defer a.mu.Unlock() + if !a.open { + return a.balance, false + } + if amt < 0 && a.balance+amt < 0 { + return a.balance, false + } + a.balance += amt + return a.balance, true +} + +func (a *Account) Close() (pay int64, ok bool) { + a.mu.Lock() + defer a.mu.Unlock() + if !a.open { + return 0, false + } + a.open = false + if a.balance < 0 { + return 0, true + } + pay = a.balance + a.balance = 0 + return pay, true +} diff --git a/testrunner/testdata/practice/race/bank_account.go b/testrunner/testdata/practice/race/bank_account.go new file mode 100644 index 0000000..6867536 --- /dev/null +++ b/testrunner/testdata/practice/race/bank_account.go @@ -0,0 +1,54 @@ +package account + +import "sync" + +type Account struct { + mu *sync.RWMutex + open bool + balance int64 +} + +func Open(amt int64) *Account { + if amt < 0 { + return nil + } + return &Account{ + open: true, + balance: amt, + mu: new(sync.RWMutex), + } +} + +func (a *Account) Balance() (bal int64, ok bool) { + // Locking missing here. + bal, ok = a.balance, a.open + return +} + +func (a *Account) Deposit(amt int64) (newBal int64, ok bool) { + a.mu.Lock() + defer a.mu.Unlock() + if !a.open { + return a.balance, false + } + if amt < 0 && a.balance+amt < 0 { + return a.balance, false + } + a.balance += amt + return a.balance, true +} + +func (a *Account) Close() (pay int64, ok bool) { + a.mu.Lock() + defer a.mu.Unlock() + if !a.open { + return 0, false + } + a.open = false + if a.balance < 0 { + return 0, true + } + pay = a.balance + a.balance = 0 + return pay, true +} diff --git a/testrunner/testdata/practice/race/bank_account_test.go b/testrunner/testdata/practice/race/bank_account_test.go new file mode 100644 index 0000000..dc10382 --- /dev/null +++ b/testrunner/testdata/practice/race/bank_account_test.go @@ -0,0 +1,62 @@ +package account + +import ( + "runtime" + "sync" + "sync/atomic" + "testing" + "time" +) + +func TestConcDeposit(t *testing.T) { + if runtime.NumCPU() < 2 { + t.Skip("Multiple CPU cores required for concurrency tests.") + } + if runtime.GOMAXPROCS(0) < 2 { + runtime.GOMAXPROCS(2) + } + a := Open(0) + if a == nil { + t.Fatal("Open(0) = nil, want non-nil *Account.") + } + const amt = 10 + const c = 1000 + var negBal int32 + var start, g sync.WaitGroup + start.Add(1) + g.Add(3 * c) + for i := 0; i < c; i++ { + go func() { // deposit + start.Wait() + a.Deposit(amt) // ignore return values + g.Done() + }() + go func() { // withdraw + start.Wait() + for { + if _, ok := a.Deposit(-amt); ok { + break + } + time.Sleep(time.Microsecond) // retry + } + g.Done() + }() + go func() { // watch that balance stays >= 0 + start.Wait() + if p, _ := a.Balance(); p < 0 { + atomic.StoreInt32(&negBal, 1) + } + g.Done() + }() + } + start.Done() + g.Wait() + if negBal == 1 { + t.Fatal("Balance went negative with concurrent deposits and " + + "withdrawals. Want balance always >= 0.") + } + if p, ok := a.Balance(); !ok || p != 0 { + t.Fatalf("After equal concurrent deposits and withdrawals, "+ + "a.Balance = %d, %t. Want 0, true", p, ok) + } +} diff --git a/testrunner/testdata/practice/race/go.mod b/testrunner/testdata/practice/race/go.mod new file mode 100644 index 0000000..59fce13 --- /dev/null +++ b/testrunner/testdata/practice/race/go.mod @@ -0,0 +1,3 @@ +module account + +go 1.16