agoda-com / agoda-com/AgodaAnalyzers

AG0053: Detect Playwright Snapshot/Screenshot Assertions Without Preceding Explicit Waits

Open
#224 0 comments 0 reactions 0 assignees View on GitHub
enhancement New Rule
Dominant language
C#
Stars
25
Forks
15
PR merge metrics
No merged PRs in 30d

Description

## Overview

Create a Roslyn analyzer rule to detect Playwright snapshot or screenshot assertions that are not preceded by an explicit element wait (e.g. `ToBeVisibleAsync`, `WaitForAsync`, `WaitForSelectorAsync`). Taking a snapshot before the DOM is fully rendered is a top source of visual test flakiness.

## Background & Motivation

From a recent flakey test analysis, the pattern of **"assert or snapshot before the DOM is ready"** was identified as the most common Playwright flakiness pattern. Tests that take screenshots or compare visual snapshots without first ensuring the target elements are rendered and stable produce intermittent failures.

The fix is always the same: add an explicit wait for element visibility or stability before taking the snapshot. This rule would catch the problem at code review time rather than after CI failures.

## Detection Strategy

Detect calls to Playwright screenshot/snapshot methods:
- `ScreenshotAsync()`
- `Expect(...).ToHaveScreenshotAsync()`
- `Expect(...).ToMatchSnapshotAsync()` (if using snapshot extensions)

Then check whether there is a preceding explicit wait within the same method scope:
- `ToBeVisibleAsync()`
- `WaitForAsync()`
- `WaitForSelectorAsync()`
- `Locator(...).WaitForAsync()`

If no preceding wait is found, report a diagnostic.

## Bad Examples

```csharp
// BAD: Screenshot without any explicit wait
[Test]
public async Task Dashboard_should_match_baseline()
{
await _page.GotoAsync("/dashboard");

// No wait — animations may be in-flight, lazy content not loaded
var screenshot = await _page.ScreenshotAsync();
await Expect(_page).ToHaveScreenshotAsync();
}

// BAD: Snapshot after GotoAsync but no element wait
[Test]
public async Task Profile_page_visual_regression()
{
await _page.GotoAsync("/profile");
await _page.WaitForLoadStateAsync(LoadState.DOMContentLoaded);

// DOMContentLoaded doesn't guarantee all async components are rendered
await Expect(_page).ToHaveScreenshotAsync();
}

// BAD: Component snapshot without waiting for component to render
[Test]
public async Task Modal_should_render_correctly()
{
await _page.GetByTestId("open-modal").ClickAsync();

// Modal has enter animation — snapshot may capture mid-animation state
var modal = _page.GetByTestId("modal-container");
await Expect(modal).ToHaveScreenshotAsync();
}

// BAD: Using networkidle (unreliable) before snapshot
[Test]
public async Task Settings_page_visual_test()
{
await _page.GotoAsync("/settings");
await _page.WaitForLoadStateAsync(LoadState.NetworkIdle); // unreliable!

await Expect(_page).ToHaveScreenshotAsync();
}
```

## Good Examples

```csharp
// GOOD: Wait for specific content element before snapshot
[Test]
public async Task Dashboard_should_match_baseline()
{
await _page.GotoAsync("/dashboard");

// Wait for the main content to be visible and stable
await Expect(_page.GetByTestId("dashboard-loaded")).ToBeVisibleAsync();

await Expect(_page).ToHaveScreenshotAsync();
}

// GOOD: Wait for animation to complete before snapshot
[Test]
public async Task Modal_should_render_correctly()
{
await _page.GetByTestId("open-modal").ClickAsync();

var modal = _page.GetByTestId("modal-container");
await Expect(modal).ToBeVisibleAsync();
// Optional: wait for animation to settle
await modal.WaitForAsync(new() { State = WaitForSelectorState.Stable });

await Expect(modal).ToHaveScreenshotAsync();
}

// GOOD: Wait for specific data to load before snapshot
[Test]
public async Task Profile_page_visual_regression()
{
await _page.GotoAsync("/profile");

// Wait for the profile data to be rendered
await Expect(_page.GetByTestId("profile-name")).ToBeVisibleAsync();
await Expect(_page.GetByTestId("profile-avatar")).ToBeVisibleAsync();

await Expect(_page).ToHaveScreenshotAsync();
}

// GOOD: Use locator-level screenshot with preceding wait
[Test]
public async Task Chart_component_visual_test()
{
await _page.GotoAsync("/analytics");

var chart = _page.GetByTestId("revenue-chart");
await Expect(chart).ToBeVisibleAsync();

await Expect(chart).ToHaveScreenshotAsync();
}
```

## Implementation Notes

- **Rule ID**: `AG0053`
- **Category**: Test Quality / Playwright
- **Default Severity**: Warning
- **Detection approach**:
- Register for `InvocationExpression` syntax nodes
- When a screenshot/snapshot method is found, walk backwards through preceding statements in the same method to check for explicit wait calls
- Consider data flow: the wait should be on the same page/locator (or a parent) as the screenshot
- **Known complexity**:
- Waits may be in helper methods (hard to trace cross-method)
- The wait and screenshot may target different locators but share the same page
- Start with same-method analysis; cross-method analysis can be a future enhancement
- **False positive mitigation**: If `ToBeVisibleAsync` or `WaitForAsync` appears anywhere before the snapshot call in the same method, consider it sufficient (conservative approach)

## Root Cause Data

This rule addresses **Category C4 (Visual / snapshot timing)** and overlaps with **C1 (Race condition / timing)** from flakey test analysis. Key patterns observed:
- Components with enter/exit animations not rendered when snapshot is taken
- Icons loaded from CDN missing from snapshots
- Font loading timeouts causing visual differences
- Lazy-loaded content not present at snapshot time

## References

- AG0040 (`WaitUntilState.NetworkIdle` prevention)
- AG0049 (`WaitForResponseAsync` prevention)
- [Playwright Visual Comparisons docs](https://playwright.dev/dotnet/docs/test-snapshots)
- Flakey Test Root Cause Classification — C4 (Visual / snapshot timing)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.