From 5d68ebb2076f2776e5c553b56f92b57e99a8fe61 Mon Sep 17 00:00:00 2001 From: johncming Date: Sat, 21 Mar 2020 21:34:00 +0800 Subject: [PATCH 1/2] pkg/rulefmt: fix bug of validate. Signed-off-by: johncming --- model/rulefmt/rulefmt.go | 18 +- .../record_and_alert.both_set.bad.yaml | 6 + .../record_and_alert.both_unset.bad.yaml | 4 + pkg/rulefmt/rulefmt_test.go | 175 ++++++++++++++++++ 4 files changed, 191 insertions(+), 12 deletions(-) create mode 100644 model/rulefmt/testdata/record_and_alert.both_set.bad.yaml create mode 100644 model/rulefmt/testdata/record_and_alert.both_unset.bad.yaml create mode 100644 pkg/rulefmt/rulefmt_test.go diff --git a/model/rulefmt/rulefmt.go b/model/rulefmt/rulefmt.go index 30b3face0d..6be93f6428 100644 --- a/model/rulefmt/rulefmt.go +++ b/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{ - err: fmt.Errorf("one of 'record' or 'alert' must be set"), - node: &r.Alert, - }) - } else { - nodes = append(nodes, WrappedError{ - err: fmt.Errorf("one of 'record' or 'alert' must be set"), - node: &r.Record, - }) - } + nodes = append(nodes, WrappedError{ + err: fmt.Errorf("one of 'record' or 'alert' must be set"), + node: &r.Record, + nodeAlt: &r.Alert, + }) } 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 diff --git a/model/rulefmt/testdata/record_and_alert.both_set.bad.yaml b/model/rulefmt/testdata/record_and_alert.both_set.bad.yaml new file mode 100644 index 0000000000..0ba81b7423 --- /dev/null +++ b/model/rulefmt/testdata/record_and_alert.both_set.bad.yaml @@ -0,0 +1,6 @@ +groups: +- name: yolo + rules: + - record: Hi + alert: Hello + expr: 1 diff --git a/model/rulefmt/testdata/record_and_alert.both_unset.bad.yaml b/model/rulefmt/testdata/record_and_alert.both_unset.bad.yaml new file mode 100644 index 0000000000..64d2e8f20d --- /dev/null +++ b/model/rulefmt/testdata/record_and_alert.both_unset.bad.yaml @@ -0,0 +1,4 @@ +groups: + - name: yolo + rules: + - expr: 1 diff --git a/pkg/rulefmt/rulefmt_test.go b/pkg/rulefmt/rulefmt_test.go new file mode 100644 index 0000000000..58b5d5ad36 --- /dev/null +++ b/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) + } + +} From c52db2b196238e376dba6d580d95373ba794a226 Mon Sep 17 00:00:00 2001 From: Julien Pivotto Date: Fri, 29 Sep 2023 10:38:10 +0200 Subject: [PATCH 2/2] Remove duplicate tests Signed-off-by: Julien Pivotto --- model/rulefmt/rulefmt.go | 2 +- .../record_and_alert.both_set.bad.yaml | 6 - .../record_and_alert.both_unset.bad.yaml | 4 - pkg/rulefmt/rulefmt_test.go | 175 ------------------ 4 files changed, 1 insertion(+), 186 deletions(-) delete mode 100644 model/rulefmt/testdata/record_and_alert.both_set.bad.yaml delete mode 100644 model/rulefmt/testdata/record_and_alert.both_unset.bad.yaml delete mode 100644 pkg/rulefmt/rulefmt_test.go diff --git a/model/rulefmt/rulefmt.go b/model/rulefmt/rulefmt.go index 6be93f6428..03cbd8849c 100644 --- a/model/rulefmt/rulefmt.go +++ b/model/rulefmt/rulefmt.go @@ -244,7 +244,7 @@ func (r *RuleNode) Validate() (nodes []WrappedError) { nodes = append(nodes, WrappedError{err: err}) } - return nodes + return } // testTemplateParsing checks if the templates used in labels and annotations diff --git a/model/rulefmt/testdata/record_and_alert.both_set.bad.yaml b/model/rulefmt/testdata/record_and_alert.both_set.bad.yaml deleted file mode 100644 index 0ba81b7423..0000000000 --- a/model/rulefmt/testdata/record_and_alert.both_set.bad.yaml +++ /dev/null @@ -1,6 +0,0 @@ -groups: -- name: yolo - rules: - - record: Hi - alert: Hello - expr: 1 diff --git a/model/rulefmt/testdata/record_and_alert.both_unset.bad.yaml b/model/rulefmt/testdata/record_and_alert.both_unset.bad.yaml deleted file mode 100644 index 64d2e8f20d..0000000000 --- a/model/rulefmt/testdata/record_and_alert.both_unset.bad.yaml +++ /dev/null @@ -1,4 +0,0 @@ -groups: - - name: yolo - rules: - - expr: 1 diff --git a/pkg/rulefmt/rulefmt_test.go b/pkg/rulefmt/rulefmt_test.go deleted file mode 100644 index 58b5d5ad36..0000000000 --- a/pkg/rulefmt/rulefmt_test.go +++ /dev/null @@ -1,175 +0,0 @@ -// 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) - } - -}