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:
- 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
- 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
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:
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
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:
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:
Tasks
e2e/directory to identify tests with implicit UI state dependenciese2e/fixtures/legacy-defaults.tsorsetupLegacyDefaults()if they exist)npm run test:e2eto verify all tests pass with the changesExamples
Before (implicit dependency):
After (explicit state):
Testing
After refactoring:
npm run test:e2e(mock mode)npm run test:e2e:prod(prod mode) if applicableNotes