Skip to content

test(e2e): assert captured output without shell eval#10257

Merged
jdx merged 1 commit into
mainfrom
fix/e2e-assert-captured-output
Jun 6, 2026
Merged

test(e2e): assert captured output without shell eval#10257
jdx merged 1 commit into
mainfrom
fix/e2e-assert-captured-output

Conversation

@jdx

@jdx jdx commented Jun 6, 2026

Copy link
Copy Markdown
Owner

Summary

  • add text-based e2e assertion helpers for already-captured output
  • update the Python GitHub attestation e2e test to avoid re-evaluating captured output through bash -c

Why

When mise install output contains warning text with shell-sensitive characters, e.g. (403 Forbidden) inside a quoted URL error, assert_contains "echo "$output"" ... can turn captured output back into shell syntax and fail with bash: -c: line 5: syntax error near unexpected token ('`.

Verification

  • bash -n e2e/assert.sh e2e/core/test_python_github_attestations
  • synthetic assertion check with (403 Forbidden) warning text
  • mise run test:e2e e2e/core/test_python_github_attestations

This PR was generated by an AI coding assistant.


Note

Low Risk
Test-only assertion helpers and one e2e test update; no production or runtime behavior changes.

Overview
Adds assert_contains_text and assert_not_contains_text in e2e/assert.sh so e2e tests can assert substrings on already-captured stdout/stderr without wrapping it in a command that runs under bash -c.

The Python GitHub attestations e2e test now passes $output directly to those helpers instead of assert_contains "echo \"$output\"" ..., avoiding shell parse errors when install logs include parenthesized text (e.g. (403 Forbidden) in URL errors).

Reviewed by Cursor Bugbot for commit 4a6a86c. Bugbot is set up for automated code reviews on this repo. Configure here.

Summary by CodeRabbit

Release Notes

  • Tests
    • Added new assertion helpers for substring validation in test framework
    • Updated existing tests to use improved assertion methods

@coderabbitai

coderabbitai Bot commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

This PR adds two new e2e assertion helper functions—assert_contains_text and assert_not_contains_text—to validate substring presence/absence in already-captured text, and updates the GitHub attestations e2e test to use these helpers for cleaner attestation message checking.

Changes

E2E Substring Assertion Helpers

Layer / File(s) Summary
Substring assertion helpers and test usage
e2e/assert.sh, e2e/core/test_python_github_attestations
New assert_contains_text(actual, substring) and assert_not_contains_text(actual, substring) functions verify substring presence/absence with simple success/fail output. The GitHub attestations test switches from assert_contains to assert_contains_text for cleaner verification of the mise install output messages.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Poem

🐰 Two helpers hopping into the test,
Searching for substrings, putting assertions to rest,
contains_text, not_contains_text dance hand in hand,
Attestations verified with the finest of sand! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 60.87% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'test(e2e): assert captured output without shell eval' accurately describes the main change—adding assertion helpers to check captured output without shell re-evaluation, directly addressing the core issue of shell interpretation errors.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/e2e-assert-captured-output

Comment @coderabbitai help to get the list of available commands and usage tips.

@jdx jdx force-pushed the fix/e2e-assert-captured-output branch from fd9f41b to fc66841 Compare June 6, 2026 23:35
@greptile-apps

greptile-apps Bot commented Jun 6, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a shell-injection bug in the e2e test framework where capturing mise install output containing shell-sensitive characters (e.g. (403 Forbidden) in a URL) and then re-evaluating it via assert_contains "echo \"$output\"" would cause a bash syntax error.

  • Adds assert_contains_text and assert_not_contains_text helpers that accept pre-captured text directly as the first argument, bypassing the bash -c re-evaluation path entirely.
  • Updates test_python_github_attestations to use the new helpers, eliminating the unsafe echo "$output" shell re-evaluation pattern.

Confidence Score: 5/5

Safe to merge — changes are confined to the e2e test helpers and a single test file, with no production code affected.

The fix is correct and well-scoped: the new helpers directly compare pre-captured text without shell re-evaluation, and the updated test properly passes the captured variable rather than wrapping it in a command string. No production code paths are touched.

No files require special attention.

Important Files Changed

Filename Overview
e2e/assert.sh Adds assert_contains_text and assert_not_contains_text helpers that check pre-captured text directly without re-evaluating it via bash. Logic is correct; failure messages omit the actual captured value (P2).
e2e/core/test_python_github_attestations Switches from assert_contains "echo \"$output\"" to assert_contains_text "$output", correctly avoiding shell re-evaluation of captured output that could contain special characters like (403 Forbidden).

Fix All in Claude Code

Reviews (2): Last reviewed commit: "test(e2e): assert captured output withou..." | Re-trigger Greptile

Comment thread e2e/assert.sh
@jdx jdx force-pushed the fix/e2e-assert-captured-output branch from fc66841 to 4a6a86c Compare June 6, 2026 23:39
@jdx jdx enabled auto-merge (squash) June 6, 2026 23:47
@github-actions

github-actions Bot commented Jun 6, 2026

Copy link
Copy Markdown

Hyperfine Performance

mise x -- echo

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2026.6.0 x -- echo 18.8 ± 0.9 16.8 22.9 1.00
mise x -- echo 19.3 ± 1.0 17.5 25.1 1.03 ± 0.07

mise env

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2026.6.0 env 18.3 ± 0.8 16.4 22.5 1.00
mise env 18.8 ± 0.8 17.0 22.6 1.03 ± 0.07

mise hook-env

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2026.6.0 hook-env 18.8 ± 0.8 17.3 23.2 1.00
mise hook-env 19.5 ± 0.9 17.8 25.0 1.04 ± 0.07

mise ls

Command Mean [ms] Min [ms] Max [ms] Relative
mise-2026.6.0 ls 15.4 ± 0.8 13.7 19.5 1.00
mise ls 16.1 ± 0.8 14.8 19.6 1.05 ± 0.08

xtasks/test/perf

Command mise-2026.6.0 mise Variance
install (cached) 131ms 133ms -1%
ls (cached) 57ms 59ms -3%
bin-paths (cached) 63ms 64ms -1%
task-ls (cached) 123ms 124ms +0%

@jdx jdx disabled auto-merge June 6, 2026 23:50
@jdx jdx merged commit 9ccaabd into main Jun 6, 2026
33 checks passed
@jdx jdx deleted the fix/e2e-assert-captured-output branch June 6, 2026 23:50
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