Skip to content

test(workflows): pin numeric equality on captured shell stdout with trailing newline - #5357

Merged
mnriem merged 1 commit into
github:mainfrom
rohanb123:tests/test-workflows-pin-numeric-equality-on-c
Nov 10, 2026
Merged

mnriem merged 1 commit into
github:mainfrom
rohanb123:tests/test-workflows-pin-numeric-equality-on-c

Conversation

@rohanb123

Copy link
Copy Markdown
Contributor

Description

#5310 made == and != coerce numeric strings the same way >, <, >= and <= already did, so captured stdout such as "10" is now == 10. Its regression test used a hand-built StepContext with stdout: "10". Real shell output keeps its trailing newline (echo 10 captures "10\n"), and no test covered that path end to end.

This adds one parametrized test to tests/workflows/test_shell_step.py, written like the existing engine-level tests in that file. A shell step prints a value and an if step branches on steps.count.output.stdout. The test checks:

  • == 10 / != 11 take then, and != 10 / == 11 take else, for stdout "10\n"
  • >= 10 and <= 10 agree with == on the same value
  • a decimal value ("2.5\n" == 2.5) compares as a number
  • non-numeric output ("10x\n") is still compared as a string, so == 10 is false and != 10 is true
  • the raw stdout field still keeps its trailing newline

This only adds tests. Runtime behavior is unchanged.

Testing

Set up this checkout's .venv and editable install using the local development guide. Run commands from the checkout root.

Ran in a clean python:3.14 container:

pip install --no-cache-dir uv && uv sync --extra test
export LC_ALL=C.UTF-8 && .venv/bin/python -m pytest tests/workflows/test_shell_step.py -q && uvx ruff@0.15.0 check tests/workflows/test_shell_step.py

Result: pytest collected 160 items in tests/workflows/test_shell_step.py and reported 160 passed in 1.12s. ruff check on the same file reported All checks passed!. The command exited with status 0.

  • Tested locally with .venv/bin/specify --help (Windows PowerShell: .\.venv\Scripts\specify.exe --help)
  • Ran relevant existing tests with .venv/bin/python -m pytest tests -q (Windows PowerShell: .\.venv\Scripts\python.exe -m pytest tests -q); use affected test paths for focused checks and LC_ALL=en_US.UTF-8 on macOS
  • Tested with a sample project (if applicable)

AI Disclosure

  • I did not use AI assistance for this contribution
  • I did use AI assistance (fill in the disclosure below)

AI disclosure: Written by Claude Code (model: Claude Opus 5.5, low reasoning effort, autonomous) on behalf of Rohan Bharti. The agent picked the gap, wrote the test and wrote this description. Nobody reviewed the change line by line before it was opened.


Prepared with AI assistance (Claude Code). The tests described above were run before opening this PR. The submitter reviewed the diff before it was opened.

…railing newline

Co-Authored-By: Claude <noreply@anthropic.com>
@rohanb123
rohanb123 requested a review from mnriem as a code owner November 10, 2026 00:56
Copilot AI auto review requested due to automatic review settings November 10, 2026 00:57

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The review records no active issues, and the change is limited to focused regression coverage for shell-output comparisons.

No open issues 🎉

What changed in this PR

Adds end-to-end regression coverage for numeric comparisons against shell-step stdout with a trailing newline. Runtime behavior is unchanged.

Changes:

  • Tests equality, inequality, and ordering comparisons against captured stdout.
  • Covers decimal values and non-numeric string fallback behavior.
  • Verifies raw stdout retains its trailing newline and conditional branches behave correctly.
File Description
tests/​workflows/​test_shell_step.py Adds parametrized engine-level coverage for numeric and string comparisons on captured shell output.

Review effort: Lite (auto)

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

No active review issues remain, and the changes are limited to focused end-to-end regression coverage for numeric comparisons involving shell-step stdout.

No open issues 🎉

Review effort: Lite (auto)

@mnriem mnriem added the triage-can-wait Verdict: valid and in-scope but deprioritized; held behind the evidence gate label Nov 10, 2026
@mnriem
mnriem merged commit 20f0161 into github:main Nov 10, 2026
16 checks passed
@mnriem

mnriem commented Nov 10, 2026

Copy link
Copy Markdown
Collaborator

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

triage-can-wait Verdict: valid and in-scope but deprioritized; held behind the evidence gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants