diff --git a/internal/runner/runner.go b/internal/runner/runner.go index 725ca25..27fdd86 100644 --- a/internal/runner/runner.go +++ b/internal/runner/runner.go @@ -7,6 +7,7 @@ import ( "fmt" "log/slog" "strings" + "sync" "time" "golang.org/x/sync/errgroup" @@ -90,11 +91,14 @@ func Run(ctx context.Context, options Options) (Summary, error) { // goroutine, whose CDP round-trip can otherwise outrun the step // budget on a hung tab. g, gctx := errgroup.WithContext(ctx) + si := stepIndex + // fetchSyncedState pairs hierarchy and screenshot in the same goroutine + // so when a retry happens on a transitional tree, the screenshot stays + // aligned with the final hierarchy snapshot. g.Go(func() error { - tree, hierarchyErr = fetchHierarchy(gctx, options.Driver) + tree, hierarchyErr = fetchSyncedState(gctx, options, logger, si) return nil }) - si := stepIndex g.Go(func() error { metrics = captureMetrics(gctx, options, logger, si) return nil @@ -116,10 +120,6 @@ func Run(ctx context.Context, options Options) (Summary, error) { return nil }) } - g.Go(func() error { - captureScreenshot(gctx, options, logger, si) - return nil - }) // All goroutines write to local variables and return nil, so the Wait // error is always nil; ignored intentionally. _ = g.Wait() @@ -414,6 +414,89 @@ func fetchHierarchy(ctx context.Context, drv driver.DeviceDriver) (*hierarchy.Tr return hierarchy.Parse(xmlText) } +// transitionalRetryAttempts caps how many times we re-fetch hierarchy when a +// tree carries more than one route-level Screen tag (NavHost cross-fade in +// flight). Each retry pauses transitionalRetrySleep before the next fetch. +const ( + transitionalRetryAttempts = 4 + transitionalRetrySleep = 200 * time.Millisecond +) + +// fetchSyncedState fetches hierarchy and screenshot together so the recorded +// pair shows the same UI moment. If the hierarchy looks like a NavHost +// cross-fade (multiple route-level *Screen tags), the function waits briefly +// and re-fetches the pair, up to transitionalRetryAttempts times. This +// handles transitions whose async work begins after the sidecar's settle +// poll has already exited. +func fetchSyncedState(ctx context.Context, options Options, logger *slog.Logger, stepIndex int) (*hierarchy.Tree, error) { + var tree *hierarchy.Tree + var hierarchyErr error + var pngBytes []byte +retryLoop: + for attempt := range transitionalRetryAttempts { + var wg sync.WaitGroup + var localTree *hierarchy.Tree + var localHierErr error + var localImg driver.Image + var localImgErr error + wg.Add(2) + go func() { + defer wg.Done() + localTree, localHierErr = fetchHierarchy(ctx, options.Driver) + }() + go func() { + defer wg.Done() + localImg, localImgErr = options.Driver.Screenshot(ctx) + }() + wg.Wait() + tree = localTree + hierarchyErr = localHierErr + if localImgErr == nil { + pngBytes = localImg.PNG + } + if hierarchyErr != nil || !isTransitionalHierarchy(tree) { + break + } + if attempt == transitionalRetryAttempts-1 { + break + } + timer := time.NewTimer(transitionalRetrySleep) + select { + case <-ctx.Done(): + timer.Stop() + break retryLoop + case <-timer.C: + } + } + if len(pngBytes) > 0 { + if writeErr := options.TraceWriter.WriteScreenshot(stepIndex, pngBytes); writeErr != nil { + logger.Warn("screenshot write failed", "step", stepIndex, "err", writeErr) + } + } + return tree, hierarchyErr +} + +// isTransitionalHierarchy returns true when the tree carries more than one +// resource-id ending in "Screen" - the marker of a Compose NavHost mid +// cross-fade where both source and destination route composables are alive. +// Mirrors the sidecar's stabilitySnapshot heuristic so runner-side rejection +// stays consistent with the settle poll. +func isTransitionalHierarchy(tree *hierarchy.Tree) bool { + if tree == nil { + return false + } + screens := 0 + for _, element := range tree.Elements { + if strings.HasSuffix(element.ResourceID, "Screen") { + screens++ + if screens > 1 { + return true + } + } + } + return false +} + func traceActionFor(action verifier.Action, tree *hierarchy.Tree) *trace.Action { traceAction := &trace.Action{Kind: string(action.Kind), X: action.X, Y: action.Y} switch action.Kind { @@ -541,20 +624,6 @@ func nextActionFromV8(ctx context.Context, web driver.WebDriver) (verifier.Actio } } -func captureScreenshot(ctx context.Context, options Options, logger *slog.Logger, stepIndex int) { - image, err := options.Driver.Screenshot(ctx) - if err != nil { - logger.Warn("screenshot capture failed", "step", stepIndex, "err", err) - return - } - if len(image.PNG) == 0 { - return - } - if writeErr := options.TraceWriter.WriteScreenshot(stepIndex, image.PNG); writeErr != nil { - logger.Warn("screenshot write failed", "step", stepIndex, "err", writeErr) - } -} - func encodeExtractorChanges(changes map[string]verifier.ExtractorChange) map[string]trace.ExtractorChange { if len(changes) == 0 { return nil diff --git a/internal/runner/runner_test.go b/internal/runner/runner_test.go index 637369e..422d215 100644 --- a/internal/runner/runner_test.go +++ b/internal/runner/runner_test.go @@ -17,6 +17,7 @@ import ( "github.com/priyanshujain/sanderling/internal/driver" mockdriver "github.com/priyanshujain/sanderling/internal/driver/mock" + "github.com/priyanshujain/sanderling/internal/hierarchy" "github.com/priyanshujain/sanderling/internal/trace" "github.com/priyanshujain/sanderling/internal/verifier" ) @@ -493,6 +494,34 @@ func TestRunner_OneScreenshotPerStep(t *testing.T) { } } +// TestIsTransitionalHierarchy_DetectsMultipleScreens covers the runner-side +// guard that re-fetches when the hierarchy still carries two route-level +// *Screen ids - the NavHost cross-fade signature. +func TestIsTransitionalHierarchy_DetectsMultipleScreens(t *testing.T) { + multi, err := hierarchy.Parse(`{"attributes":{"resource-id":"root"},"children":[ + {"attributes":{"resource-id":"AddAccountScreen"},"children":[]}, + {"attributes":{"resource-id":"HomeScreen"},"children":[]} + ]}`) + if err != nil { + t.Fatal(err) + } + if !isTransitionalHierarchy(multi) { + t.Error("expected multi-screen tree to be flagged as transitional") + } + + single, err := hierarchy.Parse(`{"attributes":{"resource-id":"HomeScreen"},"children":[]}`) + if err != nil { + t.Fatal(err) + } + if isTransitionalHierarchy(single) { + t.Error("single-screen tree must not be flagged as transitional") + } + + if isTransitionalHierarchy(nil) { + t.Error("nil tree must not be flagged as transitional") + } +} + // TestRunner_WaitActionSkipsIdle ensures the runner does not call WaitForIdle // after a Wait action - the action already provides settling time. func TestRunner_WaitActionSkipsIdle(t *testing.T) {