Bug - Pattern Liquid tests #244
Conversation
WalkthroughAdds YAML anchor/alias preservation to pattern-based test filtering in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Comment |
delphine-demeulenaere
force-pushed
the
bug_pattern_liquid_testing
branch
from
bb83e86 to
399cec8
Compare
delphine-demeulenaere
force-pushed
the
bug_pattern_liquid_testing
branch
from
399cec8 to
615505f
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@CHANGELOG.md`:
- Around line 5-7: Fix typos in the CHANGELOG entry for version header "##
[1.52.1] (21/01/2026)" by correcting "conatin" to "contain", adding the missing
space between the inline code and the word "command" so "`silverfin run-test`
command" reads correctly, and add the missing comma on the line describing how
to enable it so the sentence reads "To enable it, run `silverfin run-test -p
\"string pattern\" -h template_handle`".
In `@lib/liquidTestRunner.js`:
- Around line 176-195: The current logic prepends whole test blocks from
extractAnchorBlocks into filteredContent, which reintroduces excluded tests and
breaks line tracking; change extractAnchorBlocks (and its call site) to return
only the anchor node definitions (or directly inline the resolved anchor nodes
into the matching tests found by findAliasReferences) rather than full test
blocks pulled from testContent/indexes, and when you insert those anchor
snippets update lineAdjustments (which currently only maps orderedMatchingTests)
to include mappings for any added lines so error locations remain correct;
alternatively, if you prefer server-side enforcement, modify the payload sent to
the backend to include the filtering pattern or explicit test names instead of
sending all tests so the server will not execute injected blocks.
CHANGELOG.md
Outdated
| ## [1.52.1] (21/01/2026) | ||
| In this version we are introducing Liquid batch/pattern testing. An extra option was added to the `silverfin run-test`command to run all tests which conatin a common string. | ||
| To enable it run `silverfin run-test -p "string pattern" -h template_handle` |
There was a problem hiding this comment.
Fix typos in the 1.52.1 entry.
Line 6 has “conatin” and a missing space in “run-test`command”; Line 7 is missing a comma.
✏️ Proposed fix
- In this version we are introducing Liquid batch/pattern testing. An extra option was added to the `silverfin run-test`command to run all tests which conatin a common string.
- To enable it run `silverfin run-test -p "string pattern" -h template_handle`
+ In this version we are introducing Liquid batch/pattern testing. An extra option was added to the `silverfin run-test` command to run all tests which contain a common string.
+ To enable it, run `silverfin run-test -p "string pattern" -h template_handle`📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## [1.52.1] (21/01/2026) | |
| In this version we are introducing Liquid batch/pattern testing. An extra option was added to the `silverfin run-test`command to run all tests which conatin a common string. | |
| To enable it run `silverfin run-test -p "string pattern" -h template_handle` | |
| ## [1.52.1] (21/01/2026) | |
| In this version we are introducing Liquid batch/pattern testing. An extra option was added to the `silverfin run-test` command to run all tests which contain a common string. | |
| To enable it, run `silverfin run-test -p "string pattern" -h template_handle` |
🤖 Prompt for AI Agents
In `@CHANGELOG.md` around lines 5 - 7, Fix typos in the CHANGELOG entry for
version header "## [1.52.1] (21/01/2026)" by correcting "conatin" to "contain",
adding the missing space between the inline code and the word "command" so
"`silverfin run-test` command" reads correctly, and add the missing comma on the
line describing how to enable it so the sentence reads "To enable it, run
`silverfin run-test -p \"string pattern\" -h template_handle`".
| // Check for alias references in the filtered content and add missing anchor definitions | ||
| const aliasReferences = findAliasReferences(filteredContent); | ||
| if (aliasReferences.length > 0) { | ||
| // Find anchors that are defined in the filtered content | ||
| const definedAnchors = new Set(); | ||
| const anchorPattern = /&([a-zA-Z_][a-zA-Z0-9_]*)/g; | ||
| let match; | ||
| while ((match = anchorPattern.exec(filteredContent)) !== null) { | ||
| definedAnchors.add(match[1]); | ||
| } | ||
|
|
||
| // Find aliases that reference anchors not in the filtered content | ||
| const missingAnchors = aliasReferences.filter((alias) => !definedAnchors.has(alias)); | ||
|
|
||
| if (missingAnchors.length > 0) { | ||
| // Extract the test blocks containing the missing anchors | ||
| const anchorBlocks = extractAnchorBlocks(testContent, missingAnchors, indexes); | ||
| if (anchorBlocks) { | ||
| filteredContent = anchorBlocks + "\n\n" + filteredContent; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "liquidTestRunner.js" | head -20Repository: silverfin/silverfin-cli
Length of output: 92
🏁 Script executed:
cd lib && wc -l liquidTestRunner.jsRepository: silverfin/silverfin-cli
Length of output: 90
🏁 Script executed:
cat -n lib/liquidTestRunner.js | sed -n '170,200p'Repository: silverfin/silverfin-cli
Length of output: 1477
🏁 Script executed:
cat -n lib/liquidTestRunner.js | sed -n '195,250p'Repository: silverfin/silverfin-cli
Length of output: 1947
🏁 Script executed:
ast-grep --pattern 'function extractAnchorBlocks($_) { $$$}'Repository: silverfin/silverfin-cli
Length of output: 49
🏁 Script executed:
rg -n "extractAnchorBlocks" lib/liquidTestRunner.jsRepository: silverfin/silverfin-cli
Length of output: 229
🏁 Script executed:
rg -n "function extractAnchorBlocks" lib/Repository: silverfin/silverfin-cli
Length of output: 163
🏁 Script executed:
cat -n lib/liquidTestRunner.js | sed -n '84,150p'Repository: silverfin/silverfin-cli
Length of output: 2495
🏁 Script executed:
cat -n lib/liquidTestRunner.js | sed -n '1,83p' | tail -40Repository: silverfin/silverfin-cli
Length of output: 1474
🏁 Script executed:
rg -n "filterTestsByPattern" lib/liquidTestRunner.jsRepository: silverfin/silverfin-cli
Length of output: 286
🏁 Script executed:
cat -n lib/liquidTestRunner.js | sed -n '250,350p'Repository: silverfin/silverfin-cli
Length of output: 4065
🏁 Script executed:
rg -n "lineAdjustments" lib/liquidTestRunner.jsRepository: silverfin/silverfin-cli
Length of output: 849
🏁 Script executed:
rg -n "test_line" lib/liquidTestRunner.jsRepository: silverfin/silverfin-cli
Length of output: 124
🏁 Script executed:
cat -n lib/liquidTestRunner.js | sed -n '294,310p'Repository: silverfin/silverfin-cli
Length of output: 596
🏁 Script executed:
rg -n "SF\." lib/liquidTestRunner.js | head -20Repository: silverfin/silverfin-cli
Length of output: 336
🏁 Script executed:
cat -n lib/liquidTestRunner.js | sed -n '420,470p'Repository: silverfin/silverfin-cli
Length of output: 2497
🏁 Script executed:
rg -n "pattern" lib/liquidTestRunner.js | head -30Repository: silverfin/silverfin-cli
Length of output: 1497
🏁 Script executed:
rg -n "createTestRun\|createPreviewRun" lib/ -A 5Repository: silverfin/silverfin-cli
Length of output: 49
🏁 Script executed:
fd "*.js" lib/ -o | xargs grep -l "createTestRun\|createPreviewRun" | head -5Repository: silverfin/silverfin-cli
Length of output: 183
🏁 Script executed:
find lib -name "*.js" -type f | head -20Repository: silverfin/silverfin-cli
Length of output: 570
🏁 Script executed:
rg -n "createTestRun" lib/Repository: silverfin/silverfin-cli
Length of output: 306
🏁 Script executed:
cat -n lib/api/sfApi.js | sed -n '407,450p'Repository: silverfin/silverfin-cli
Length of output: 1784
🏁 Script executed:
cat -n lib/liquidTestRunner.js | sed -n '550,565p'Repository: silverfin/silverfin-cli
Length of output: 774
🏁 Script executed:
cat -n lib/liquidTestRunner.js | sed -n '116,125p'Repository: silverfin/silverfin-cli
Length of output: 420
Pattern filtering is undermined by prepending full test blocks containing unmatched tests.
The extractAnchorBlocks function (line 117) returns entire test blocks, not just anchor definitions. These blocks come from tests that don't match the filter pattern and are prepended to filteredContent (line 194). Since no pattern information is passed to the backend API (lines 294-298 send only template, tests, and mode), the server executes all test blocks in the YAML—including those injected for anchor resolution. This reintroduces excluded tests and defeats the filtering guarantee.
Additionally, lineAdjustments only contains mappings for orderedMatchingTests (lines 202-208), so tests from injected anchor blocks lack proper line tracking, potentially causing incorrect error reporting.
Consider instead inlining referenced anchor nodes into matching tests or implementing server-side test filtering by name.
🤖 Prompt for AI Agents
In `@lib/liquidTestRunner.js` around lines 176 - 195, The current logic prepends
whole test blocks from extractAnchorBlocks into filteredContent, which
reintroduces excluded tests and breaks line tracking; change extractAnchorBlocks
(and its call site) to return only the anchor node definitions (or directly
inline the resolved anchor nodes into the matching tests found by
findAliasReferences) rather than full test blocks pulled from
testContent/indexes, and when you insert those anchor snippets update
lineAdjustments (which currently only maps orderedMatchingTests) to include
mappings for any added lines so error locations remain correct; alternatively,
if you prefer server-side enforcement, modify the payload sent to the backend to
include the filtering pattern or explicit test names instead of sending all
tests so the server will not execute injected blocks.
Description
The CLI reads the YAML file as a string. When we run tests and define a pattern, those tests can include an alias, but the anchor was defined in a different section that gets filtered out when running the tests.
Testing Instructions
Steps to reproduce the behavior:
silverfin run-test -f <firm_id> -h <handle> --pattern "pattern"Include a template test pattern that makes use of an alias in the Liquid test.
Author Checklist
Reviewer Checklist