Skip to content

Fix PIM test failures: resilient navigation + unique Employee ID generation - #35

Merged
bg-playground merged 2 commits into
mainfrom
copilot/fix-pim-test-failures
Mar 6, 2026
Merged

Fix PIM test failures: resilient navigation + unique Employee ID generation#35
bg-playground merged 2 commits into
mainfrom
copilot/fix-pim-test-failures

Conversation

Copilot AI commented Mar 6, 2026

Copy link
Copy Markdown
Contributor

Two independent root causes were causing all 5 PIM tests to fail in CI: navigateToPIM() blocked on a Dashboard heading that isn't present when called from a non-Dashboard page, and auto-generated Employee IDs collided with existing records on the shared demo site.

Changes

DashboardPage.jsnavigateToPIM()

  • Removed the dashboardTitle.waitFor() guard (was timing out after 60s when beforeEach ran on subsequent tests while already on the PIM page)
  • Inverted the strategy: direct URL navigation (/web/index.php/pim/viewEmployeeList) is now the primary path; sidebar menu click is the fallback
// Before: blocked on Dashboard heading — 60s timeout if not on Dashboard
await this.dashboardTitle.waitFor({ state: 'visible', timeout: 60000 });

// After: go directly, no page-state assumption
await this.page.goto('/web/index.php/pim/viewEmployeeList');
await this.page.waitForURL(/.*pim/, { timeout: 30000 });

PIMPage.jsAddEmployeePage.addEmployee()

  • Always generates a timestamp-based Employee ID when none is supplied, preventing "Employee Id already exists" errors on the shared OrangeHRM demo site
// Always set a unique Employee ID to prevent collisions on the shared demo site
const uniqueId = employeeId || String(Date.now()).slice(-4);
await this.employeeIdInput.clear();
await this.employeeIdInput.fill(uniqueId);
Original prompt

Fix PIM test failures: navigation assumes Dashboard page state + Employee ID collisions

Replaces the approach in PR #32, which misdiagnosed the root cause and made failures slower without fixing them.

Root Cause Analysis

Problem 1: navigateToPIM() assumes we're on the Dashboard page

In automated-testing/pages/DashboardPage.js, the navigateToPIM() method (line 64-81) starts with:

await this.dashboardTitle.waitFor({ state: 'visible', timeout: 60000 });

This waits up to 60 seconds for the "Dashboard" heading to be visible. However, pim.spec.js calls navigateToPIM() in beforeEach (line 22), and beforeEach runs before EVERY test. After the first test runs and the browser is on the PIM page, subsequent beforeEach calls hit navigateToPIM() while still on the PIM page — causing a 60-second timeout waiting for a "Dashboard" heading that isn't there.

Evidence from CI: all 5 PIM tests fail at DashboardPage.js:68 (the navigateToPIM method), and the total run time is ~9.6 minutes — consistent with multiple 60-second timeouts.

However, this actually works in practice because pim.spec.js line 19-21 does loginPage.goto() + loginAndWaitForDashboard() before calling navigateToPIM(), which navigates to the login page and then to the dashboard. So the test always starts from Dashboard. The real issue is that the OrangeHRM demo site is sometimes slow to load the Dashboard heading, causing the 60-second timeout to be insufficient, OR the login redirects to a different page.

The fix should make navigateToPIM() resilient — navigate directly via URL rather than depending on the Dashboard heading being visible, matching the pattern used by navigateToAdmin()'s fallback.

Problem 2: addEmployee() doesn't clear/set a unique Employee ID

In automated-testing/pages/PIMPage.js, the AddEmployeePage.addEmployee() method (line 44-52) only sets a custom Employee ID if one is explicitly passed. When called without an Employee ID from pim.spec.js line 49-51, the auto-generated ID from OrangeHRM might collide with existing employees on the shared demo site. The screenshot evidence shows error "Employee Id already exists" for ID 0456.

The fix: addEmployee() should always clear the Employee ID field and fill it with a unique value based on the timestamp to prevent collisions.

Files to Modify

1. automated-testing/pages/DashboardPage.js

Replace the navigateToPIM() method (lines 64-81) with a more robust version that:

  • Navigates directly to the PIM Employee List URL (/web/index.php/pim/viewEmployeeList) as the primary approach, rather than depending on being on the Dashboard page first
  • Falls back to sidebar menu click if the direct navigation fails
  • Verifies the PIM heading is visible after navigation
  • Does NOT start by waiting for this.dashboardTitle — this is the root cause of the timeout
  • Keeps the same method signature and return behavior

The new navigateToPIM() should look like this:

async navigateToPIM() {
    // Navigate directly to PIM Employee List — avoids dependency on being on Dashboard page
    try {
      await this.page.goto('/web/index.php/pim/viewEmployeeList');
      await this.page.waitForURL(/.*pim/, { timeout: 30000 });
    } catch (e) {
      // Fallback: try sidebar menu click
      await this.ensureMenuVisible();
      await this.pimMenu.scrollIntoViewIfNeeded();
      await this.pimMenu.waitFor({ state: 'visible', timeout: 15000 });
      await this.pimMenu.click();
      await this.page.waitForURL(/.*pim/, { timeout: 30000 });
    }

    // Verify PIM page loaded
    await this.page.getByRole('heading', { name: 'PIM' }).waitFor({ state: 'visible', timeout: 30000 });
}

Do NOT modify any other methods in DashboardPage.js. The navigateToAdmin(), navigateToLeave(), navigateToTime(), navigateToRecruitment(), constructor, isDashboardVisible(), logout(), ensureMenuVisible(), and getUsername() methods must remain exactly as they are.

2. automated-testing/pages/PIMPage.js

Modify the addEmployee() method in the AddEmployeePage class (lines 44-52) to always generate and use a unique Employee ID when none is provided. This prevents "Employee Id already exists" errors on the shared demo site:

async addEmployee(firstName, lastName, employeeId) {
    await this.firstNameInput.fill(firstName);
    await this.lastNameInput.fill(lastName);
    // Always set a unique Employee ID to prevent collisions on the shared demo site
    const uniqueId = employeeId || String(Date.now()).slice(-4);
    await this.employeeIdInput.clear();
    await this.employeeIdInput.fill(uniqueId);
    await this.saveButton.click();
}

Do NOT modify any other methods or classes in PIMPage.js. The PIMPage class and all its methods must remain exactly as they are.

What NOT to change

  • Do not modify pim.spec.js — the test file is correct
  • Do not modify any other page objec...

This pull request was created from Copilot chat.


🔒 GitHub Advanced Security automatically protects Copilot coding agent pull requests. You can protect all pull requests by enabling Advanced Security for your repositories. Learn more about Advanced Security.

…eneration

Co-authored-by: bg-playground <259109604+bg-playground@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix PIM test failures due to incorrect navigation assumptions Fix PIM test failures: resilient navigation + unique Employee ID generation Mar 6, 2026
@bg-playground
bg-playground marked this pull request as ready for review March 6, 2026 03:34
@bg-playground
bg-playground merged commit ccc720f into main Mar 6, 2026
1 check passed
@bg-playground
bg-playground deleted the copilot/fix-pim-test-failures branch March 6, 2026 03:34
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