Skip to content

Commit d933f9a

Browse files
committed
fix: Report the CLI's own reason when a parse yields no verdict
`astro dev parse` states why it refused — a project configured for standalone dev mode, a venv without pytest — but parse_check folded every non-completing run into rc 2 and verify.sh reported the catch-all "produced no recognizable result". Diagnosing that required digging the raw log out of a CI run. Capture the CLI's message as "cli_error" and prefer it in the fallback note. The rc contract is unchanged: these are all still "no verdict, degrade", and the image-build failure keeps rc 4 because BUILD_MARKER is matched first even though it wears the same wrapper. A timeout keeps its own wording, since a kill leaves no explanation behind.
1 parent b735753 commit d933f9a

3 files changed

Lines changed: 79 additions & 4 deletions

File tree

scripts/parse_check.py

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,9 @@
1818
the repo's committed .astro/test_dag_integrity_default.py (generated by
1919
an older astro CLI) calls DagBag with an argument the target Airflow
2020
removed, so the harness itself is incompatible with the target.
21-
2 output not recognized as an `astro dev parse` run (infra; caller treats
22-
like an env failure, never like a code verdict)
21+
2 no verdict: the run never completed, or the CLI refused to start it. When
22+
the CLI said why, "cli_error" carries its reason verbatim so the caller can
23+
show it instead of a catch-all (infra; never a code verdict)
2324
"""
2425

2526
from __future__ import annotations
@@ -47,6 +48,14 @@
4748
_EXC_HEAD = re.compile(r"^E\s+Exception: (?P<path>\S+) failed to import with message")
4849
_EXC_CLASS = re.compile(r"^([A-Za-z_][A-Za-z0-9_.]*(?:Error|Exception|Warning))\b")
4950
_CLEAN = "No errors detected in your DAGs"
51+
# The CLI's own reason for not producing a result, e.g. a project configured for
52+
# standalone dev mode ("no virtual environment found — run 'astro dev start'
53+
# first") or a venv without pytest. Kept out of the rc contract: these are all
54+
# "no verdict, degrade" (rc 2), and the value only enriches the message. The
55+
# image-build failure wears the same wrapper, which is why BUILD_MARKER is
56+
# matched first — a build failure is a real verdict (rc 4), not a missing one.
57+
_CLI_ERROR = re.compile(
58+
r"^Error: something went wrong while parsing your DAGs: (?P<cause>.+)$", re.MULTILINE)
5059

5160

5261
def parse_output(text: str) -> tuple[int, dict]:
@@ -103,7 +112,11 @@ def parse_output(text: str) -> tuple[int, dict]:
103112
# items" alone is what a timeout kill leaves behind.
104113
completed = _CLEAN in text or _SUMMARY.search(text) is not None
105114
if not completed:
106-
return 2, {"checked": 0, "failures": []}
115+
result: dict = {"checked": 0, "failures": []}
116+
m = _CLI_ERROR.search(text)
117+
if m:
118+
result["cli_error"] = m.group("cause").strip()
119+
return 2, result
107120
return (3 if failures else 0), {"checked": checked, "failures": sorted(
108121
failures.values(), key=lambda f: f["path"])}
109122

scripts/verify.sh

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,18 @@ verify_parse_level() {
238238
return 1
239239
elif [[ "$target_rc" -eq 2 ]]; then
240240
local reason="produced no recognizable result"
241-
[[ "$target_run_rc" -eq 124 ]] && reason="timed out before completing"
241+
local cli_error
242+
if [[ "$target_run_rc" -eq 124 ]]; then
243+
# A timeout kill leaves no explanation of its own, so keep ours.
244+
reason="timed out before completing"
245+
else
246+
# The CLI usually states exactly why it refused (a project configured for
247+
# standalone dev mode, a venv without pytest, …). Its words beat the
248+
# catch-all: "produced no recognizable result" is what made a field case
249+
# take a full log dig to explain.
250+
cli_error=$(jq -r '.cli_error // empty' "$WORKDIR/import-failures.json" 2>/dev/null || true)
251+
[[ -n "$cli_error" ]] && reason="failed: ${cli_error}"
252+
fi
242253
fallback_note="ℹ️ Image-level verification could not run (\`astro dev parse\` ${reason}). Results below come from the import-level fallback."
243254
echo "::warning::verify-level parse: astro dev parse ${reason}; falling back to import-level verification."
244255
return 1

tests/test_parse_check.py

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,57 @@ def test_collection_error_detected_with_cause(tmp_path):
114114
def test_unrecognized_output_is_infra(tmp_path):
115115
rc, result, *_ = _run(tmp_path, "docker daemon not running\n")
116116
assert rc == 2
117+
# Nothing to quote: the caller falls back to its own catch-all wording.
118+
assert "cli_error" not in result
119+
120+
121+
STANDALONE_NO_VENV_RUN = """\
122+
Checking your DAGs for errors…
123+
Error: something went wrong while parsing your DAGs: no virtual environment found \
124+
— run 'astro dev start' first
125+
"""
126+
127+
STANDALONE_NO_PYTEST_RUN = """\
128+
Checking your DAGs for errors…
129+
Error: something went wrong while parsing your DAGs: exec: "pytest": \
130+
executable file not found in $PATH
131+
"""
132+
133+
134+
def test_cli_refusal_carries_its_own_reason(tmp_path):
135+
# A project committing `dev.mode: standalone` sends the CLI to a gitignored
136+
# local .venv no CI checkout has. Still no verdict (rc 2), but the reason
137+
# must reach the caller — reporting only "produced no recognizable result"
138+
# is what made this take a log dig to diagnose in the field.
139+
rc, result, *_ = _run(tmp_path, STANDALONE_NO_VENV_RUN)
140+
assert rc == 2
141+
assert result["cli_error"] == "no virtual environment found — run 'astro dev start' first"
142+
143+
144+
def test_cli_refusal_half_provisioned_venv(tmp_path):
145+
# The nastier standalone shape: the venv exists (so the CLI's own venv check
146+
# passes) but nothing installed pytest into it.
147+
rc, result, *_ = _run(tmp_path, STANDALONE_NO_PYTEST_RUN)
148+
assert rc == 2
149+
assert result["cli_error"] == 'exec: "pytest": executable file not found in $PATH'
150+
151+
152+
def test_build_failure_wrapper_is_not_demoted_to_cli_error(tmp_path):
153+
# The image-build failure wears the same "something went wrong while parsing
154+
# your DAGs" wrapper. It is a real verdict (rc 4) and must not be reclassified
155+
# as a missing one just because the wrapper matches.
156+
rc, result, *_ = _run(tmp_path, BUILD_FAILURE_RUN)
157+
assert rc == 4
158+
assert "cli_error" not in result
159+
160+
161+
def test_completed_run_never_gains_a_cli_error(tmp_path):
162+
# Defensive: a finished run is a verdict. Even if the wrapper somehow appears
163+
# alongside a closing summary, the summary wins.
164+
text = FAILING_RUN + "Error: something went wrong while parsing your DAGs: noise\n"
165+
rc, result, *_ = _run(tmp_path, text)
166+
assert rc == 3
167+
assert "cli_error" not in result
117168

118169

119170
def test_long_run_summary_with_hms_suffix_is_recognized(tmp_path):

0 commit comments

Comments
 (0)