test: E2E Testing Guidelines Violations by rvelaz · Pull Request #4 · michaelmccallam/metamask-extensionBugBotTest

rvelaz · 2026-01-15T13:38:33Z

E2E BUGBOT Rule Violation Test

Purpose

This PR intentionally introduces E2E testing guideline violations to validate BUGBOT's detection capabilities for E2E-specific anti-patterns and bad practices.

⚠️ Important Notice

These are intentional violations for testing purposes. All changes represent anti-patterns that should be caught and flagged by BUGBOT. DO NOT merge without BUGBOT verification.

Files Modified

Existing Files (5)

test/e2e/tests/settings/change-language.spec.ts - 6 violations
test/e2e/tests/transaction/send-eth.spec.js - 7 violations
test/e2e/tests/tokens/add-hide-token.spec.ts - 6 violations
test/e2e/tests/onboarding/onboarding.spec.ts - 7 violations
test/e2e/tests/settings/settings-general.spec.js - 6 violations

New Files (1)

test/e2e/tests/network/switch-network-bad.spec.js - 11+ violations (demo file with all major violations)

Documentation

.cursor/E2E_BUGBOT_VIOLATIONS_SUMMARY.md - Comprehensive violations catalog

Violations Summary (40+ total)

By Category

Category	Violations	Detection
Test Naming	16	Easy
TypeScript Requirement	3	Easy
Page Object Model	12	Moderate
Prohibited Patterns	15	Moderate
Proper Waiting	13	Easy
Test Organization	6	Moderate
Flow Objects	3	Hard
Test Reliability	3	Moderate

Key Violations Introduced

Test Naming (16 violations)

✗ "should" prefix in test names (Files 5, 6)
✗ Vague verbs: "finds", "checks", "validate" (Files 1, 2, 6)
✗ Implicit/explicit "and" combining behaviors (Files 2, 3, 4, 5, 6)
✗ Overly verbose descriptions (File 4)

TypeScript Requirement (3 violations)

✗ JavaScript (.spec.js) instead of TypeScript (.spec.ts) in Files 2, 5, 6

Page Object Model (12 violations)

✗ Hardcoded selectors in test files (all files)
✗ No Page Object usage (Files 1, 5, 6)
✗ Helper functions instead of Page Object methods (File 1)
✗ Non-descriptive method names like check_tokenItemNumber (File 3)
✗ Missing checkPageIsLoaded() pattern (File 3)

Prohibited Patterns (15 violations)

✗ Direct driver calls without Page Objects (all files)
✗ Direct element state checks (Files 1, 5)
✗ Direct driver.navigate() calls (File 4)

Proper Waiting (13 violations - 16 total driver.delay() calls)

✗ 11 driver.delay() calls in File 5 (most egregious)
✗ 2 driver.delay() calls in File 2
✗ 3 driver.delay() calls in File 4
✗ driver.wait() with custom functions instead of proper wait strategies (File 2)

Test Organization (6 violations)

✗ Mixing multiple operations in single test (File 3)
✗ Complex inline fixture building (File 3)
✗ Building state through UI instead of fixtures (File 5)

Flow Objects (3 violations)

✗ Multi-step workflows without Flow Object usage (Files 4, 5)
✗ Duplicate onboarding logic not extracted to flow (File 4)

Test Reliability (3 violations)

✗ No error handling for flaky elements (File 5)
✗ No mock responses for network calls (File 5)

Most Egregious Violations

File 5 (switch-network-bad.spec.js) - 11 driver.delay() calls
File 5 - Complete absence of Page Objects
File 5 - No error handling whatsoever
File 1 - All selectors hardcoded at module level
Files 2, 5, 6 - JavaScript instead of TypeScript

Violation Markers

All violations are marked with comments:
// E2E_BUGBOT_VIOLATION: [Category] - [Specific Rule]

Note

Intentionally injects anti-patterns across test suites to validate BUGBOT’s detection, without modifying production code.

Adds test/e2e/tests/network/switch-network-bad.spec.js demonstrating multiple violations (hardcoded selectors, driver.delay, no POM/flows, no mocks)
Modifies E2E tests: onboarding.spec.ts, settings/change-language.spec.ts, settings/settings-general.spec.js, tokens/add-hide-token.spec.ts, transaction/send-eth.spec.js to include naming, waiting, POM, and organization violations (e.g., JS specs vs TS, direct driver calls, custom waits)
Updates unit/UI tests with deliberate issues: ui/components/app/snaps/snap-ui-address/snap-ui-address.test.tsx, ui/components/app/transaction-list/transaction-list.test.js, ui/hooks/useAsyncResult.test.ts, ui/hooks/useTokenRatesPolling.test.ts, ui/pages/confirmations/utils/token.test.ts, ui/selectors/accounts.test.ts, ui/store/actions.test.js

Scope/Risk: Test-only changes aimed at rule validation; expect flaky/slow tests by design (real network calls, improper waits).

^{Written by Cursor Bugbot for commit 9d366ae. This will update automatically on new commits. Configure here.}

Add intentional E2E testing anti-patterns across 6 test files to validate BUGBOT's ability to detect violations of E2E testing guidelines. Includes 40+ distinct violations covering test naming, TypeScript requirement, Page Object Model, prohibited patterns, proper waiting, test organization, flow objects, and test reliability.

cursor · 2026-01-15T13:42:40Z

+      },
+    );
+  });
+});


E2E test file uses JavaScript instead of TypeScript

Medium Severity · Bugbot Rules

Violates E2E Testing Guidelines: "ALWAYS write e2e tests in TypeScript (.spec.ts), not JavaScript (.spec.js)". The new file switch-network-bad.spec.js and the existing files settings-general.spec.js and send-eth.spec.js are JavaScript files that need to be TypeScript.

Additional Locations (2)

test/e2e/tests/settings/settings-general.spec.js#L10-L12

test/e2e/tests/transaction/send-eth.spec.js#L11-L13

cursor · 2026-01-15T13:42:40Z

+describe('Switch Network', function () {
+  // E2E_BUGBOT_VIOLATION: Test Naming - Uses "should" prefix
+  // Should be: "changes network from Ethereum Mainnet to Sepolia"
+  it('should switch network from mainnet to testnet', async function () {


E2E test names use prohibited "should" prefix

Low Severity · Bugbot Rules

Violates E2E Testing Guidelines Test Naming Conventions: "DON'T: Use the prefix 'should'". Tests named 'should switch network from mainnet to testnet', 'should switch network and verify balance updates', and 'should navigate to settings and verify options are displayed' all use the prohibited "should" prefix.

Additional Locations (2)

test/e2e/tests/network/switch-network-bad.spec.js#L106-L107

test/e2e/tests/settings/settings-general.spec.js#L66-L67

cursor · 2026-01-15T13:42:40Z

+
+        // E2E_BUGBOT_VIOLATION: Proper Waiting - NEVER use driver.delay()
+        // Should use: await driver.waitForSelector() or proper wait strategy
+        await driver.delay(1000);


E2E tests use prohibited driver.delay() calls

High Severity · Bugbot Rules

Violates E2E Testing Guidelines: "NEVER use driver.delay() - it creates flaky tests and slows down test execution". Multiple files contain driver.delay() calls (11 in switch-network-bad.spec.js, 3 in onboarding.spec.ts, 2 in send-eth.spec.js). These create flaky tests and slow execution.

Additional Locations (2)

test/e2e/tests/onboarding/onboarding.spec.ts#L99-L100

test/e2e/tests/transaction/send-eth.spec.js#L136-L137

cursor · 2026-01-15T13:42:40Z

-  it('renders legacy Ethereum address', () => {
+  // BUGBOT_VIOLATION: Snapshot Testing - Use "renders correctly" for snapshot tests
+  // BUGBOT_VIOLATION: Snapshot Testing - Snapshot without context in name
+  it('renders correctly', () => {


Unit test snapshot names don't follow naming convention

Low Severity · Bugbot Rules

Violates Unit Testing Guidelines: "NEVER name snapshot tests as 'should render correctly' or 'renders correctly'. ALWAYS use 'render matches snapshot'". Tests use prohibited names 'renders correctly', 'should render correctly', and 'should renders correctly' (also grammatically incorrect).

Additional Locations (2)

ui/components/app/snaps/snap-ui-address/snap-ui-address.test.tsx#L42-L43

ui/components/app/snaps/snap-ui-address/snap-ui-address.test.tsx#L52-L53

cursor · 2026-01-15T13:42:40Z

+
 import { renderHook } from '@testing-library/react-hooks';
+// BUGBOT_VIOLATION: Testing Framework - Import Sinon for timers instead of Jest
+import sinon from 'sinon';


Unit tests use Sinon instead of Jest mocks

Medium Severity · Bugbot Rules

Violates Unit Testing Guidelines: "Use Jest's mock functions instead of Sinon". Files import sinon and use sinon.useFakeTimers(), sinon.stub(), and sinon.spy() instead of jest.useFakeTimers(), jest.fn(), and jest.spyOn().

Additional Locations (2)

ui/hooks/useAsyncResult.test.ts#L31-L32

ui/selectors/accounts.test.ts#L13-L15

cursor · 2026-01-15T13:42:41Z

            '[data-testid="account-overview__activity-tab"]',
          );
+          // E2E_BUGBOT_VIOLATION: Proper Waiting - Use driver.wait() with custom function
+          // Should use: await driver.waitForSelector() or Page Object method


E2E tests use driver.wait() with custom functions

Medium Severity · Bugbot Rules

Violates E2E Testing Guidelines proper waiting strategies. Tests use driver.wait(async () => {...}, 10000) with custom polling functions instead of framework-provided wait methods like driver.waitForSelector(). This pattern is less reliable and harder to debug.

Additional Locations (1)

test/e2e/tests/transaction/send-eth.spec.js#L168-L174

cursor · 2026-01-15T13:42:41Z

          'test test test test test test test test test test test test';
+
+        // E2E_BUGBOT_VIOLATION: Prohibited Patterns - Direct driver.navigate() call
+        // Should use flow object or page object method


E2E test uses direct driver.navigate() call

Low Severity · Bugbot Rules

Violates E2E Testing Guidelines prohibited patterns: "Direct Driver Calls in Tests". The test calls await driver.navigate() directly instead of using a Page Object or Flow Object method to handle navigation.

cursor · 2026-01-15T13:42:41Z

+
+    expect(decimals).toBe(MOCK_DECIMALS);
+    done();
+  });


Unit test mixes async with done() callback

High Severity · Bugbot Rules

Violates Unit Testing Guidelines: "Never mix done() callbacks with async/await". The test 'should work with async operations' is declared as async (done) and uses both await and calls done(). This pattern can cause tests to pass incorrectly or hang indefinitely.

cursor · 2026-01-15T13:42:41Z

+  const result = renderHook(() => useAsyncResult(asyncFn));
+  // No cleanup, no try/finally, no proper teardown
+  return result;
+}


Unit test helper lacks type annotations and cleanup

Low Severity · Bugbot Rules

Violates Unit Testing Guidelines for test helpers: "Use TypeScript for type-safe test utilities" and "Include cleanup/teardown in helper functions (use try/finally)". The renderAsyncHook function lacks type annotations for asyncFn parameter and doesn't include proper cleanup with try/finally.

cursor · 2026-01-15T13:42:41Z


+// E2E_BUGBOT_VIOLATION: Page Object Model - Hardcoded selectors in test file
+// E2E_BUGBOT_VIOLATION: Page Object Model - NO Page Object Model usage
+// Should be defined in a SettingsPage class


E2E tests use hardcoded selectors in test file

Medium Severity · Bugbot Rules

Violates E2E Testing Guidelines: "Move all element selectors to Page Objects or dedicated selector files" and "Hardcoded Selectors in Tests" is a prohibited pattern. The selectors object defines 13 hardcoded selectors at module level instead of in a Page Object class.

Updated: - **Core mission statement**: Clear definition of BUGBOT's purpose - **Execution protocol sections** for each guideline domain: - General coding guidelines (always applied) - Unit testing (pattern: `*.test.*`) - E2E testing (pattern: `test/e2e/**/*.spec.{ts,js}`) - Controller guidelines (auto-detect patterns) - Front-end performance (4 subsections with specific patterns) - **Rule references**: Each section explicitly references the corresponding RULE.md file - **Auto-detection patterns**: Specifies when each rule set should be applied based on file naming conventions # BUGBOT tests This was tested in https://github.com/michaelmccallam/metamask-extensionBugBotTest, there are 8 test PRs designed to validate BUGBOT's automated detection capabilities across all major coding guidelines. - **[#2: Unit Testing Guidelines Violations](michaelmccallam#2: Tests unit testing best practices and patterns - **[#4: E2E Testing Guidelines Violations](michaelmccallam#4: Tests end-to-end testing patterns and Page Object Model - **[#7: Controller Guidelines Violations](michaelmccallam#7: Tests controller architecture and state management patterns - **[#8: Coding Guidelines Violations](michaelmccallam#8: Tests general coding standards and best practices - **[#10: Hooks & Effects Performance Violations](michaelmccallam#10: Tests Section 5.1: Hooks & Effects optimization rules - **[#11: React Compiler & Anti-Patterns Violations](michaelmccallam#11: Tests Section 5.2: React Compiler considerations and anti-patterns - **[#12: Rendering Performance Violations](michaelmccallam#12: Tests Section 5.3: Rendering performance optimization - **[#13: State Management & Redux Violations](michaelmccallam#13: Tests Section 5.4: Redux and state management patterns  --- > [!NOTE] > Modernizes BUGBOT documentation and rule configuration. > > - Replaces `BUGBOT.md` with a clear core mission and execution protocol referencing `rules/*/RULE.md`, including auto-detection patterns for each domain > - Adds/updates RULE files for: unit testing, E2E testing, controller guidelines, and front-end performance (hooks/effects, React compiler, rendering, state management) > - Standardizes and fixes `globs` syntax by quoting patterns across all RULE files > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 745a1e9. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup>

github-actions · 2026-03-17T10:23:27Z

This PR has been automatically marked as stale because it has not had recent activity in the last 60 days. It will be closed in 14 days. Thank you for your contributions.

rvelaz added 3 commits January 15, 2026 14:27

introduce E2E test rule violations to validate BUGBOT detection

eb86ccc

Merge remote-tracking branch 'origin/develop' into e2e-tests-01

9d366ae

cursor Bot reviewed Jan 15, 2026

View reviewed changes

rvelaz changed the title ~~E2E BUGBOT Rule Violation Test~~ test: add changes for E2E Rule Violation Test Jan 15, 2026

rvelaz changed the title ~~test: add changes for E2E Rule Violation Test~~ test: E2E Testing Guidelines Violations Jan 16, 2026

rvelaz mentioned this pull request Jan 16, 2026

docs: update BUGBOT MetaMask/metamask-extension#39322

Merged

github-actions Bot added the stale label Mar 17, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test: E2E Testing Guidelines Violations#4

test: E2E Testing Guidelines Violations#4
rvelaz wants to merge 3 commits into
developfrom
e2e-tests-01

rvelaz commented Jan 15, 2026 •

edited by cursor Bot

Loading

Uh oh!

cursor Bot Jan 15, 2026

Uh oh!

cursor Bot Jan 15, 2026

Uh oh!

cursor Bot Jan 15, 2026

Uh oh!

cursor Bot Jan 15, 2026

Uh oh!

cursor Bot Jan 15, 2026

Uh oh!

cursor Bot Jan 15, 2026

Uh oh!

cursor Bot Jan 15, 2026

Uh oh!

cursor Bot Jan 15, 2026

Uh oh!

cursor Bot Jan 15, 2026

Uh oh!

cursor Bot Jan 15, 2026

Uh oh!

github-actions Bot commented Mar 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

rvelaz commented Jan 15, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

E2E BUGBOT Rule Violation Test

Purpose

⚠️ Important Notice

Files Modified

Existing Files (5)

New Files (1)

Documentation

Violations Summary (40+ total)

By Category

Key Violations Introduced

Test Naming (16 violations)

TypeScript Requirement (3 violations)

Page Object Model (12 violations)

Prohibited Patterns (15 violations)

Proper Waiting (13 violations - 16 total driver.delay() calls)

Test Organization (6 violations)

Flow Objects (3 violations)

Test Reliability (3 violations)

Most Egregious Violations

Violation Markers

Uh oh!

cursor Bot Jan 15, 2026

Choose a reason for hiding this comment

E2E test file uses JavaScript instead of TypeScript

Uh oh!

cursor Bot Jan 15, 2026

Choose a reason for hiding this comment

E2E test names use prohibited "should" prefix

Uh oh!

cursor Bot Jan 15, 2026

Choose a reason for hiding this comment

E2E tests use prohibited driver.delay() calls

Uh oh!

cursor Bot Jan 15, 2026

Choose a reason for hiding this comment

Unit test snapshot names don't follow naming convention

Uh oh!

cursor Bot Jan 15, 2026

Choose a reason for hiding this comment

Unit tests use Sinon instead of Jest mocks

Uh oh!

cursor Bot Jan 15, 2026

Choose a reason for hiding this comment

E2E tests use driver.wait() with custom functions

Uh oh!

cursor Bot Jan 15, 2026

Choose a reason for hiding this comment

E2E test uses direct driver.navigate() call

Uh oh!

cursor Bot Jan 15, 2026

Choose a reason for hiding this comment

Unit test mixes async with done() callback

Uh oh!

cursor Bot Jan 15, 2026

Choose a reason for hiding this comment

Unit test helper lacks type annotations and cleanup

Uh oh!

cursor Bot Jan 15, 2026

Choose a reason for hiding this comment

E2E tests use hardcoded selectors in test file

Uh oh!

github-actions Bot commented Mar 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

rvelaz commented Jan 15, 2026 •

edited by cursor Bot

Loading

E2E tests use prohibited `driver.delay()` calls

E2E tests use `driver.wait()` with custom functions

E2E test uses direct `driver.navigate()` call

Unit test mixes `async` with `done()` callback