Uh oh!
There was an error while loading. Please reload this page.
test(submit): cover Run non-detach watch path (root-cause #4) - #259
Merged
Conversation
All six pre-existing submit.Run tests set Detach:true and returned before WatchJob, leaving the entire post-watch surface — the WatchError wrap, the per-DetachReason switch, and the summary-render branch — unexecuted (the last structural root cause from the coverage audit). Add a watchJobFn seam over WatchJob (same pattern as doctorRunFn/listDatasetsFn) so Run's non-detach path is exercisable with a canned WatchResult/error, no live Job needed. Pure seam routing — the package var defaults to the real WatchJob, so there is no production behavior change. Coverage: submit.Run 97.3%, package own-coverage 76.2% -> 80.4%. New tests: - a watch failure is tagged WatchError (exit-9 mapping) and Result.Submit survives; - the per-DetachReason switch renders each timeout's distinct diagnostic + the reconnect hint (mutation guard: pod-wait-timeout must not read as a Ctrl-C stop); - a completed run renders the summary panel, and a nil summary skips it. make ci green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
LukasWodka
commented
Jul 14, 2026
ContributorAuthor
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 3686351. Configure here.
saadqbal
approved these changes
Jul 14, 2026
saadqbal
left a comment
Collaborator
There was a problem hiding this comment.
LGTM 👍 One-line seam, and it unlocks the whole post-watch surface the Detach:true tests never reached. Verified all four DetachReason arms + the WatchError→exit-9 wrap match Run, and the PodWaitTimeout mutation guard (must not read as a Ctrl-C stop) is a good pin given the Bugbot PR#10 history.
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 freeto 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.
Summary
Closes the last structural root-cause from the coverage audit (#4): all six pre-existing
submit.Runtests setDetach:trueand returned beforeWatchJob, so the entire non-detach post-watch surface — theWatchErrorwrap, the per-DetachReasonswitch, and the summary-render branch — was unexecuted.Adds a
watchJobFnseam overWatchJob(same pattern asdoctorRunFn/listDatasetsFn) soRun's non-detach path is exercisable with a cannedWatchResult/error — no live Job to watch. Pure seam routing — the package var defaults to the realWatchJob, so no production behavior change.Coverage
internal/submitown-coverage 76.2% → 80.4%;submit.Run→ 97.3%.New tests:
WatchError(so the orchestrator maps it to exit 9, not 8) andResult.Submitstill survives (the run was already accepted server-side).DetachReasonswitch renders each timeout's distinct diagnostic + the samekubectl logsreconnect hint — with a mutation guard that the pod-wait-timeout path must not read as a Ctrl-C stop (the exact Bugbot PR feat(dataset): Phase 4 — submit to jobs-manager + watch + summary (#152) #10 r7 regression).nilsummary skips it (thewr.Summary != nilfalse branch).Test plan
make cigreen (build, vet, gofmt -s,go test -race -cover, errcheck/ineffassign/misspell, schema-check).fakeSubmitterharness + the newwatchJobFnseam — no real cluster, no watch mechanics (those stay covered bywatch_test.go).🤖 Generated with Claude Code
Note
Low Risk
Test-only changes plus a package-level indirection that still defaults to the real WatchJob; no runtime behavior change.
Overview
Adds a
watchJobFntest seam (defaults toWatchJob) soRunwithDetach:falsecan be tested without a live cluster Job.Runnow invokeswatchJobFninstead of callingWatchJobdirectly; production behavior is unchanged.New tests in
run_watch_test.goexercise the post-submit watch path that priorDetach:truetests skipped:WatchErrorwrapping (exit-9 mapping) withResult.Submitpreserved, per-DetachReasonuser messages plus thekubectl logshint (including a guard that pod-wait-timeout is not worded like Ctrl-C), and summary panel rendering vs skipping whenSummaryis nil.Reviewed by Cursor Bugbot for commit 3686351. Bugbot is set up for automated code reviews on this repo. Configure here.