{{ message }}
Repair the Tauri retry test against the download-marker pipeline - #8996
Merged
Conversation
Shell installer tests has been failing on main, and failing in the worst way: the file printed its header and exited 1 with no FAIL line, because two separate things broke and both of them are silent. #8805 added the uv download markers, which put the wrapped command inside `{ ...; } | _uv_download_markers ...`. The verbose path was already a pipe; the quiet path, which is the one this file exercises, was not. Two consequences: 1. This file hand-copies five functions out of install.sh and stubs five more. It never learned about _uv_download_markers, so the last stage of that pipeline was "command not found", every wrapped command looked like it exited 127, and `set -e` ended the run. Extract the real function rather than stub it, so the test still exercises the pipe the installer actually runs. 2. The command now runs in a subshell, so _test_command's `_test_attempt` increments were discarded. It failed on every attempt instead of succeeding on the second, which is precisely the retry-recovers path the first assertion is about. Move the counter into a file. Both are needed; each alone still fails, at a different assertion. #8805 is right on both counts. Streaming output through a marker filter requires a pipe, and real installs run external commands, so a subshell costs them nothing. It was the test's harness that had gone stale. Also add `|| true` to the two `grep -c` calls that gate the first assertion. grep -c exits 1 on a zero count, which under `set -e` aborts with no output. That is why this read as silence rather than as a failure, and the same guard is already applied to the grep -c calls later in the file. tests/sh: 47 files, all passing.
The second trap replaces the first, so naming the attempt file only in the first one leaked it: one stray temp file per run, measured.
My earlier commit message claimed the grep -c guard was already applied to the rest of the file. That was wrong: lines 231, 326 and 368 were still unguarded, so a zero count there aborts under set -e before the FAIL line it gates. Worse, the guard I did add only covers the case where a wrapped command succeeds and emits the wrong markers. The dominant path is the command itself returning non-zero: under set -e that aborts at the call, before any assertion runs. A broken installer retry therefore still reproduced the exact header-then-silence symptom this file was fixed for. Guard the three grep -c sites, and give the four commands that must succeed an explicit failure message instead of letting set -e end the run mutely. Each path mutation checked, silence to a named failure: retry never recovers -> FAIL: recovered retry returned non-zero rollback_substep renamed -> FAIL: successful Unix rollback output ... UNSLOTH_TAURI_MODE renamed -> FAIL: Unix installer does not pass Tauri mode tests/sh: 47 files, all passing, no temp files leaked.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

Shell installer testsis failing on main, and failing in the worst way: the file prints its header and exits 1 with no FAIL line at all.Two separate things broke, and both are silent.
Cause
#8805 (
b69bfe021) added the uv download markers, which put the wrapped command inside{ ...; } | _uv_download_markers .... The verbose path was already a pipe. The quiet path, which is the one this file exercises, was not:That has two consequences for this test:
The helper is missing from the harness. This file hand-copies five functions out of
install.shand stubs five more. It never learned about_uv_download_markers, so the last stage of the pipeline was "command not found", every wrapped command looked like it exited 127, andset -eended the run.The command now runs in a subshell.
_test_commandsimulated "fail once, then succeed" by incrementing a shell variable. Those increments are discarded now, so it failed on every attempt instead of succeeding on the second, which is precisely the retry-recovers path the first assertion exists to check.#8805 is correct on both counts. Streaming output through a marker filter requires a pipe, and real installs run external commands, so running them in a subshell costs them nothing. It is the test harness that had gone stale.
Fix
_uv_download_markersfrominstall.shrather than stub it, so the test still exercises the real pipe the installer runs.|| trueto the twogrep -ccalls gating the first assertion.grep -cexits 1 on a zero count, which underset -eaborts with no output. That is why this read as silence rather than as a failure. The same guard is already applied to thegrep -ccalls further down the file.Verification
Both fixes are needed. Each alone still fails, at a different assertion:
FAIL: quiet failure did not bind its command output to the structured errorWhole suite, discovered and run the way the workflow does it: 47 files, 0 failures.