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 open-policy-agent#954 Signed-off-by: Torin Sandall <[email protected]>
tsandall · Sep 17, 2018 · bd8f346 · bd8f346
1 parent 6a7c0b3
commit bd8f346
Show file tree

Hide file tree

Showing 5 changed files with 14 additions and 20 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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])
 			}
 		}