From 8b65cb5f18f6bf47e8a81bd39cc0685ff64018bb Mon Sep 17 00:00:00 2001
From: Torin Sandall <torinsandall@gmail.com>
Date: Mon, 17 Sep 2018 10:08:02 -0700
Subject: [PATCH] Update test runner to set Fail to true

Previously the test runner would set fail to the value generated by the
test rule or false on undefined. The intent was to communicate the value
generated by the rule. In practice users are not writing tests that
generate values other than true so this is essentially unnecessary.

Fixes #954

Signed-off-by: Torin Sandall <torinsandall@gmail.com>
---
 docs/book/how-do-i-test-policies.md |  8 ++++----
 tester/reporter.go                  |  2 +-
 tester/reporter_test.go             |  8 +++-----
 tester/runner.go                    | 14 +++++---------
 tester/runner_test.go               |  2 +-
 5 files changed, 14 insertions(+), 20 deletions(-)

diff --git a/docs/book/how-do-i-test-policies.md b/docs/book/how-do-i-test-policies.md
index 8808564626..4215bdd7e9 100644
--- a/docs/book/how-do-i-test-policies.md
+++ b/docs/book/how-do-i-test-policies.md
@@ -169,7 +169,7 @@ $ opa test --format=json pass_fail_error_test.rego
     },
     "package": "data.example",
     "name": "test_ok",
-    "duration": 610111
+    "duration": 618515
   },
   {
     "location": {
@@ -179,8 +179,8 @@ $ opa test --format=json pass_fail_error_test.rego
     },
     "package": "data.example",
     "name": "test_failure",
-    "fail": false,
-    "duration": 325989
+    "fail": true,
+    "duration": 322177
   },
   {
     "location": {
@@ -199,7 +199,7 @@ $ opa test --format=json pass_fail_error_test.rego
         "col": 5
       }
     },
-    "duration": 325903
+    "duration": 345148
   }
 ]
 ```
diff --git a/tester/reporter.go b/tester/reporter.go
index 0b7601b9ed..86136ebd2f 100644
--- a/tester/reporter.go
+++ b/tester/reporter.go
@@ -40,7 +40,7 @@ func (r PrettyReporter) Report(ch chan *Result) error {
 			pass++
 		} else if tr.Error != nil {
 			errs++
-		} else if tr.Fail != nil {
+		} else if tr.Fail {
 			fail++
 		}
 		if !tr.Pass() || r.Verbose {
diff --git a/tester/reporter_test.go b/tester/reporter_test.go
index 596803278a..ee2b12c129 100644
--- a/tester/reporter_test.go
+++ b/tester/reporter_test.go
@@ -8,12 +8,10 @@ import (
 
 func TestPrettyReporter(t *testing.T) {
 
-	var badResult interface{} = "fail"
-
 	ts := []*Result{
-		{nil, "data.foo.bar", "test_baz", nil, nil, 0},
-		{nil, "data.foo.bar", "test_qux", nil, fmt.Errorf("some err"), 0},
-		{nil, "data.foo.bar", "test_corge", &badResult, nil, 0},
+		{nil, "data.foo.bar", "test_baz", false, nil, 0},
+		{nil, "data.foo.bar", "test_qux", false, fmt.Errorf("some err"), 0},
+		{nil, "data.foo.bar", "test_corge", true, nil, 0},
 	}
 
 	var buf bytes.Buffer
diff --git a/tester/runner.go b/tester/runner.go
index 5cb37a337a..31a9d2bf0a 100644
--- a/tester/runner.go
+++ b/tester/runner.go
@@ -51,7 +51,7 @@ type Result struct {
 	Location *ast.Location `json:"location"`
 	Package  string        `json:"package"`
 	Name     string        `json:"name"`
-	Fail     *interface{}  `json:"fail,omitempty"`
+	Fail     bool          `json:"fail,omitempty"`
 	Error    error         `json:"error,omitempty"`
 	Duration time.Duration `json:"duration"`
 }
@@ -67,7 +67,7 @@ func newResult(loc *ast.Location, pkg, name string, duration time.Duration) *Res
 
 // Pass returns true if the test case passed.
 func (r Result) Pass() bool {
-	return r.Fail == nil && r.Error == nil
+	return !r.Fail && r.Error == nil
 }
 
 func (r *Result) String() string {
@@ -78,16 +78,12 @@ func (r *Result) outcome() string {
 	if r.Pass() {
 		return "PASS"
 	}
-	if r.Fail != nil {
+	if r.Fail {
 		return "FAIL"
 	}
 	return "ERROR"
 }
 
-func (r *Result) setFail(fail interface{}) {
-	r.Fail = &fail
-}
-
 // Runner implements simple test discovery and execution.
 type Runner struct {
 	compiler *ast.Compiler
@@ -184,9 +180,9 @@ func (r *Runner) runTest(ctx context.Context, mod *ast.Module, rule *ast.Rule) (
 			stop = true
 		}
 	} else if len(rs) == 0 {
-		tr.setFail(false)
+		tr.Fail = true
 	} else if b, ok := rs[0].Expressions[0].Value.(bool); !ok || !b {
-		tr.setFail(rs[0].Expressions[0].Value)
+		tr.Fail = true
 	}
 
 	return tr, stop
diff --git a/tester/runner_test.go b/tester/runner_test.go
index 88d93b140d..1321b603e8 100644
--- a/tester/runner_test.go
+++ b/tester/runner_test.go
@@ -56,7 +56,7 @@ func TestRun(t *testing.T) {
 			exp, ok := tests[k]
 			if !ok {
 				t.Errorf("Unexpected result for %v", k)
-			} else if exp.wantErr != (rs[i].Error != nil) || exp.wantFail != (rs[i].Fail != nil) {
+			} else if exp.wantErr != (rs[i].Error != nil) || exp.wantFail != rs[i].Fail {
 				t.Errorf("Expected %v for %v but got: %v", exp, k, rs[i])
 			}
 		}