Skip to content

Commit e673dbc

Browse files
committed
Support and test regexp error patterns; match error class name; document in Good Specs
Enable !ruby/regexp in spec YAML for flexible error-pattern matching: - spec_loader: allow Regexp in safe_yaml_load permitted classes; relax the !ruby/ tag guard to reject only non-regexp tags (!ruby/object etc.) so !ruby/regexp passes through - A string pattern stays a case-insensitive literal substring; a Regexp pattern is used as-is (alternation, anchors, metacharacters work). Note: backslash escapes must be doubled in YAML (write \d). runner: check_error_patterns now also matches each pattern against the exception's class name (e.g. NoMethodError, Liquid::ArgumentError), so specs can assert the kind of error, not just its message. Tests (test/error_pattern_test.rb, 10 cases; +2 in lazy_spec_test.rb): - !ruby/regexp loads and matches; non-matching regexp fails - mixed regexp + string patterns (all must match) - multiple substring patterns; one missing fails - case-insensitive substring matching - error class name matching (pass and fail) - !ruby/object still rejected, !ruby/regexp allowed - error_patterns passes Regexp through unchanged (unit) AGENTS.md Good Specs: document string vs Regexp patterns, class-name matching, the backslash-doubling caveat, with worked examples.
1 parent ab346f3 commit e673dbc

4 files changed

Lines changed: 261 additions & 35 deletions

File tree

CLAUDE.md

Lines changed: 27 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -267,15 +267,17 @@ Two raised-error forms exist:
267267

268268
### Matching error patterns
269269

270-
Each entry under `parse_error:` / `render_error:` is a **case-insensitive,
271-
literal substring** matched against:
270+
Each entry under `parse_error:` / `render_error:` is matched against:
272271

273272
1. The full exception message
274273
2. The "core" message (text after the last `): ` or `: `)
275274
3. The exception **class name** (e.g. `NoMethodError`, `Liquid::ArgumentError`)
276275

277-
**All** listed patterns must match for the spec to pass. Use multiple
278-
patterns to pin down the error precisely without coupling to exact wording:
276+
A **string** pattern is a case-insensitive, literal substring (special
277+
characters are escaped). A **Regexp** pattern (`!ruby/regexp /.../i`) is
278+
used as-is, so metacharacters like `|`, `.+`, and anchors work. **All**
279+
listed patterns must match for the spec to pass. Use multiple patterns to
280+
pin down the error precisely without coupling to exact wording:
279281

280282
```yaml
281283
- name: sec_wide_open_object_name_raises_no_to_liquid
@@ -309,6 +311,18 @@ patterns to pin down the error precisely without coupling to exact wording:
309311
error_mode: lax
310312
```
311313

314+
A **Regexp** pattern (used as-is, unlike string patterns which are literal
315+
substrings) — useful for alternation without doubling up specs:
316+
317+
```yaml
318+
- name: regexp_render_error_match
319+
template: "{{ 10 | divided_by: 0 }}"
320+
errors:
321+
render_error:
322+
- !ruby/regexp /ZeroDivision|divided by/i
323+
complexity: 200
324+
```
325+
312326
**Best practices for error patterns:**
313327

314328
- **Match the error class name** (`NoMethodError`, `Liquid::SyntaxError`, …)
@@ -319,32 +333,15 @@ patterns to pin down the error precisely without coupling to exact wording:
319333
- **Match location info when relevant** — partial name and `line N` — but
320334
keep those as separate patterns so an implementation that lacks one of
321335
them fails clearly on that specific pattern.
322-
- **Never set `render_errors: true`** for a raised-error spec. That flag
323-
switches the engine to inline-error mode, which is a different (and rarer)
324-
behaviour tested with `errors: output:` + `features: [inline_errors]`.
325-
326-
### Inline error specs (rare)
327-
328-
Only use the inline form when you are specifically testing that an error is
329-
*rendered into the output* rather than raised. These specs require
330-
`render_errors: true` and `features: [inline_errors]`:
331-
332-
```yaml
333-
- name: render_prohibits_include_inline
334-
template: "{% render 'outer' %}"
335-
filesystem:
336-
outer: "{% include 'inner' %}"
337-
inner: should not render
338-
errors:
339-
output:
340-
- include usage is not allowed in this context
341-
render_errors: true
342-
features: [inline_errors]
343-
complexity: 220
344-
```
345-
346-
This is the exception, not the rule. When in doubt, let the error raise and
347-
use `errors: render_error:` / `errors: parse_error:`.
336+
- **Use a Regexp (`!ruby/regexp`)** when you need alternation or anchors
337+
rather than a literal substring. Note: backslash escapes inside the YAML
338+
regexp literal must be doubled (write `\\d`, not `\d`), because YAML
339+
processes the scalar before the regexp is compiled. Prefer backslash-free
340+
patterns where possible (e.g. `!ruby/regexp /ZeroDivision|divided by/i`).
341+
- **Never set `render_errors: true`** for new specs. New specs should always
342+
let errors raise and match them with `errors: parse_error:` /
343+
`errors: render_error:`. The inline form (`render_errors: true` with
344+
`errors: output:`) is legacy and should not be added to new specs.
348345

349346
## Suite Configuration
350347

lib/liquid/spec/spec_loader.rb

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,11 @@ def clear!
4545
end
4646

4747
# Shared YAML loading with permitted classes for specs
48+
# Regexp is permitted so error patterns can use !ruby/regexp /.../i for
49+
# flexible matching (string patterns are matched case-insensitively as
50+
# literal substrings; Regexp patterns are used as-is).
4851
def self.safe_yaml_load(content)
49-
YAML.safe_load(content, permitted_classes: [Symbol, Date, Time, Range], aliases: true)
52+
YAML.safe_load(content, permitted_classes: [Symbol, Date, Time, Range, Regexp], aliases: true)
5053
end
5154

5255
# Loads specs from YAML files without instantiating drop objects
@@ -124,12 +127,13 @@ def load_yaml_file(path, suite: nil)
124127
content = File.read(path)
125128
specs = []
126129

127-
# Fail fast if file contains !ruby/ tags - these must be converted to instantiate format
128-
if content.include?("!ruby/")
129-
# Find the line numbers with !ruby/ tags
130+
# Fail fast if file contains dangerous !ruby/ tags (e.g. !ruby/object)
131+
# which must be converted to instantiate format. !ruby/regexp is allowed
132+
# for flexible error-pattern matching.
133+
if content.match?(/!ruby\/(?!regexp\b)/)
130134
bad_lines = []
131135
content.each_line.with_index do |line, idx|
132-
bad_lines << (idx + 1) if line.include?("!ruby/")
136+
bad_lines << (idx + 1) if line.match?(/!ruby\/(?!regexp\b)/)
133137
end
134138
raise "YAML file contains !ruby/ tags which are not allowed. " \
135139
"Convert to instantiate format.\n" \

test/error_pattern_test.rb

Lines changed: 202 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,202 @@
1+
# frozen_string_literal: true
2+
3+
require_relative "test_helper"
4+
require "liquid"
5+
require "tmpdir"
6+
require "fileutils"
7+
8+
# Integration tests for error-pattern matching in spec YAML:
9+
# * multiple patterns (all must match)
10+
# * case-insensitive literal substring matching (string patterns)
11+
# * Regexp patterns (!ruby/regexp) used as-is, with metacharacters
12+
# * error class name matching
13+
# * !ruby/regexp allowed through the loader, !ruby/object still rejected
14+
class ErrorPatternTest < Minitest::Test
15+
def with_temp_spec(content)
16+
Dir.mktmpdir do |dir|
17+
path = File.join(dir, "spec.yml")
18+
File.write(path, content)
19+
yield path
20+
end
21+
end
22+
23+
def test_loader_allows_ruby_regexp_tag
24+
with_temp_spec(<<~YAML) do |path|
25+
---
26+
- name: t
27+
template: "{{ 10 | divided_by: 0 }}"
28+
errors:
29+
render_error:
30+
- !ruby/regexp /ZeroDivision|divided by/i
31+
YAML
32+
specs = Liquid::Spec::SpecLoader.load_yaml_file(path)
33+
assert_equal 1, specs.size
34+
patterns = specs.first.error_patterns(:render_error)
35+
assert_equal 1, patterns.size
36+
assert_kind_of Regexp, patterns[0]
37+
assert patterns[0].match?("Liquid error: divided by 0")
38+
assert patterns[0].match?("ZeroDivisionError")
39+
refute patterns[0].match?("something unrelated")
40+
end
41+
end
42+
43+
def test_loader_rejects_ruby_object_tag
44+
with_temp_spec(<<~YAML) do |path|
45+
---
46+
- name: t
47+
template: "{{ x }}"
48+
environment:
49+
o: !ruby/object:Object {}
50+
YAML
51+
err = assert_raises(RuntimeError) do
52+
Liquid::Spec::SpecLoader.load_yaml_file(path)
53+
end
54+
assert_match(/!ruby\/ tags which are not allowed/, err.message)
55+
end
56+
end
57+
58+
def test_loader_mixed_regexp_and_string_patterns
59+
with_temp_spec(<<~YAML) do |path|
60+
---
61+
- name: t
62+
template: "{{ 10 | divided_by: 0 }}"
63+
errors:
64+
render_error:
65+
- !ruby/regexp /ZeroDivision|divided by/i
66+
- divided by 0
67+
YAML
68+
specs = Liquid::Spec::SpecLoader.load_yaml_file(path)
69+
patterns = specs.first.error_patterns(:render_error)
70+
assert_equal 2, patterns.size
71+
assert patterns[0].match?("ZeroDivision")
72+
# String pattern is escaped + case-insensitive substring
73+
assert patterns[1].match?("DIVIDED BY 0")
74+
end
75+
end
76+
77+
def test_run_spec_regexp_pattern_matches
78+
skip "liquid gem required" unless defined?(Liquid::Template)
79+
assert_spec_passes(<<~YAML)
80+
---
81+
- name: t
82+
template: "{{ 10 | divided_by: 0 }}"
83+
errors:
84+
render_error:
85+
- !ruby/regexp /ZeroDivision|divided by/i
86+
YAML
87+
end
88+
89+
def test_run_spec_regexp_pattern_non_match_fails
90+
skip "liquid gem required" unless defined?(Liquid::Template)
91+
refute_spec_passes(<<~YAML)
92+
---
93+
- name: t
94+
template: "{{ 10 | divided_by: 0 }}"
95+
errors:
96+
render_error:
97+
- !ruby/regexp /this will not match/
98+
YAML
99+
end
100+
101+
def test_run_spec_multiple_substring_patterns_all_match
102+
skip "liquid gem required" unless defined?(Liquid::Template)
103+
assert_spec_passes(<<~YAML)
104+
---
105+
- name: t
106+
template: "{{ 10 | divided_by: 0 }}"
107+
errors:
108+
render_error:
109+
- divided
110+
- by 0
111+
YAML
112+
end
113+
114+
def test_run_spec_multiple_substring_patterns_one_missing_fails
115+
skip "liquid gem required" unless defined?(Liquid::Template)
116+
refute_spec_passes(<<~YAML)
117+
---
118+
- name: t
119+
template: "{{ 10 | divided_by: 0 }}"
120+
errors:
121+
render_error:
122+
- divided by 0
123+
- this substring is absent
124+
YAML
125+
end
126+
127+
def test_run_spec_class_name_match
128+
# The reference liquid-ruby raises Liquid::ZeroDivisionError; matching the
129+
# class name as a substring ("ZeroDivisionError") should pass.
130+
skip "liquid gem required" unless defined?(Liquid::Template)
131+
assert_spec_passes(<<~YAML)
132+
---
133+
- name: t
134+
template: "{{ 10 | divided_by: 0 }}"
135+
errors:
136+
render_error:
137+
- ZeroDivisionError
138+
YAML
139+
end
140+
141+
def test_run_spec_class_name_non_match_fails
142+
skip "liquid gem required" unless defined?(Liquid::Template)
143+
refute_spec_passes(<<~YAML)
144+
---
145+
- name: t
146+
template: "{{ 10 | divided_by: 0 }}"
147+
errors:
148+
render_error:
149+
- ArgumentError
150+
YAML
151+
end
152+
153+
def test_run_spec_case_insensitive_substring
154+
skip "liquid gem required" unless defined?(Liquid::Template)
155+
assert_spec_passes(<<~YAML)
156+
---
157+
- name: t
158+
template: "{{ 10 | divided_by: 0 }}"
159+
errors:
160+
render_error:
161+
- DIVIDED BY 0
162+
YAML
163+
end
164+
165+
private
166+
167+
def run_one_spec(spec_yaml)
168+
setup_liquid_ruby_adapter!
169+
Dir.mktmpdir do |dir|
170+
path = File.join(dir, "spec.yml")
171+
File.write(path, spec_yaml)
172+
specs = Liquid::Spec::SpecLoader.load_yaml_file(path)
173+
spec = specs.first
174+
require "liquid/spec/cli/runner"
175+
runner = Liquid::Spec::CLI::Runner
176+
# run_single_spec is a private class method; invoke it via send
177+
result = runner.send(:run_single_spec, spec, nil)
178+
result
179+
end
180+
end
181+
182+
@adapter_loaded = false
183+
def setup_liquid_ruby_adapter!
184+
return if self.class.instance_variable_get(:@adapter_loaded)
185+
require "liquid"
186+
example = File.expand_path("../examples/liquid_ruby.rb", __dir__)
187+
load example
188+
self.class.instance_variable_set(:@adapter_loaded, true)
189+
end
190+
191+
def assert_spec_passes(spec_yaml)
192+
result = run_one_spec(spec_yaml)
193+
assert_equal :pass, result[:status],
194+
"expected spec to pass but got #{result[:status]}: #{result.inspect}"
195+
end
196+
197+
def refute_spec_passes(spec_yaml)
198+
result = run_one_spec(spec_yaml)
199+
refute_equal :pass, result[:status],
200+
"expected spec to NOT pass but it did: #{result.inspect}"
201+
end
202+
end

test/lazy_spec_test.rb

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,29 @@ def test_error_patterns
6868
assert patterns[1].match?("UNEXPECTED token")
6969
end
7070

71+
def test_error_patterns_pass_regexp_through_unchanged
72+
# A Regexp pattern is used as-is (not escaped), so metacharacters work.
73+
regexp = /divided by \d+/i
74+
spec = create_spec(errors: { "render_error" => [regexp] })
75+
patterns = spec.error_patterns(:render_error)
76+
77+
assert_equal 1, patterns.size
78+
assert_same regexp, patterns[0]
79+
assert patterns[0].match?("Liquid error: divided by 0")
80+
refute patterns[0].match?("divided by zero")
81+
end
82+
83+
def test_error_patterns_mix_regexp_and_string
84+
spec = create_spec(errors: { "render_error" => [/ZeroDivision|divided by/i, "divided by 0"] })
85+
patterns = spec.error_patterns(:render_error)
86+
87+
assert_equal 2, patterns.size
88+
assert patterns[0].is_a?(Regexp)
89+
assert patterns[0].match?("ZeroDivision")
90+
# String pattern is escaped + case-insensitive substring
91+
assert patterns[1].match?("DIVIDED BY 0")
92+
end
93+
7194
def test_features
7295
spec = create_spec(features: [:shopify_tags, :shopify_filters])
7396
assert_equal [:shopify_tags, :shopify_filters], spec.features

0 commit comments

Comments
 (0)