Skip to content

test: add E2E tests for untested pages - #1530

Open
eran132 wants to merge 7 commits into
mainfrom
test/add-e2e-coverage
Open

test: add E2E tests for untested pages#1530
eran132 wants to merge 7 commits into
mainfrom
test/add-e2e-coverage

Conversation

@eran132

@eran132 eran132 commented Apr 24, 2026

Copy link
Copy Markdown
Collaborator

Re-submitted from upstream branch (previously #1515).

Summary

Added Playwright E2E tests for pages with zero/minimal coverage:

  • homepage, velocityHeatmap, lineProfile, publicAppeal, gaps, gapsPatterns, dataResearch, donate

Test plan

  • TypeScript compiles clean
  • All CI checks pass

🤖 Generated with Claude Code

eran132 and others added 7 commits April 15, 2026 16:52
Adds Playwright E2E tests for pages that had zero or minimal coverage:

- homepage.spec.ts: heading, illustration, nav links, footer copyright
- velocityHeatmap.spec.ts: heading, date controls, radio buttons, map,
  expand button, date navigation
- lineProfile.spec.ts: dynamic route rendering, operator/date selectors, map
- publicAppealFull.spec.ts: heading, task list, RTL layout

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
New test files (4):
- gaps.spec.ts: title, description, date/operator/line selectors, route
  selector cascade, not-found message (6 tests)
- gapsPatterns.spec.ts: heading, date range selectors, operator/line
  selection, route selector, invalid date range (7 tests)
- dataResearch.spec.ts: direct navigation, heading, description,
  chart rendering, selectors (5 tests)
- donate.spec.ts: modal opens from menu, donation link, bank details,
  modal close (4 tests)

Extended test files (3):
- homepage.spec.ts: Hebrew text validation, link hrefs, mobile/desktop
  visibility (4 new tests)
- velocityHeatmap.spec.ts: date navigation, tile layer, legend,
  all 5 nav buttons (4 new tests)
- lineProfile.spec.ts: not-found for invalid ID, stop selector (2 new)

Total: 28 new test cases across 7 files.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Simplified gaps.spec.ts and gapsPatterns.spec.ts to only test UI
  element presence, not dropdown interactions that require matching HAR
  data (operator/route options were not in the HAR files)
- Fixed homepage.spec.ts to use i18next for text matching and removed
  year assertion that broke due to mocked system time
- Removed Selectors model dependency from simplified tests

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- homepage.spec.ts: used getByRole('heading') instead of locator('h1')
  which resolved to 2 elements (sidebar logo + page heading)
- lineProfile.spec.ts: simplified to only test route loading and map
  presence since API calls are aborted in test setup

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Removed assertions for elements that only render when API calls succeed
(leaflet map container, tile pane, legend). In CI, stride-api calls are
aborted so these elements never appear. Kept only tests that verify
page routing and UI shell rendering.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The i18next menu label doesn't match visible page content. Keep only
route and RTL layout tests that are reliable in CI.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 24, 2026 23:21
@eran132
eran132 requested a review from AvivAbachi as a code owner April 24, 2026 23:21

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds Playwright E2E coverage for previously untested/minimally tested pages to reduce end-to-end blind spots across key routes and UI flows.

Changes:

  • Introduced new Playwright spec files for multiple pages (homepage, heatmap, line profile, public appeal, gaps, patterns, data research, donate)
  • Added basic render/visibility assertions and a few interaction checks (radio selection, date navigation, modal open/close)
  • Added HAR-based routing for gaps-related pages to make tests deterministic

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
tests/velocityHeatmap.spec.ts Adds E2E coverage for the velocity heatmap page, including mode toggles and date navigation controls
tests/publicAppealFull.spec.ts Adds smoke + RTL-direction checks for the public appeal page
tests/lineProfile.spec.ts Adds direct navigation smoke test for a profile route
tests/homepage.spec.ts Adds homepage content checks (headings, illustration, links, footer, responsive sections)
tests/gapsPatterns.spec.ts Adds HAR-backed tests for gaps patterns page UI controls
tests/gaps.spec.ts Adds HAR-backed tests for gaps page UI controls
tests/donate.spec.ts Adds modal open/content/close tests for donate flow
tests/dataResearch.spec.ts Adds direct route/load checks plus assertions for key sections on data research page

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +60 to +67
await page.waitForTimeout(300)
const afterPrevDay = await dateInput.inputValue()
expect(afterPrevDay).not.toEqual(initialValue)

await page.getByRole('button', { name: i18next.t('date_navigator_next_day') }).click()
await page.waitForTimeout(300)
const afterNextDay = await dateInput.inputValue()
expect(afterNextDay).toEqual(initialValue)

Copilot AI Apr 24, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

page.waitForTimeout(300) makes this test timing-dependent and flaky under slow CI. Prefer waiting on an explicit condition (e.g., await expect(dateInput).not.toHaveValue(initialValue) after prev-day click, and then await expect(dateInput).toHaveValue(initialValue) after next-day click), or use expect.poll on inputValue() so Playwright waits until the UI updates.

Suggested change
await page.waitForTimeout(300)
const afterPrevDay = await dateInput.inputValue()
expect(afterPrevDay).not.toEqual(initialValue)
await page.getByRole('button', { name: i18next.t('date_navigator_next_day') }).click()
await page.waitForTimeout(300)
const afterNextDay = await dateInput.inputValue()
expect(afterNextDay).toEqual(initialValue)
await expect(dateInput).not.toHaveValue(initialValue)
await page.getByRole('button', { name: i18next.t('date_navigator_next_day') }).click()
await expect(dateInput).toHaveValue(initialValue)

Copilot uses AI. Check for mistakes.
})

test('page displays heading and date controls', async ({ page }) => {
await expect(page.getByRole('heading', { name: 'Velocity Aggregation Heatmap' })).toBeVisible()

Copilot AI Apr 24, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assertion hardcodes an English heading string while the rest of the spec uses i18next keys. If the app runs under a non-English locale (or the copy changes), this will break. Prefer asserting with i18next.t(...) (or by checking a translated key that visitPage already targets) to keep the test aligned with the i18n approach used elsewhere.

Suggested change
await expect(page.getByRole('heading', { name: 'Velocity Aggregation Heatmap' })).toBeVisible()
await expect(
page.getByRole('heading', { name: i18next.t('velocity_heatmap_page_title') }),
).toBeVisible()

Copilot uses AI. Check for mistakes.
Comment thread tests/homepage.spec.ts
})

test('homepage displays bus illustration', async ({ page }) => {
const img = page.locator('img[alt="Public Transportaion Bus Illustration"]')

Copilot AI Apr 24, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The selector contains a likely typo in the alt text ("Transportaion"). If the actual UI text is corrected, this test will fail. Consider using the correct spelling (or a less brittle selector, e.g., regex / translation-based alt text) to avoid coupling the test to a misspelling.

Suggested change
const img = page.locator('img[alt="Public Transportaion Bus Illustration"]')
const img = page.getByAltText(/Public Transporta(?:tion|ion) Bus Illustration/)

Copilot uses AI. Check for mistakes.
Comment thread tests/homepage.spec.ts
Comment on lines +24 to +26
const links = page.locator('section.links .page-link')
const count = await links.count()
expect(count).toBe(6)

Copilot AI Apr 24, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Asserting an exact link count is brittle and will fail on legitimate UI changes (adding/removing a link, A/B tests, responsive variants). A more stable approach is to assert the presence of specific expected links (by role/name) or to check a minimum count if the exact number isn’t a strict product requirement.

Suggested change
const links = page.locator('section.links .page-link')
const count = await links.count()
expect(count).toBe(6)
const linksSection = page.locator('section.links')
await expect(linksSection.getByRole('link', { name: i18next.t('nav.home') })).toBeVisible()
await expect(linksSection.getByRole('link', { name: i18next.t('nav.routes') })).toBeVisible()
await expect(linksSection.getByRole('link', { name: i18next.t('nav.stops') })).toBeVisible()
await expect(linksSection.getByRole('link', { name: i18next.t('nav.map') })).toBeVisible()
await expect(linksSection.getByRole('link', { name: i18next.t('nav.about') })).toBeVisible()
await expect(linksSection.getByRole('link', { name: i18next.t('nav.contact') })).toBeVisible()

Copilot uses AI. Check for mistakes.
Comment on lines +6 to +43
})

test('accessing /data-research directly works', async ({ page }) => {
await page.goto('/data-research')
await page.locator('.preloader').waitFor({ state: 'hidden' })
await page.waitForLoadState('networkidle')
await expect(page).toHaveURL(/data-research/)
})

test('page displays research section heading', async ({ page }) => {
await page.goto('/data-research')
await page.locator('.preloader').waitFor({ state: 'hidden' })
await page.waitForLoadState('networkidle')
const title = page.locator('h2', { hasText: 'מחקרים' })
await expect(title).toBeVisible()
})

test('page displays research description text', async ({ page }) => {
await page.goto('/data-research')
await page.locator('.preloader').waitFor({ state: 'hidden' })
await page.waitForLoadState('networkidle')
await expect(
page.getByText('אם יש לכם רעיון מעניין למה קורים פה דברים, דברו איתנו בסלאק!'),
).toBeVisible()
})

test('stacked research section with charts is rendered', async ({ page }) => {
await page.goto('/data-research')
await page.locator('.preloader').waitFor({ state: 'hidden' })
await page.waitForLoadState('networkidle')
const etlWidget = page.locator('h2', { hasText: 'בעיות etl/gps/משהו גלובאלי אחר' })
await expect(etlWidget).toBeVisible()
})

test('research page has date selectors and operator selector', async ({ page }) => {
await page.goto('/data-research')
await page.locator('.preloader').waitFor({ state: 'hidden' })
await page.waitForLoadState('networkidle')

Copilot AI Apr 24, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file repeats the same navigation + waiting sequence in multiple tests, which will significantly slow the suite and increases surface area for flakes. Since these tests all target the same route, consider moving the goto('/data-research') + wait logic into beforeEach (or a shared helper like visitPage) and keep tests focused on assertions.

Suggested change
})
test('accessing /data-research directly works', async ({ page }) => {
await page.goto('/data-research')
await page.locator('.preloader').waitFor({ state: 'hidden' })
await page.waitForLoadState('networkidle')
await expect(page).toHaveURL(/data-research/)
})
test('page displays research section heading', async ({ page }) => {
await page.goto('/data-research')
await page.locator('.preloader').waitFor({ state: 'hidden' })
await page.waitForLoadState('networkidle')
const title = page.locator('h2', { hasText: 'מחקרים' })
await expect(title).toBeVisible()
})
test('page displays research description text', async ({ page }) => {
await page.goto('/data-research')
await page.locator('.preloader').waitFor({ state: 'hidden' })
await page.waitForLoadState('networkidle')
await expect(
page.getByText('אם יש לכם רעיון מעניין למה קורים פה דברים, דברו איתנו בסלאק!'),
).toBeVisible()
})
test('stacked research section with charts is rendered', async ({ page }) => {
await page.goto('/data-research')
await page.locator('.preloader').waitFor({ state: 'hidden' })
await page.waitForLoadState('networkidle')
const etlWidget = page.locator('h2', { hasText: 'בעיות etl/gps/משהו גלובאלי אחר' })
await expect(etlWidget).toBeVisible()
})
test('research page has date selectors and operator selector', async ({ page }) => {
await page.goto('/data-research')
await page.locator('.preloader').waitFor({ state: 'hidden' })
await page.waitForLoadState('networkidle')
await page.goto('/data-research')
await page.locator('.preloader').waitFor({ state: 'hidden' })
await page.waitForLoadState('networkidle')
})
test('accessing /data-research directly works', async ({ page }) => {
await expect(page).toHaveURL(/data-research/)
})
test('page displays research section heading', async ({ page }) => {
const title = page.locator('h2', { hasText: 'מחקרים' })
await expect(title).toBeVisible()
})
test('page displays research description text', async ({ page }) => {
await expect(
page.getByText('אם יש לכם רעיון מעניין למה קורים פה דברים, דברו איתנו בסלאק!'),
).toBeVisible()
})
test('stacked research section with charts is rendered', async ({ page }) => {
const etlWidget = page.locator('h2', { hasText: 'בעיות etl/gps/משהו גלובאלי אחר' })
await expect(etlWidget).toBeVisible()
})
test('research page has date selectors and operator selector', async ({ page }) => {

Copilot uses AI. Check for mistakes.
Comment thread tests/donate.spec.ts
Comment on lines +43 to +44
// Close button is the X button inside the modal
const closeButton = modal.locator('button').first()

Copilot AI Apr 24, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Selecting the “first button” in the modal is fragile (button order changes easily). Prefer locating the close control by accessible name/label (e.g., getByRole('button', { name: ... })) or a stable attribute (like aria-label="Close"), so the test targets the intended control.

Suggested change
// Close button is the X button inside the modal
const closeButton = modal.locator('button').first()
// Target the modal's close control by semantics instead of button order
const closeButton = modal.getByRole('button', { name: /close/i })

Copilot uses AI. Check for mistakes.
@eran132

eran132 commented Apr 25, 2026

Copy link
Copy Markdown
Collaborator Author

The Playwright Visual Tests show 4 visual diffs out of 182 — these are from the new E2E test pages being compared against baseline screenshots for the first time. These need approval on the Applitools dashboard by a maintainer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants