pkg/rulefmt: fix bug of validate.

Signed-off-by: johncming <johncming@yahoo.com>

model/rulefmt/rulefmt.go

@@ -173,17 +173,11 @@ func (r *RuleNode) Validate() (nodes []WrappedError) {
 		})
 	}
 	if r.Record.Value == "" && r.Alert.Value == "" {
-		if r.Record.Value == "0" {
+		nodes = append(nodes, WrappedError{
-			nodes = append(nodes, WrappedError{
+			err:     fmt.Errorf("one of 'record' or 'alert' must be set"),
-				err:  fmt.Errorf("one of 'record' or 'alert' must be set"),
+			node:    &r.Record,
-				node: &r.Alert,
+			nodeAlt: &r.Alert,
-			})
+		})
-		} else {
-			nodes = append(nodes, WrappedError{
-				err:  fmt.Errorf("one of 'record' or 'alert' must be set"),
-				node: &r.Record,
-			})
-		}
 	}
 
 	if r.Expr.Value == "" {
@@ -250,7 +244,7 @@ func (r *RuleNode) Validate() (nodes []WrappedError) {
 		nodes = append(nodes, WrappedError{err: err})
 	}
 
-	return
+	return nodes
 }
 
 // testTemplateParsing checks if the templates used in labels and annotations

model/rulefmt/testdata/record_and_alert.both_set.bad.yaml

@@ -0,0 +1,6 @@
+groups:
+- name: yolo
+  rules:
+  - record: Hi
+    alert: Hello
+    expr: 1

model/rulefmt/testdata/record_and_alert.both_unset.bad.yaml

@@ -0,0 +1,4 @@
+groups:
+  - name: yolo
+    rules:
+      - expr: 1

pkg/rulefmt/rulefmt_test.go

@@ -0,0 +1,175 @@
+// Copyright 2017 The Prometheus Authors
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package rulefmt
+
+import (
+	"path/filepath"
+	"strings"
+	"testing"
+
+	"github.com/prometheus/prometheus/util/testutil"
+)
+
+func TestParseFileSuccess(t *testing.T) {
+	if _, errs := ParseFile("testdata/test.yaml"); len(errs) > 0 {
+		t.Errorf("unexpected errors parsing file")
+		for _, err := range errs {
+			t.Error(err)
+		}
+	}
+}
+
+func TestParseFileFailure(t *testing.T) {
+	table := []struct {
+		filename string
+		errMsg   string
+	}{
+		{
+			filename: "duplicate_grp.bad.yaml",
+			errMsg:   "groupname: \"yolo\" is repeated in the same file",
+		},
+		{
+			filename: "bad_expr.bad.yaml",
+			errMsg:   "parse error",
+		},
+		{
+			filename: "record_and_alert.both_set.bad.yaml",
+			errMsg:   "'alert' and 'record' cannot be set at the same time",
+		},
+		{
+			filename: "record_and_alert.both_unset.bad.yaml",
+			errMsg:   "one of 'record' or 'alert' must be set",
+		},
+		{
+			filename: "no_rec_alert.bad.yaml",
+			errMsg:   "one of 'record' or 'alert' must be set",
+		},
+		{
+			filename: "noexpr.bad.yaml",
+			errMsg:   "field 'expr' must be set in rule",
+		},
+		{
+			filename: "bad_lname.bad.yaml",
+			errMsg:   "invalid label name",
+		},
+		{
+			filename: "bad_annotation.bad.yaml",
+			errMsg:   "invalid annotation name",
+		},
+		{
+			filename: "invalid_record_name.bad.yaml",
+			errMsg:   "invalid recording rule name",
+		},
+		{
+			filename: "bad_field.bad.yaml",
+			errMsg:   "field annotation not found",
+		},
+		{
+			filename: "invalid_label_name.bad.yaml",
+			errMsg:   "invalid label name",
+		},
+	}
+
+	for _, c := range table {
+		_, errs := ParseFile(filepath.Join("testdata", c.filename))
+		if errs == nil {
+			t.Errorf("Expected error parsing %s but got none", c.filename)
+			continue
+		}
+		if !strings.Contains(errs[0].Error(), c.errMsg) {
+			t.Errorf("Expected error for %s to contain %q but got: %s", c.filename, c.errMsg, errs)
+		}
+	}
+
+}
+
+func TestTemplateParsing(t *testing.T) {
+	tests := []struct {
+		ruleString string
+		shouldPass bool
+	}{
+		{
+			ruleString: `
+groups:
+- name: example
+  rules:
+  - alert: InstanceDown
+    expr: up == 0
+    for: 5m
+    labels:
+      severity: "page"
+    annotations:
+      summary: "Instance {{ $labels.instance }} down"
+`,
+			shouldPass: true,
+		},
+		{
+			// `$label` instead of `$labels`.
+			ruleString: `
+groups:
+- name: example
+  rules:
+  - alert: InstanceDown
+    expr: up == 0
+    for: 5m
+    labels:
+      severity: "page"
+    annotations:
+      summary: "Instance {{ $label.instance }} down"
+`,
+			shouldPass: false,
+		},
+		{
+			// `$this_is_wrong`.
+			ruleString: `
+groups:
+- name: example
+  rules:
+  - alert: InstanceDown
+    expr: up == 0
+    for: 5m
+    labels:
+      severity: "{{$this_is_wrong}}"
+    annotations:
+      summary: "Instance {{ $labels.instance }} down"
+`,
+			shouldPass: false,
+		},
+		{
+			// `$labels.quantile * 100`.
+			ruleString: `
+groups:
+- name: example
+  rules:
+  - alert: InstanceDown
+    expr: up == 0
+    for: 5m
+    labels:
+      severity: "page"
+    annotations:
+      summary: "Instance {{ $labels.instance }} down"
+      description: "{{$labels.quantile * 100}}"
+`,
+			shouldPass: false,
+		},
+	}
+
+	for _, tst := range tests {
+		rgs, errs := Parse([]byte(tst.ruleString))
+		testutil.Assert(t, rgs != nil, "Rule parsing, rule=\n"+tst.ruleString)
+		passed := (tst.shouldPass && len(errs) == 0) || (!tst.shouldPass && len(errs) > 0)
+		testutil.Assert(t, passed, "Rule validation failed, rule=\n"+tst.ruleString)
+	}
+
+}