fix(extensions-cli): fail build and bundle when the frontend build fails - #44407
mikebridge wants to merge 1 commit into
Conversation
Code Review Agent Run #81962dActionable Suggestions - 0Additional Suggestions - 1
Review Details
Bito Usage GuideCommands Type the following command in the pull request comment and save the comment.
Refer to the documentation for additional commands. Configuration This repository uses Documentation & Help |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #44407 +/- ##
==========================================
+ Coverage 80.41% 80.45% +0.03%
==========================================
Files 2932 2937 +5
Lines 174621 175249 +628
Branches 40559 40559
==========================================
+ Hits 140419 140992 +573
- Misses 31535 31590 +55
Partials 2667 2667
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
richardfogaca
left a comment
There was a problem hiding this comment.
Richard's agent here:
Reviewed 7218402767. No substantive issues found; approving.
Verified: bundle reaches the new non-zero exit through ctx.invoke(build), so it exits before reading a stale dist/manifest.json or writing an archive; the failure message is already emitted by rebuild_frontend, so the exit adds no duplicate output; dev is unchanged and its watcher still swallows a failed rebuild and publishes on the next success, which the new watcher test pins. The parametrized test that drives the real run_frontend_build seam for both commands is a good addition, since the old test literally asserted that a failed frontend still produced "Full build completed".
Validation: CI is green on this head.
SUMMARY
Fixes the upstream CLI failure-propagation portion of SC-119473.
A real frontend npm build script (
node -e "process.exit(1)") reproduces false success: bothbuildandbundlereturned exit 0, andbundleproduced a manifest-only.supxafter reporting the frontend failure. With this change both commands return exit 1, without writing a new manifest or archive.The guard belongs at the
buildcall site.rebuild_frontendretains itsNone-on-failure contract so thedevwatcher can recover on a subsequent change. Backend-only builds remain unaffected. Existing archives are retained; callers must check the exit status.Related downstream guard: https://github.com/preset-io/superset-shell/pull/5161. The unbounded
apache-superset-coredependency pin is tracked separately in #42568 and is untouched.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
CLI-only change; no UI screenshots.
buildexit statusbundleexit status.supxTESTING INSTRUCTIONS
From
superset-extensions-cli, runpytest -o addopts='' -q tests/with the local CLI and core packages importable. Result: 275 passed, with 15 existing Click/semver deprecation warnings. All applicable changed-file pre-commit hooks passed, including package MyPy, Ruff and custom pylint.Updated parametrized tests cover failed
buildandbundlecommands and the helper's nonzero compiler results. Two new command-level test functions cover successful/failed compiler propagation through real manifest/archive paths, rejection of stale frontend output on failure, and the registered dev callback recovering through success/failure/success without a live watcher thread.Manual reproduction used an actual failing npm subprocess before and after the fix, not a mocked compiler. Before: exit 0 and a manifest-only archive. After: exit 1 and no new archive. No dependency installations or Docker builds were required.
ADDITIONAL INFORMATION