process testing flags from .meta/config.json (#70)

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
 

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`.

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
+}

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"
 

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"
+    ]
+  }
+}

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"
+    ]
+  }
+}

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
+}

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
+}

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)
+	}
+}

testrunner/testdata/practice/race/go.mod

@@ -0,0 +1,3 @@
+module account
+
+go 1.16