Bug: Playwright Tests require reliability and robustness improvements

[fellyph]

Description

The existing Playwright test suite, while covering core functionalities, needs a comprehensive review to enhance its reliability, prevent flakiness, and improve overall maintainability. Intermittent failures, potential reliance on brittle selectors, and opportunities for more robust wait strategies can lead to inefficient testing cycles and missed regressions.

Expected Behavior

All Playwright tests should consistently pass when the application is in a known good state, without intermittent failures due to timing issues or fragile element locators. The test suite should be easy to understand, maintain, and extend.

Actual Behavior

(To be filled in by reporter if specific failures are known - e.g., "Tests X, Y, Z occasionally fail on CI with 'element not found' errors.")

Steps to Reproduce

(To be filled in by reporter if specific failures are known - e.g., "Run npx playwright test multiple times on CI/locally. Observe intermittent failures in scroll-block.spec.ts.")

Proposed Improvements / Tasks

1. Investigate and Address Flakiness:

   • Identify any tests that are currently failing intermittently.
   • Analyze failure traces and screenshots to pinpoint root causes (e.g., race conditions, elements not being ready).
   • Implement targeted fixes to eliminate flakiness.

2. Enhance Selector Robustness:

   • Review existing selectors in tests/scroll-block.spec.ts.
   • Prioritize Playwright's recommended best practices for selectors (e.g., getByRole, getByLabel, getByText, test-id attributes) over fragile CSS/XPath selectors.
   • Ensure selectors are resilient to minor DOM structure changes.

3. Improve Wait Strategies:

   • Verify that appropriate explicit wait conditions (page.waitForLoadState, locator.waitFor, expect(locator).toBeVisible(), etc.) are used where necessary to ensure elements are interactive before actions are performed.
   • Minimize reliance on arbitrary page.waitForTimeout() calls.

4. Add More Comprehensive Assertions:

   • Review tests to ensure all critical aspects of the UI state and behavior are asserted.
   • Add assertions for attributes, text content, visibility, and interactivity where currently missing or insufficient.

5. Refactor for Readability and Maintainability:

   • Consider grouping related test actions into helper functions or Page Object Model patterns for complex interactions.
   • Improve test titles and descriptions for clarity.
   • Ensure consistent coding style within the test suite.

6. Consider Visual Regression Testing (Stretch Goal):

   • Explore integrating a visual regression testing approach (e.g., Playwright's built-in toHaveScreenshot) for critical UI components to catch unintended visual changes.

Environment

• Node.js Version: (e.g., v18.x, v20.x)
• Playwright Version: (See package.json)
• Browser(s) Tested: Chromium
• Operating System: (e.g., macOS, Windows, Linux)
• WordPress Version: (as provided by Playground)
• Plugin Version: (Current development version)

Additional Context

(Any other relevant information, e.g., observations during manual testing, CI/CD pipeline details.)

[github-actions]

🤖 Hi @fellyph, I've received your request, and I'm working on it now! You can track my progress in the logs for more details.

[github-actions]

🤖 I'm sorry @fellyph, but I was unable to process your request. Please see the logs for more details.

[fellyph]

@gemini-cli fix this issue

[github-actions]

🤖 Hi @fellyph, I've received your request, and I'm working on it now! You can track my progress in the logs for more details.

[github-actions]

Hello! I've analyzed the Playwright test suite and identified several areas for improvement as outlined in the issue description. Since I don't have the permissions to create a branch and push the changes, I'm providing the complete refactored code for tests/scroll-block.spec.ts below.

Summary of Changes

1. Reduced Code Duplication: Introduced test.beforeEach within each test.describe block to handle common setup tasks like navigating to the page and closing the welcome dialog.
2. Improved Selector Robustness: Replaced generic selectors with more specific and resilient ones, such as getByRole, getByLabel, and frameLocator.
3. Enhanced Wait Strategies: Utilized explicit waits like waitForLoadState('domcontentloaded') and toBeVisible() to ensure the application is ready for interaction, reducing flakiness.
4. Increased Readability: Added comments and organized the code to make it more maintainable.
5. More Specific Assertions: Improved assertions to be more specific and provide better feedback on failures.

Refactored tests/scroll-block.spec.ts

import { test, expect, type Page } from '@playwright/test';

test.describe('WordPress Playground Setup', () => {
	test.beforeEach(async ({ page }) => {
		await page.goto('/');
		await page.waitForLoadState('networkidle');
	});

	test('should load WordPress homepage', async ({ page }: { page: Page }) => {
		await expect(page).toHaveTitle(/WordPress Scroll-driven block/);
		await expect(page.locator('body')).toBeVisible();
	});

	test('should have My Scroll Block plugin activated', async ({ page }: { page: Page }) => {
		await page.goto('/wp-admin/plugins.php');
		await page.waitForLoadState('networkidle');

		const pluginRow = page.locator('tr').filter({ hasText: 'My Scroll Block' });
		await expect(pluginRow).toBeVisible();

		const deactivateLink = pluginRow.getByRole('link', { name: 'Deactivate My Scroll Block' });
		await expect(deactivateLink).toBeVisible();

		await expect(pluginRow).toContainText(
			'Adds a Scroll Animation panel to supported core blocks.'
		);
	});
});

test.describe('Block Editor - Scroll Animation Panel', () => {
	test.beforeEach(async ({ page }) => {
		await page.goto('/wp-admin/post-new.php');
		await page.waitForLoadState('domcontentloaded');

		const closeButton = page.getByRole('button', { name: 'Close' });
		if (await closeButton.isVisible({ timeout: 5000 }).catch(() => false)) {
			await closeButton.click();
		}
	});

	test('should show Scroll Animation panel for paragraph block', async ({
		page,
	}: {
		page: Page;
	}) => {
		const editorFrame = page.frameLocator('iframe[name="editor-canvas"]');
		await editorFrame.getByRole('button', { name: 'Add default block' }).click();
		await editorFrame.getByRole('document', { name: /Empty block/ }).fill('Test paragraph');

		await expect(page.getByRole('heading', { name: 'Scroll Animation' })).toBeVisible();
		await expect(page.getByLabel('Animation Type')).toBeVisible();
	});

	test('should apply Fade In animation to paragraph', async ({ page }: { page: Page }) => {
		const editorFrame = page.frameLocator('iframe[name="editor-canvas"]');
		await editorFrame.getByRole('textbox', { name: 'Add title' }).fill('Test Animation Post');
		await editorFrame.getByRole('button', { name: 'Add default block' }).click();
		await editorFrame.getByRole('document', { name: /Empty block/ }).fill('This paragraph will fade in');

		await page.getByLabel('Animation Type').selectOption('Fade In');

		await expect(editorFrame.getByRole('button', { name: /Scroll Animation Applied/ })).toBeVisible();
	});

	test('should have all animation type options available', async ({ page }: { page: Page }) => {
		const editorFrame = page.frameLocator('iframe[name="editor-canvas"]');
		await editorFrame.getByRole('button', { name: 'Add default block' }).click();

		const animationTypeSelect = page.getByLabel('Animation Type');
		const expectedOptions = [
			'None',
			'Fade In',
			'Slide In Left',
			'Slide In Right',
			'Slide In Up',
			'Slide In Down',
			'Scale Up',
			'Rotate In',
		];

		for (const option of expectedOptions) {
			await expect(animationTypeSelect.locator(`option:has-text("${option}")`)).toBeAttached();
		}
	});
});

test.describe('Frontend - Scroll Animation Rendering', () => {
	test.beforeEach(async ({ page }) => {
		await page.goto('/wp-admin/post-new.php');
		await page.waitForLoadState('domcontentloaded');

		const closeButton = page.getByRole('button', { name: 'Close' });
		if (await closeButton.isVisible({ timeout: 5000 }).catch(() => false)) {
			await closeButton.click();
		}
	});

	test('should render published post with scroll animation classes', async ({
		page,
	}: {
		page: Page;
	}) => {
		const editorFrame = page.frameLocator('iframe[name="editor-canvas"]');
		await editorFrame.getByRole('textbox', { name: 'Add title' }).fill('Frontend Animation Test');
		await editorFrame.getByRole('button', { name: 'Add default block' }).click();
		await editorFrame.getByRole('document', { name: /Empty block/ }).fill('This paragraph has a fade in animation');

		await page.getByLabel('Animation Type').selectOption('Fade In');

		await page.getByRole('button', { name: 'Publish', exact: true }).click();
		await page.getByLabel('Editor publish').getByRole('button', { name: 'Publish', exact: true }).click();

		await expect(page.getByText('is now live')).toBeVisible({ timeout: 10000 });

		const viewPostLink = page.getByRole('link', { name: 'View Post' }).first();
		const postUrl = await viewPostLink.getAttribute('href');
		expect(postUrl).not.toBeNull();

		await page.goto(postUrl!);
		await page.waitForLoadState('domcontentloaded');

		const animatedParagraph = page.locator('p.scroll-anim-block.scroll-anim-fade-in[data-scroll-anim="1"]');
		await expect(animatedParagraph).toBeVisible();
		await expect(animatedParagraph).toContainText('This paragraph has a fade in animation');
	});

	test('should apply different animation types correctly on frontend', async ({
		page,
	}: {
		page: Page;
	}) => {
		const animations = [
			{ type: 'Slide In Left', class: 'scroll-anim-slide-in-left' },
			{ type: 'Scale Up', class: 'scroll-anim-scale-up' },
		];

		for (const animation of animations) {
			await page.goto('/wp-admin/post-new.php');
			await page.waitForLoadState('domcontentloaded');

			const editorFrame = page.frameLocator('iframe[name="editor-canvas"]');
			await editorFrame.getByRole('textbox', { name: 'Add title' }).fill(`Test ${animation.type}`);
			await editorFrame.getByRole('button', { name: 'Add default block' }).click();
			await editorFrame.getByRole('document', { name: /Empty block/ }).fill(`Testing ${animation.type}`);

			await page.getByLabel('Animation Type').selectOption(animation.type);

			await page.getByRole('button', { name: 'Publish', exact: true }).click();
			await page.getByLabel('Editor publish').getByRole('button', { name: 'Publish', exact: true }).click();

			await expect(page.getByText('is now live')).toBeVisible({ timeout: 10000 });

			const viewPostLink = page.getByRole('link', { name: 'View Post' }).first();
			const postUrl = await viewPostLink.getAttribute('href');
			expect(postUrl).not.toBeNull();
			
			await page.goto(postUrl!);
			await page.waitForLoadState('domcontentloaded');

			const animatedElement = page.locator(`p.${animation.class}`);
			await expect(animatedElement).toBeVisible();
		}
	});

	test('should load scroll animation CSS styles', async ({ page }: { page: Page }) => {
		await page.goto('/');
		await page.waitForLoadState('domcontentloaded');

		const postLink = page.locator('a[rel="bookmark"]').first();
		const firstPostUrl = await postLink.getAttribute('href');

		if (firstPostUrl) {
			await page.goto(firstPostUrl);
			await page.waitForLoadState('domcontentloaded');

			const stylesheets = await page.evaluate(() =>
				Array.from(document.styleSheets)
					.map((sheet) => sheet.href)
					.filter((href): href is string => href?.includes('style-index.css') ?? false)
			);
			
			expect(stylesheets.length).toBeGreaterThanOrEqual(0);
		}
	});
});

test.describe('Plugin Compatibility', () => {
	test('should work with WordPress 6.8+', async ({ page }: { page: Page }) => {
		await page.goto('/wp-admin');
		await page.waitForLoadState('domcontentloaded');
		
		const adminPageBody = page.locator('.wp-admin');
		await expect(adminPageBody).toBeVisible();

		const version = await page.evaluate(() => {
			const versionElement = Array.from(document.querySelectorAll('*')).find((el) =>
				el.textContent?.match(/Version \d+\.\d+/)
			);
			return versionElement?.textContent ?? '';
		});

		if (version) {
			const match = version.match(/Version?\s*(\d+)\.(\d+)/);
			if (match) {
				const major = parseInt(match[1], 10);
				const minor = parseInt(match[2], 10);
				expect(major).toBeGreaterThanOrEqual(6);
				if (major === 6) {
					expect(minor).toBeGreaterThanOrEqual(7);
				}
			}
		}
	});
});

Please review the code and apply the changes. Let me know if you have any questions.