Skip to content

[E2E Tests] Remove implicit dependency on UI defaults #339

Description

@pedropaulovc

Description

E2E tests should not have implicit dependencies on UI defaults. Currently, many tests assume certain default UI states (view modes, content modes, filter states) without explicitly configuring them in their setup. This makes tests brittle and harder to maintain, as changes to UI defaults can break tests that rely on those implicit states.

This is a code quality and maintainability refactoring to ensure tests are explicit about their required state.

Current Issue

Currently, E2E tests have implicit dependencies on:

  • Default view mode (inline vs side-by-side/sxs)
  • Default content mode (changes vs full file)
  • Default filter state (both vs left vs right)
  • Other UI preferences/defaults that may change over time

For example, a test that expects inline mode might fail if the app changes its default view mode. Tests should not rely on these implicit defaults.

Solution

Tests should be refactored to either:

  1. Work with equivalent states: Some tests that work in inline mode should also work in sxs mode (or vice versa), so they're not dependent on a specific default
  2. Explicitly configure required state: Tests that need a specific view mode or state should explicitly set it up in their test steps (e.g., pressing 'x' to switch to sxs, or navigating to set the view mode explicitly)

Tasks

  • Audit all E2E test files in e2e/ directory to identify tests with implicit UI state dependencies
  • For each test, determine whether it should:
    • Be made mode-agnostic (work in both inline and sxs, for example)
    • Explicitly set up its required state in the test body
  • Remove any "legacy defaults" setup functions or fixtures (e.g., e2e/fixtures/legacy-defaults.ts or setupLegacyDefaults() if they exist)
  • Update failing tests by adding explicit state configuration
  • Document which tests depend on which UI states as comments in the test files
  • Run npm run test:e2e to verify all tests pass with the changes
  • Verify tests are still properly checking the behavior they're meant to test

Examples

Before (implicit dependency):

test("should apply filter correctly", async ({ page }) => {
  // Assumes default view mode is inline
  // Test will fail if app changes default to sxs
  const filterRadio = page.getByRole("radio", { name: "Left" });
  await filterRadio.click();
  // ... assertions
});

After (explicit state):

test("should apply filter correctly in inline mode", async ({ page }) => {
  // ... setup to load PR and file
  // Explicitly ensure we're in inline mode
  const toolbar = page.getByRole("toolbar", { name: "Diff view controls" });
  const viewModeBtn = toolbar.getByRole("button", { name: "View mode" });
  
  // Get current view mode and switch to inline if needed
  const currentMode = await viewModeBtn.textContent();
  if (!currentMode?.includes("Inline")) {
    await viewModeBtn.click();
    await page.getByRole("option", { name: /Inline/i }).click();
  }
  
  // Now test the filter
  const filterRadio = page.getByRole("radio", { name: "Left" });
  await filterRadio.click();
  // ... assertions
});

Testing

After refactoring:

  • Run npm run test:e2e (mock mode)
  • Run npm run test:e2e:prod (prod mode) if applicable
  • Confirm all tests pass without relying on hardcoded UI defaults

Notes

  • Focus on making tests explicit and maintainable
  • Tests should clearly document what UI state they require (via comments or explicit setup)
  • Prefer making tests work with equivalent states when possible (more resilient)
  • Only add explicit setup when tests truly need a specific state

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions