mirror of
https://github.com/priyanshujain/sanderling.git
synced 2026-10-04 12:07:09 +00:00
refactor(runner): one concurrent screenshot per step
Move screenshot capture into the post-action errgroup so it observes the same UI moment as the hierarchy fetch. Drop the pre-action and deferred -after captures. Skip WaitForIdle when the action is Wait since the wait itself provides settling time.
This commit is contained in:
1 parent
095b58afcf
commit
fb69c1b774
2 files changed
+60
-46
No files matched your search
+18
-34
@@ -63,8 +63,6 @@ func Run(ctx context.Context, options Options) (Summary, error) {
|
|||||||
stepIndex := 0
|
stepIndex := 0
|
||||||
var lastAction *verifier.Action
|
var lastAction *verifier.Action
|
||||||
var lastLogTime time.Time
|
var lastLogTime time.Time
|
||||||
var pendingPostScreenshotStep int
|
|
||||||
pendingPostScreenshot := false
|
|
||||||
for time.Now().Before(deadline) {
|
for time.Now().Before(deadline) {
|
||||||
if err := ctx.Err(); err != nil {
|
if err := ctx.Err(); err != nil {
|
||||||
break
|
break
|
||||||
@@ -118,14 +116,10 @@ func Run(ctx context.Context, options Options) (Summary, error) {
|
|||||||
return nil
|
return nil
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
if pendingPostScreenshot {
|
g.Go(func() error {
|
||||||
postStep := pendingPostScreenshotStep
|
captureScreenshot(gctx, options, logger, si)
|
||||||
g.Go(func() error {
|
return nil
|
||||||
captureScreenshot(gctx, options, logger, postStep, true)
|
})
|
||||||
return nil
|
|
||||||
})
|
|
||||||
pendingPostScreenshot = false
|
|
||||||
}
|
|
||||||
// All goroutines write to local variables and return nil, so the Wait
|
// All goroutines write to local variables and return nil, so the Wait
|
||||||
// error is always nil; ignored intentionally.
|
// error is always nil; ignored intentionally.
|
||||||
_ = g.Wait()
|
_ = g.Wait()
|
||||||
@@ -205,7 +199,6 @@ func Run(ctx context.Context, options Options) (Summary, error) {
|
|||||||
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)
|
||||||
}
|
}
|
||||||
captureScreenshot(ctx, options, logger, stepIndex, false)
|
|
||||||
summary.Steps = stepIndex
|
summary.Steps = stepIndex
|
||||||
if len(violations) > 0 {
|
if len(violations) > 0 {
|
||||||
summary.Violations = append(summary.Violations, ViolationRecord{
|
summary.Violations = append(summary.Violations, ViolationRecord{
|
||||||
@@ -227,20 +220,17 @@ func Run(ctx context.Context, options Options) (Summary, error) {
|
|||||||
lastAction = nil
|
lastAction = nil
|
||||||
}
|
}
|
||||||
|
|
||||||
idleCtx, idleCancel := context.WithTimeout(ctx, options.IdleTimeout)
|
// Wait actions are themselves a settling: skip the idle poll. Actions
|
||||||
idleErr := options.Driver.WaitForIdle(idleCtx, options.IdleTimeout)
|
// that mutate the UI fall through to WaitForIdle so the next step's
|
||||||
if nextErr == nil {
|
// concurrent fetches observe a stable post-action state.
|
||||||
pendingPostScreenshot = true
|
if nextErr == nil && nextAction.Kind != verifier.ActionKindWait {
|
||||||
pendingPostScreenshotStep = stepIndex
|
idleCtx, idleCancel := context.WithTimeout(ctx, options.IdleTimeout)
|
||||||
|
idleErr := options.Driver.WaitForIdle(idleCtx, options.IdleTimeout)
|
||||||
|
if idleErr != nil && idleCtx.Err() == nil {
|
||||||
|
logger.Warn("wait_for_idle failed", "step", stepIndex, "err", idleErr)
|
||||||
|
}
|
||||||
|
idleCancel()
|
||||||
}
|
}
|
||||||
if idleErr != nil && idleCtx.Err() == nil {
|
|
||||||
logger.Warn("wait_for_idle failed", "step", stepIndex, "err", idleErr)
|
|
||||||
}
|
|
||||||
idleCancel()
|
|
||||||
}
|
|
||||||
|
|
||||||
if pendingPostScreenshot {
|
|
||||||
captureScreenshot(ctx, options, logger, pendingPostScreenshotStep, true)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
summary.EndTime = time.Now()
|
summary.EndTime = time.Now()
|
||||||
@@ -550,23 +540,17 @@ func nextActionFromV8(ctx context.Context, web driver.WebDriver) (verifier.Actio
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func captureScreenshot(ctx context.Context, options Options, logger *slog.Logger, stepIndex int, after bool) {
|
func captureScreenshot(ctx context.Context, options Options, logger *slog.Logger, stepIndex int) {
|
||||||
image, err := options.Driver.Screenshot(ctx)
|
image, err := options.Driver.Screenshot(ctx)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
logger.Warn("screenshot capture failed", "step", stepIndex, "after", after, "err", err)
|
logger.Warn("screenshot capture failed", "step", stepIndex, "err", err)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
if len(image.PNG) == 0 {
|
if len(image.PNG) == 0 {
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
var writeErr error
|
if writeErr := options.TraceWriter.WriteScreenshot(stepIndex, image.PNG); writeErr != nil {
|
||||||
if after {
|
logger.Warn("screenshot write failed", "step", stepIndex, "err", writeErr)
|
||||||
writeErr = options.TraceWriter.WriteScreenshotAfter(stepIndex, image.PNG)
|
|
||||||
} else {
|
|
||||||
writeErr = options.TraceWriter.WriteScreenshot(stepIndex, image.PNG)
|
|
||||||
}
|
|
||||||
if writeErr != nil {
|
|
||||||
logger.Warn("screenshot write failed", "step", stepIndex, "after", after, "err", writeErr)
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -451,7 +451,10 @@ func TestRunner_ParallelFetchCallsAllDriverMethods(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
func TestRunner_PipelinedPostScreenshotWritten(t *testing.T) {
|
// TestRunner_OneScreenshotPerStep verifies the runner writes a single
|
||||||
|
// screenshot per step, captured concurrently with hierarchy so the two
|
||||||
|
// observations describe the same UI moment.
|
||||||
|
func TestRunner_OneScreenshotPerStep(t *testing.T) {
|
||||||
state := newHarness(t)
|
state := newHarness(t)
|
||||||
state.mock.ImageData = driver.Image{PNG: []byte("fakepng"), Width: 100, Height: 200}
|
state.mock.ImageData = driver.Image{PNG: []byte("fakepng"), Width: 100, Height: 200}
|
||||||
|
|
||||||
@@ -468,24 +471,51 @@ func TestRunner_PipelinedPostScreenshotWritten(t *testing.T) {
|
|||||||
t.Fatalf("Run: %v", err)
|
t.Fatalf("Run: %v", err)
|
||||||
}
|
}
|
||||||
if summary.Steps < 2 {
|
if summary.Steps < 2 {
|
||||||
t.Fatalf("need at least 2 steps for pipelining test, got %d", summary.Steps)
|
t.Fatalf("need at least 2 steps for screenshot test, got %d", summary.Steps)
|
||||||
}
|
}
|
||||||
|
|
||||||
screenshotDir := filepath.Join(state.writer.Directory(), "screenshots")
|
screenshotDir := filepath.Join(state.writer.Directory(), "screenshots")
|
||||||
|
for step := 1; step <= summary.Steps; step++ {
|
||||||
preFile := filepath.Join(screenshotDir, "step-00001.png")
|
path := filepath.Join(screenshotDir, fmt.Sprintf("step-%05d.png", step))
|
||||||
if _, err := os.Stat(preFile); os.IsNotExist(err) {
|
if _, err := os.Stat(path); os.IsNotExist(err) {
|
||||||
t.Errorf("expected pre-screenshot for step 1: %s", preFile)
|
t.Errorf("expected screenshot for step %d at %s", step, path)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
postFile := filepath.Join(screenshotDir, "step-00001-after.png")
|
entries, err := os.ReadDir(screenshotDir)
|
||||||
if _, err := os.Stat(postFile); os.IsNotExist(err) {
|
if err != nil {
|
||||||
t.Errorf("expected pipelined post-screenshot for step 1: %s", postFile)
|
t.Fatal(err)
|
||||||
}
|
}
|
||||||
|
for _, entry := range entries {
|
||||||
|
if strings.Contains(entry.Name(), "-after") {
|
||||||
|
t.Errorf("unexpected -after screenshot remains: %s", entry.Name())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
lastAfter := filepath.Join(screenshotDir, fmt.Sprintf("step-%05d-after.png", summary.Steps))
|
// TestRunner_WaitActionSkipsIdle ensures the runner does not call WaitForIdle
|
||||||
if _, err := os.Stat(lastAfter); os.IsNotExist(err) {
|
// after a Wait action - the action already provides settling time.
|
||||||
t.Errorf("expected flushed post-screenshot for last step %d: %s", summary.Steps, lastAfter)
|
func TestRunner_WaitActionSkipsIdle(t *testing.T) {
|
||||||
|
const waitSpec = `
|
||||||
|
globalThis.actions = __sanderling__.actions(() => [__sanderling__.wait({ durationMillis: 5 })]);
|
||||||
|
`
|
||||||
|
state := newHarnessWithSpec(t, waitSpec)
|
||||||
|
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
|
||||||
|
defer cancel()
|
||||||
|
_, err := Run(ctx, Options{
|
||||||
|
Duration: 150 * time.Millisecond,
|
||||||
|
IdleTimeout: 50 * time.Millisecond,
|
||||||
|
Driver: state.mock,
|
||||||
|
Verifier: state.verifier,
|
||||||
|
TraceWriter: state.writer,
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("Run: %v", err)
|
||||||
|
}
|
||||||
|
for _, action := range state.mock.Actions() {
|
||||||
|
if action.Kind == mockdriver.ActionWaitForIdle {
|
||||||
|
t.Fatalf("Wait action must skip WaitForIdle, got: %v", action)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in new issue
Block a user