Skip to content

Clearly separate literals and patterns in the test DSL - #822

Merged
jviotti merged 3 commits into
mainfrom
clitest-literal-patterns
Aug 14, 2026
Merged

Clearly separate literals and patterns in the test DSL#822
jviotti merged 3 commits into
mainfrom
clitest-literal-patterns

Conversation

@jviotti

@jviotti jviotti commented Aug 14, 2026

Copy link
Copy Markdown
Member

Signed-off-by: Juan Cruz Viotti jv@jviotti.com

Review in cubic

Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
@augmentcode

augmentcode Bot commented Aug 14, 2026

Copy link
Copy Markdown
🤖 Augment PR Summary

Summary: This PR refines the internal CLI-test DSL by separating literal filters from regular-expression filters.

Changes:

  • Makes REPLACE use literal text by default.
  • Adds REPLACE MATCHING for regex substitution.
  • Adds DROP LINES CONTAINING while retaining regex-based DROP LINES MATCHING.
  • Moves regex compilation and validation into a dedicated helper.
  • Keeps replacement operands literal and variable-expandable.
  • Migrates path, hash, and JSON-output fixtures to literal filter forms.
  • Migrates benchmark normalisation fixtures to the explicit regex form.
Technical Notes: Patterns now compile in multiline mode and are intended not to expand variables; the fixture updates remove regex escaping where literal matching is sufficient.

🤖 Was this summary useful? React with 👍 or 👎

@augmentcode augmentcode Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review completed. 1 suggestion posted.

Fix All in Augment

Comment augment review to trigger a new review at any time.

Comment thread scripts/clitest.py Outdated
# one out of a variable is what keeps the two forms from ever meeting,
# and so is what removes the escaping rule that would otherwise be
# needed to stop an expanded path from being read as more pattern
if VARIABLE.search(token):

@augmentcode augmentcode Bot Aug 14, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

compile_pattern is documented as receiving patterns verbatim, but VARIABLE.search also matches an escaped regex dollar. For example, REPLACE MATCHING '^\s*"\$schema":' ... is rejected as a variable reference, so regex filters cannot match common JSON Schema $-prefixed keywords (nor use $$).

Severity: medium

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 issue found across 76 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="scripts/clitest.py">

<violation number="1" location="scripts/clitest.py:289">
P2: The existing help tests still pass `\\.exe` expecting regex removal. This literal branch leaves `jsonschema.exe` unchanged on Windows, so those tests fail; migrate those callers or preserve compatibility.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread scripts/clitest.py
self.expand(replacement, line_number).replace("\\", "\\\\"), content)
expanded = self.expand(replacement, line_number)
if expression is None:
content = content.replace(

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: The existing help tests still pass \\.exe expecting regex removal. This literal branch leaves jsonschema.exe unchanged on Windows, so those tests fail; migrate those callers or preserve compatibility.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/clitest.py, line 289:

<comment>The existing help tests still pass `\\.exe` expecting regex removal. This literal branch leaves `jsonschema.exe` unchanged on Windows, so those tests fail; migrate those callers or preserve compatibility.</comment>

<file context>
@@ -256,33 +267,50 @@ def command_compare(self, operands, line_number):
-                self.expand(replacement, line_number).replace("\\", "\\\\"), content)
+        expanded = self.expand(replacement, line_number)
+        if expression is None:
+            content = content.replace(
+                self.expand(operands[0], line_number), expanded)
         else:
</file context>

Comment thread scripts/clitest.py Outdated
Comment thread scripts/clitest.py
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 1 file (changes from recent commits).

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread scripts/clitest.py
Comment thread scripts/clitest.py
Signed-off-by: Juan Cruz Viotti <jv@jviotti.com>
@jviotti
jviotti merged commit e7ab75e into main Aug 14, 2026
16 checks passed
@jviotti
jviotti deleted the clitest-literal-patterns branch August 14, 2026 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant