Skip to content

Commit c51b02d

Browse files
authored
verify-action-build: fix RESULT panel cause on binary-download failure (#747)
When a JS action rebuilds cleanly but fails only the binary-download verification check, the RESULT panel incorrectly reported "Differences detected between published and rebuilt JS" because the failure branch selected its message on `is_js_action` rather than the actual cause. Reorder the branches to dispatch on the real failure condition: all_match=False → JS-mismatch message; otherwise binary download failures → binary-download message. Observed on PR #724 (posit-dev/ setup-air): JS row was ✓ pass, Binary row was ✗ fail, yet the RESULT panel still referenced a JS mismatch. Adds regression tests for both paths (JS action with unverified download; JS action with actual JS mismatch). Generated-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 0d16f2e commit c51b02d

2 files changed

Lines changed: 110 additions & 1 deletion

File tree

utils/tests/verify_action_build/test_verification.py

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,3 +254,108 @@ def capture_summary(*args, **kwargs):
254254
assert len(js_rows) == 1
255255
assert js_rows[0][1] == "fail"
256256
assert "approved lock files" in js_rows[0][2]
257+
258+
259+
class TestVerifySingleActionResultMessage:
260+
"""The RESULT panel text must describe the actual failure cause."""
261+
262+
def _patch_stack(
263+
self,
264+
*,
265+
approved: list[dict],
266+
diff_js_side_effect,
267+
binary_download_result,
268+
action_type: str = "node20",
269+
):
270+
patches = {
271+
"parse_action_ref": mock.patch(
272+
"verify_action_build.verification.parse_action_ref",
273+
return_value=("org", "repo", "", "c" * 40),
274+
),
275+
"find_approved_versions": mock.patch(
276+
"verify_action_build.verification.find_approved_versions",
277+
return_value=approved,
278+
),
279+
"build_in_docker": mock.patch(
280+
"verify_action_build.verification.build_in_docker",
281+
return_value=_build_in_docker_result(action_type=action_type),
282+
),
283+
"diff_js_files": mock.patch(
284+
"verify_action_build.verification.diff_js_files",
285+
side_effect=diff_js_side_effect,
286+
),
287+
"show_approved_versions": mock.patch(
288+
"verify_action_build.verification.show_approved_versions",
289+
return_value=None,
290+
),
291+
"show_commits_between": mock.patch(
292+
"verify_action_build.verification.show_commits_between",
293+
),
294+
"diff_approved_vs_new": mock.patch(
295+
"verify_action_build.verification.diff_approved_vs_new",
296+
),
297+
"analyze_binary_downloads_recursive": mock.patch(
298+
"verify_action_build.verification.analyze_binary_downloads_recursive",
299+
return_value=binary_download_result,
300+
),
301+
"Panel": mock.patch("verify_action_build.verification.Panel"),
302+
}
303+
started = {name: p.start() for name, p in patches.items()}
304+
started["_patchers"] = list(patches.values())
305+
return started
306+
307+
def _stop(self, started):
308+
for p in reversed(started["_patchers"]):
309+
p.stop()
310+
311+
def _result_panel_message(self, panel_mock) -> str:
312+
result_calls = [
313+
c for c in panel_mock.call_args_list
314+
if c.kwargs.get("title") == "RESULT"
315+
]
316+
assert len(result_calls) == 1, (
317+
f"expected exactly one RESULT panel, got {len(result_calls)}"
318+
)
319+
return result_calls[0].args[0]
320+
321+
def test_js_action_with_unverified_download_reports_binary_cause(self):
322+
"""Regression for the misleading RESULT panel seen on PR #724:
323+
324+
A JS action that rebuilds cleanly but fails the binary-download
325+
check must surface the binary-download reason in the RESULT
326+
panel, not the default JS-mismatch message.
327+
"""
328+
started = self._patch_stack(
329+
approved=[],
330+
diff_js_side_effect=[True],
331+
binary_download_result=(
332+
[],
333+
["Dockerfile line 3: unverified download: curl -fsSL ..."],
334+
),
335+
)
336+
try:
337+
result = verify_single_action("org/repo@" + "c" * 40, ci_mode=True)
338+
finally:
339+
self._stop(started)
340+
341+
assert result is False
342+
msg = self._result_panel_message(started["Panel"])
343+
assert "unverified binary download" in msg
344+
assert "Differences detected between published and rebuilt JS" not in msg
345+
346+
def test_js_action_with_js_mismatch_reports_js_cause(self):
347+
"""JS mismatch without binary-download failures still reports the
348+
JS-mismatch message (the original behaviour)."""
349+
started = self._patch_stack(
350+
approved=[],
351+
diff_js_side_effect=[False],
352+
binary_download_result=([], []),
353+
)
354+
try:
355+
result = verify_single_action("org/repo@" + "c" * 40, ci_mode=True)
356+
finally:
357+
self._stop(started)
358+
359+
assert result is False
360+
msg = self._result_panel_message(started["Panel"])
361+
assert "Differences detected between published and rebuilt JS" in msg

utils/verify_action_build/verification.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -454,7 +454,11 @@ def verify_single_action(
454454
border = "yellow" if not is_js_action and non_js_warnings else "green"
455455
console.print(Panel(result_msg + checklist_hint, border_style=border, title="RESULT"))
456456
else:
457-
if is_js_action:
457+
# Pick the failure message based on the actual cause, not the action
458+
# type. The binary-download check runs for every action type, so a
459+
# JS action can fail this path with all_match=True when its only
460+
# issue is an unverified download.
461+
if not all_match:
458462
if matched_with_approved_lockfile:
459463
fail_msg = (
460464
"[red bold]Compiled JS only matches when rebuilt with the "

0 commit comments

Comments
 (0)