mirror of
https://github.com/priyanshujain/sanderling.git
synced 2026-10-02 19:17:10 +00:00
fix(runner): skip verifier for transitional trees after retry budget
When fetchSyncedState exits its retry loop with a tree that still shows a NavHost cross-fade, the runner now marks the step transitional, writes the step + screenshot to the trace, and skips Verifier.PushSnapshot / EvaluateProperties / ChangedExtractors so the previous-to-current extractor advance is not poisoned by transient state. The next clean step's previous still references the prior clean state. NextAction continues to run so the loop never deadlocks on a never-stabilizing screen.
This commit is contained in:
1 parent
2d2c11f830
commit
d83b2f8da1
1 file changed
+56
-34
+56
-34
@@ -83,10 +83,11 @@ func Run(ctx context.Context, options Options) (Summary, error) {
|
|||||||
lastAction = nil
|
lastAction = nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// Hierarchy, metrics, and logs are independent device reads — run
|
// Hierarchy, metrics, and logs are independent device reads. Run
|
||||||
// them concurrently so metrics+logs hide behind the hierarchy fetch.
|
// them concurrently so metrics+logs hide behind the hierarchy fetch.
|
||||||
var tree *hierarchy.Tree
|
var tree *hierarchy.Tree
|
||||||
var hierarchyErr error
|
var hierarchyErr error
|
||||||
|
var transitional bool
|
||||||
var metrics *trace.Metrics
|
var metrics *trace.Metrics
|
||||||
var logs []verifier.LogEntry
|
var logs []verifier.LogEntry
|
||||||
|
|
||||||
@@ -100,7 +101,7 @@ func Run(ctx context.Context, options Options) (Summary, error) {
|
|||||||
// screenshot describe the same frame, then re-fetches the pair
|
// screenshot describe the same frame, then re-fetches the pair
|
||||||
// while the tree still looks transitional.
|
// while the tree still looks transitional.
|
||||||
g.Go(func() error {
|
g.Go(func() error {
|
||||||
tree, hierarchyErr = fetchSyncedState(gctx, options, logger, si)
|
tree, transitional, hierarchyErr = fetchSyncedState(gctx, options, logger, si)
|
||||||
return nil
|
return nil
|
||||||
})
|
})
|
||||||
g.Go(func() error {
|
g.Go(func() error {
|
||||||
@@ -140,36 +141,52 @@ func Run(ctx context.Context, options Options) (Summary, error) {
|
|||||||
}
|
}
|
||||||
lastLogTime = stepStart
|
lastLogTime = stepStart
|
||||||
|
|
||||||
if err := options.Verifier.PushSnapshot(verifier.SnapshotInput{
|
|
||||||
Tree: tree,
|
|
||||||
LastAction: lastAction,
|
|
||||||
StepTime: stepStart,
|
|
||||||
RunStart: summary.StartTime,
|
|
||||||
Logs: logs,
|
|
||||||
}); err != nil {
|
|
||||||
return summary, fmt.Errorf("step %d push: %w", stepIndex, err)
|
|
||||||
}
|
|
||||||
skipped, overrideErr := options.Verifier.OverrideExtractorValues(v8Overrides)
|
|
||||||
if overrideErr != nil {
|
|
||||||
logger.Warn("v8 override apply failed", "step", stepIndex, "err", overrideErr)
|
|
||||||
}
|
|
||||||
if skipped > 0 {
|
|
||||||
logger.Warn("v8 override skipped out-of-range entries",
|
|
||||||
"step", stepIndex, "skipped", skipped, "have", len(v8Overrides))
|
|
||||||
}
|
|
||||||
|
|
||||||
screen := ""
|
screen := ""
|
||||||
if tree != nil && len(tree.Elements) > 0 {
|
if tree != nil && len(tree.Elements) > 0 {
|
||||||
screen = tree.Elements[0].Screen
|
screen = tree.Elements[0].Screen
|
||||||
}
|
}
|
||||||
logger.Info("step", "index", stepIndex, "screen", screen, "nodes", treeSize)
|
|
||||||
options.Verifier.EvaluateProperties()
|
// Transitional trees describe a NavHost mid cross-fade. Pushing
|
||||||
violations := options.Verifier.NewlyViolatedProperties()
|
// one would poison the verifier's previous/current extractor
|
||||||
for _, name := range violations {
|
// advance, so the next clean step would compare against this
|
||||||
if predicateErr := options.Verifier.PredicateError(name); predicateErr != nil {
|
// transient state and emit false-positive violations. We still
|
||||||
logger.Warn("predicate error", "step", stepIndex, "property", name, "err", predicateErr)
|
// record the step (hierarchy + screenshot) for inspect-side
|
||||||
|
// debugging, but skip the verifier entirely and pick the next
|
||||||
|
// action against the unchanged prior state to keep the loop
|
||||||
|
// progressing.
|
||||||
|
var violations []string
|
||||||
|
var extractorChanges map[string]trace.ExtractorChange
|
||||||
|
if !transitional {
|
||||||
|
if err := options.Verifier.PushSnapshot(verifier.SnapshotInput{
|
||||||
|
Tree: tree,
|
||||||
|
LastAction: lastAction,
|
||||||
|
StepTime: stepStart,
|
||||||
|
RunStart: summary.StartTime,
|
||||||
|
Logs: logs,
|
||||||
|
}); err != nil {
|
||||||
|
return summary, fmt.Errorf("step %d push: %w", stepIndex, err)
|
||||||
}
|
}
|
||||||
|
skipped, overrideErr := options.Verifier.OverrideExtractorValues(v8Overrides)
|
||||||
|
if overrideErr != nil {
|
||||||
|
logger.Warn("v8 override apply failed", "step", stepIndex, "err", overrideErr)
|
||||||
|
}
|
||||||
|
if skipped > 0 {
|
||||||
|
logger.Warn("v8 override skipped out-of-range entries",
|
||||||
|
"step", stepIndex, "skipped", skipped, "have", len(v8Overrides))
|
||||||
|
}
|
||||||
|
options.Verifier.EvaluateProperties()
|
||||||
|
violations = options.Verifier.NewlyViolatedProperties()
|
||||||
|
for _, name := range violations {
|
||||||
|
if predicateErr := options.Verifier.PredicateError(name); predicateErr != nil {
|
||||||
|
logger.Warn("predicate error", "step", stepIndex, "property", name, "err", predicateErr)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
extractorChanges = encodeExtractorChanges(options.Verifier.ChangedExtractors())
|
||||||
|
} else {
|
||||||
|
logger.Warn("transitional tree after retry budget; skipping verifier",
|
||||||
|
"step", stepIndex, "screen", screen, "nodes", treeSize)
|
||||||
}
|
}
|
||||||
|
logger.Info("step", "index", stepIndex, "screen", screen, "nodes", treeSize)
|
||||||
|
|
||||||
var nextAction verifier.Action
|
var nextAction verifier.Action
|
||||||
var nextErr error
|
var nextErr error
|
||||||
@@ -199,7 +216,8 @@ func Run(ctx context.Context, options Options) (Summary, error) {
|
|||||||
Hierarchy: tree,
|
Hierarchy: tree,
|
||||||
Residuals: residuals,
|
Residuals: residuals,
|
||||||
Metrics: metrics,
|
Metrics: metrics,
|
||||||
ExtractorChanges: encodeExtractorChanges(options.Verifier.ChangedExtractors()),
|
ExtractorChanges: extractorChanges,
|
||||||
|
Transitional: transitional,
|
||||||
}
|
}
|
||||||
if err := options.TraceWriter.WriteStep(step); err != nil {
|
if err := options.TraceWriter.WriteStep(step); err != nil {
|
||||||
return summary, fmt.Errorf("step %d trace: %w", stepIndex, err)
|
return summary, fmt.Errorf("step %d trace: %w", stepIndex, err)
|
||||||
@@ -503,24 +521,28 @@ const (
|
|||||||
// The driver's Snapshot RPC captures both reads under a backend-side mutex
|
// The driver's Snapshot RPC captures both reads under a backend-side mutex
|
||||||
// so they describe the same on-device frame; the retry exists for the
|
// so they describe the same on-device frame; the retry exists for the
|
||||||
// orthogonal case where the frame itself is transitional.
|
// orthogonal case where the frame itself is transitional.
|
||||||
func fetchSyncedState(ctx context.Context, options Options, logger *slog.Logger, stepIndex int) (*hierarchy.Tree, error) {
|
//
|
||||||
var tree *hierarchy.Tree
|
// The transitional return reports whether the retry budget was exhausted
|
||||||
var hierarchyErr error
|
// on a still-transitional tree. Callers use it to skip the verifier for
|
||||||
|
// that step so the previous/current extractor advance does not absorb
|
||||||
|
// transient state.
|
||||||
|
func fetchSyncedState(ctx context.Context, options Options, logger *slog.Logger, stepIndex int) (tree *hierarchy.Tree, transitional bool, err error) {
|
||||||
var pngBytes []byte
|
var pngBytes []byte
|
||||||
retryLoop:
|
retryLoop:
|
||||||
for attempt := range transitionalRetryAttempts {
|
for attempt := range transitionalRetryAttempts {
|
||||||
hierarchyJSON, image, snapshotErr := options.Driver.Snapshot(ctx)
|
hierarchyJSON, image, snapshotErr := options.Driver.Snapshot(ctx)
|
||||||
if snapshotErr != nil {
|
if snapshotErr != nil {
|
||||||
hierarchyErr = snapshotErr
|
err = snapshotErr
|
||||||
tree = nil
|
tree = nil
|
||||||
} else {
|
} else {
|
||||||
tree, hierarchyErr = hierarchy.Parse(hierarchyJSON)
|
tree, err = hierarchy.Parse(hierarchyJSON)
|
||||||
pngBytes = image.PNG
|
pngBytes = image.PNG
|
||||||
}
|
}
|
||||||
if hierarchyErr != nil || !isTransitionalHierarchy(tree) {
|
if err != nil || !isTransitionalHierarchy(tree) {
|
||||||
break
|
break
|
||||||
}
|
}
|
||||||
if attempt == transitionalRetryAttempts-1 {
|
if attempt == transitionalRetryAttempts-1 {
|
||||||
|
transitional = true
|
||||||
break
|
break
|
||||||
}
|
}
|
||||||
timer := time.NewTimer(transitionalRetrySleep)
|
timer := time.NewTimer(transitionalRetrySleep)
|
||||||
@@ -536,7 +558,7 @@ retryLoop:
|
|||||||
logger.Warn("screenshot write failed", "step", stepIndex, "err", writeErr)
|
logger.Warn("screenshot write failed", "step", stepIndex, "err", writeErr)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
return tree, hierarchyErr
|
return tree, transitional, err
|
||||||
}
|
}
|
||||||
|
|
||||||
// isTransitionalHierarchy returns true when the tree carries more than one
|
// isTransitionalHierarchy returns true when the tree carries more than one
|
||||||
|
|||||||
Reference in new issue
Block a user