test(workflows): fill test gaps from code review of 4xx stale-restore fix#6948
Merged
facumenzella merged 1 commit intoJun 8, 2026
Conversation
- testGetWorkflowsListCallsOnCompleteOnClientError: verifies onComplete() fires on 4xx even though disk restore is skipped (callers must not block) - testGetWorkflowsListRestoresDetailsFromDiskOnServerError: verifies that on 5xx both list mapping AND workflow details are restored from disk, not just the list mapping Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
27de0b8
into
facu/workflow-list-4xx-no-stale
18 of 20 checks passed
facumenzella
added a commit
that referenced
this pull request
Jun 8, 2026
…the request (#6946) * fix(workflows): don't restore stale workflows from disk on a 4xx The getWorkflowsList failure branch restored the cached list + details on any error. That's right for transient errors (5xx / offline) but wrong for a 4xx, where the backend is authoritatively rejecting the request (workflows disabled, unauthorized), so we shouldn't serve stale prefetched data. Generalize the existing offerings classification into BackendError/NetworkError.shouldFallBackToCache (the offerings property now delegates to it, no behavior change) and gate the workflows disk restore on it. 4xx skips the restore; 5xx and connection failures still restore. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test(workflows): fill gaps found in code review of 4xx stale-restore fix (#6948) - testGetWorkflowsListCallsOnCompleteOnClientError: verifies onComplete() fires on 4xx even though disk restore is skipped (callers must not block) - testGetWorkflowsListRestoresDetailsFromDiskOnServerError: verifies that on 5xx both list mapping AND workflow details are restored from disk, not just the list mapping Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> * refactor: drop redundant shouldFallBackToCachedOfferings alias Per review: shouldFallBackToCachedOfferings was only a thin delegate to shouldFallBackToCache after the prior refactor. Remove it on NetworkError and BackendError and point callers (OfferingsManager, tests) at shouldFallBackToCache directly. Behavior-preserving. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Cesar de la Vega <664544+vegaro@users.noreply.github.com>
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
2 participants
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.
Checklist
purchases-androidand hybridsMotivation
Code review of # surfaced two test gaps in the 4xx stale-restore fix:
testGetWorkflowsListDoesNotRestoreFromDiskOnClientErrornever assertedonComplete()fires on 4xx — if that call were accidentally dropped, callers would hang with no test catching it.testGetWorkflowsListRestoresFromDiskOnServerErroronly checked that the list mapping was restored on 5xx, but not that workflow details were also restored from disk.Description
Adds two targeted tests to close those gaps:
testGetWorkflowsListCallsOnCompleteOnClientError: verifiesonComplete()fires on a 4xx even though disk restore is skipped.testGetWorkflowsListRestoresDetailsFromDiskOnServerError: verifies that on 5xx both the list mapping and workflow details are restored from disk, not just the list.Note
Low Risk
Test-only changes with no production code modifications.
Overview
Adds two unit tests in
WorkflowManagerTeststo cover behaviors from the workflows 4xx stale-restore fix that were missing from review.testGetWorkflowsListRestoresDetailsFromDiskOnServerErrorasserts that on a 5xx workflows-list failure, prefetched workflow details on disk are restored intoWorkflowsCache, not only the offering→workflow list mapping (which an existing test already covered).testGetWorkflowsListCallsOnCompleteOnClientErrorasserts thatonCompletestill runs on a 4xx when disk restore is skipped, so callers such as offerings delivery are not left waiting indefinitely.Reviewed by Cursor Bugbot for commit 5e3f924. Bugbot is set up for automated code reviews on this repo. Configure here.