Skip to content

Commit 9b90eaa

Browse files
jeremymanningclaude
andcommitted
Issue #147: make the real-world runner able to fail
main() dropped every runner.run_*_tests() return value and never called sys.exit, so the script exited 0 no matter what. The pre-push hook guards each category with `if ! python scripts/run_real_world_tests.py --<cat>`, which therefore could never fire: four categories printed "failed" and the hook still announced "All real-world tests passed!" and allowed the push. Same class as the flake8 --exit-zero and mypy continue-on-error steps fixed in #138 -- a check that reports problems but cannot fail. Also fixes the second half of #147: the failure message printed only result.stdout, which was empty in all four observed failures because a pytest collection error goes to stderr. _report_failure now prints the exit code, stdout, stderr, and says so explicitly when there was no output at all. Verified by appending a deliberately failing test to tests/real_world/test_filesystem_real.py and running the script: EXIT CODE: 1 ❌ Filesystem tests failed (exit 1) assert False, "deliberate failure to verify exit-code propagation" E AssertionError: deliberate failure to verify exit-code propagation and, with that test removed, the same command exits 0. Before this change the failing case also exited 0 with an empty message body. Also drops the removed backends from two scripts: the kubernetes tutorial entry in check_docs_examples' _SECTION_BOUNDS (that page is deleted) and the aws/azure/gcp/lambda_cloud entries in the credential display names. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012gTBDPK16HUZ3kHQ2QyjuU
1 parent 62e7a29 commit 9b90eaa

2 files changed

Lines changed: 44 additions & 25 deletions

File tree

scripts/check_docs_examples.py

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -484,12 +484,7 @@ def make_namespace() -> dict:
484484

485485
#: Pages that need a narrower window than "the whole file". Keyed by path
486486
#: relative to the repository root.
487-
_SECTION_BOUNDS = {
488-
"docs/source/tutorials/kubernetes_tutorial.rst": (
489-
"Auto-Provisioning a Cluster\n----",
490-
"Configuration Options\n---",
491-
),
492-
}
487+
_SECTION_BOUNDS: dict = {}
493488

494489
#: Directories under docs/ that are build output or vendored, not sources.
495490
_SKIP_DIRS = {"build", "_build", "_static", "_templates"}

scripts/run_real_world_tests.py

Lines changed: 43 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,24 @@
1313
from typing import Dict
1414

1515

16+
def _report_failure(label: str, result: subprocess.CompletedProcess) -> None:
17+
"""Print everything a failed pytest run produced.
18+
19+
The four categories the pre-push hook runs all reported failure with an
20+
empty body, because only ``stdout`` was printed and a collection error
21+
goes to ``stderr``. An operator was told something failed and not what
22+
(issue #147).
23+
"""
24+
print(f"\u274c {label} failed (exit {result.returncode})")
25+
if result.stdout:
26+
print(result.stdout)
27+
if result.stderr:
28+
print("--- stderr ---")
29+
print(result.stderr)
30+
if not result.stdout and not result.stderr:
31+
print("(no output captured)")
32+
33+
1634
class RealWorldTestRunner:
1735
"""Runner for real-world tests with various configurations."""
1836

@@ -51,13 +69,9 @@ def check_credentials(self) -> Dict[str, bool]:
5169
print(f" 1Password: {'✅' if manager.is_1password_available() else '❌'}")
5270

5371
service_names = {
54-
"aws": "AWS",
55-
"azure": "Azure",
56-
"gcp": "GCP",
5772
"ssh": "SSH",
5873
"slurm": "SLURM",
5974
"huggingface": "HuggingFace",
60-
"lambda_cloud": "Lambda Cloud",
6175
}
6276

6377
for service, available in credentials.items():
@@ -110,8 +124,7 @@ def run_unit_tests(self) -> bool:
110124
print("✅ Unit tests passed")
111125
return True
112126
else:
113-
print(f"❌ Unit tests failed: {result.stdout}")
114-
print(f"Error: {result.stderr}")
127+
_report_failure("Unit tests", result)
115128
return False
116129
except Exception as e:
117130
print(f"❌ Error running unit tests: {e}")
@@ -137,7 +150,7 @@ def run_filesystem_tests(self) -> bool:
137150
print("✅ Filesystem tests passed")
138151
return True
139152
else:
140-
print(f"❌ Filesystem tests failed: {result.stdout}")
153+
_report_failure("Filesystem tests", result)
141154
return False
142155
except Exception as e:
143156
print(f"❌ Error running filesystem tests: {e}")
@@ -163,7 +176,7 @@ def run_ssh_tests(self) -> bool:
163176
print("✅ SSH tests passed")
164177
return True
165178
else:
166-
print(f"❌ SSH tests failed: {result.stdout}")
179+
_report_failure("SSH tests", result)
167180
return False
168181
except Exception as e:
169182
print(f"❌ Error running SSH tests: {e}")
@@ -192,7 +205,7 @@ def run_api_tests(self, include_expensive: bool = False) -> bool:
192205
print("✅ API tests passed")
193206
return True
194207
else:
195-
print(f"❌ API tests failed: {result.stdout}")
208+
_report_failure("API tests", result)
196209
return False
197210
except Exception as e:
198211
print(f"❌ Error running API tests: {e}")
@@ -220,7 +233,7 @@ def run_visual_tests(self) -> bool:
220233
print(f"📸 Check screenshots in: {self.real_world_dir / 'screenshots'}")
221234
return True
222235
else:
223-
print(f"❌ Visual tests failed: {result.stdout}")
236+
_report_failure("Visual tests", result)
224237
return False
225238
except Exception as e:
226239
print(f"❌ Error running visual tests: {e}")
@@ -247,7 +260,7 @@ def run_hybrid_tests(self) -> bool:
247260
print("✅ Hybrid tests passed")
248261
return True
249262
else:
250-
print(f"❌ Hybrid tests failed: {result.stdout}")
263+
_report_failure("Hybrid tests", result)
251264
return False
252265
except Exception as e:
253266
print(f"❌ Error running hybrid tests: {e}")
@@ -282,7 +295,7 @@ def run_all_tests(
282295
print("✅ All tests passed")
283296
return True
284297
else:
285-
print(f"❌ Some tests failed: {result.stdout}")
298+
_report_failure("Tests", result)
286299
return False
287300
except Exception as e:
288301
print(f"❌ Error running tests: {e}")
@@ -375,26 +388,37 @@ def main():
375388
runner.run_demo()
376389
return
377390

391+
# Every return value below is collected. Dropping them is what made the
392+
# pre-push hook incapable of blocking a push (issue #147): each category
393+
# printed "failed" and the script still exited 0, so the hook's
394+
# `if ! python scripts/run_real_world_tests.py --filesystem` never fired.
395+
outcomes = []
396+
378397
if args.unit:
379-
runner.run_unit_tests()
398+
outcomes.append(runner.run_unit_tests())
380399

381400
if args.filesystem:
382-
runner.run_filesystem_tests()
401+
outcomes.append(runner.run_filesystem_tests())
383402

384403
if args.ssh:
385-
runner.run_ssh_tests()
404+
outcomes.append(runner.run_ssh_tests())
386405

387406
if args.api:
388-
runner.run_api_tests(include_expensive=args.expensive)
407+
outcomes.append(runner.run_api_tests(include_expensive=args.expensive))
389408

390409
if args.visual:
391-
runner.run_visual_tests()
410+
outcomes.append(runner.run_visual_tests())
392411

393412
if args.hybrid:
394-
runner.run_hybrid_tests()
413+
outcomes.append(runner.run_hybrid_tests())
395414

396415
if args.all:
397-
runner.run_all_tests(include_expensive=args.expensive, include_visual=True)
416+
outcomes.append(
417+
runner.run_all_tests(include_expensive=args.expensive, include_visual=True)
418+
)
419+
420+
if outcomes and not all(outcomes):
421+
sys.exit(1)
398422

399423
if not any(
400424
[

0 commit comments

Comments
 (0)