test(app): pin the template install-failure polarity as a pure function (#10074)#10170
Open
aryanorastar wants to merge 4 commits into
Open
test(app): pin the template install-failure polarity as a pure function (#10074)#10170aryanorastar wants to merge 4 commits into
aryanorastar wants to merge 4 commits into
Conversation
…on (BasedHardware#10074) BasedHardware#10100 fixed the quick-template sheet so a failed install is reported as an error instead of a false success, but shipped without a regression test: the decision was inline in the widget's post-await branch, which a hermetic test can't reach without a full widget harness (five stubbed API calls plus path_provider channels). This extracts only that decision — the success/error polarity of the terminal outcome — into templateCreationOutcomeIsError, a pure function the sheet now derives its install-branch snackbar from. Orchestration, analytics timing, getApps ordering, and the mounted gating are all byte-identical to BasedHardware#10100; the only change is that the install branch sources its success/error choice from the tested classifier instead of an inline bool, so the failed-install-reads-as-success regression cannot be silently reintroduced. Tests pin the full outcome table, including installFailed -> error (the BasedHardware#10074 guarantee) and installed/createdWithoutInstall -> success. Pure unit tests, no widget pumping, no plugin channels. Not run locally (no Flutter SDK on this machine); hermetic, app CI lane runs them. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Replace hardcoded datetime(2026, 6, 20, 12, 0, tzinfo=timezone.utc) with datetime.now(timezone.utc) so the test does not fail as wall-clock time advances past the frozen date.
Collaborator
|
Nice follow-up to #10100 — this cleanly closes the deferred-test gap: the install-failure polarity is now a pure, hermetically-testable function, and the sheet derives its snackbar from it. What I verified:
Small, cohesive, low-risk. I am leaving a positive signal rather than formal approval (automation is gated from approving on this one); a maintainer can merge this with confidence. Thanks @aryanorastar for closing the test gap properly. by AI on behalf of David — if you need David’s attention urgently, please @Git-on-my-level and escalate with |
The polarity assignment fit within the 120-col limit; dart format collapses the manual line break. Fixes the Formatting check.
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 free
to 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
The regression test #10100 deferred. #10100 fixed the quick-template sheet so a failed install is surfaced as an error instead of a false success (#10074), but shipped without a regression test — the decision lived inline in the widget's post-
awaitbranch, unreachable by a hermetic test without a full widget harness (five stubbed API calls pluspath_providerplatform channels).This extracts only that decision — the success/error polarity of the terminal outcome — into
templateCreationOutcomeIsError, a pure function the sheet now derives its install-branch snackbar from. Everything else is byte-identical to #10100.TemplateCreationOutcomeenum +templateCreationOutcomeIsError(one pure file, no Flutter deps).bool; the arm order flips (error-first) but the behavior is identical.installFailed → error— the Bug: summary template set as standard/default is not being applied #10074 guarantee — so the failed-install-reads-as-success regression cannot be silently reintroduced.Behavior identity (verified line-by-line)
Diff is the import line + the install-branch arm-swap; nothing else in
_createTemplatechanged. Confirmed unchanged vs the #10100 baseline:quickTemplateCreatedanalytics timing (still fires on submit success, before install),getApps()ordering (still before install), theif (mounted && createdApp != null)gating (so a mid-flow sheet dismissal still cancels install exactly as before), thecreatedWithoutInstall/submitFailedbranches, and the catch block.success → installed → not error → appCreatedAndInstalled + summarized-apps sheet!success → installFailed → error → failedToInstallApp(provider already showed its dialog)Failure class
Failure-Class: none
test:commit — adds the regression coverage a priorfix:(#10100) deferred; no new triggering defect.Product invariants affected
none (verified with
scripts/pr-preflight --suggest)Validation
app/test/unit/template_creation_outcome_test.dart) pin the table:installFailed → trueandsubmitFailed → true(errors);installed → falseandcreatedWithoutInstall → false(success); plus an exhaustiveness assertion that exactly{submitFailed, installFailed}read as errors. Each calls the exact function production calls — flipping the classifier fails the test.use_build_context_synchronouslyregression,createdApp.namenon-null promotion intact) independently reviewed against the fix(app): stop reporting a failed template install as success #10100 baseline before opening.make preflight(local lane, this PR body) → all selected checks pass.Out of scope, named
submitFailed/createdWithoutInstalloutcomes are classified and tested but the sheet's other two branches still snackbar inline (they were never the Bug: summary template set as standard/default is not being applied #10074 surface and have a single, unconditional polarity). Routing them through the classifier too would be ceremony, not safety.getApps()ordering, and mid-dismiss install behavior for no gain over this focused pin.