Skip to content

Conversation

@ktx-akshay
Copy link
Collaborator

No description provided.

@github-actions
Copy link
Contributor

github-actions bot commented Dec 3, 2025

Failed to generate code suggestions for PR

@ktx-akshay ktx-akshay force-pushed the stabilize-dashboard-test-suite branch from 7857b21 to a55d65b Compare December 3, 2025 11:35
@ktx-akshay ktx-akshay marked this pull request as ready for review December 3, 2025 11:47
@github-actions
Copy link
Contributor

github-actions bot commented Dec 3, 2025

Failed to generate code suggestions for PR

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 3, 2025

Greptile Overview

Greptile Summary

This PR significantly improves the stability and robustness of dashboard import end-to-end tests by adding comprehensive retry logic, extended wait conditions, and better error handling throughout the import and deletion workflows.

Key Changes

  • Enhanced retry mechanisms: Added retry loops (3-5 attempts) for file uploads, URL tab clicks, and dashboard row interactions to handle timing issues
  • Improved wait conditions: Extended timeouts from 20s to 30s in critical areas and added explicit waitFor calls with appropriate states (visible, attached, detached)
  • Better selector strategies: Replaced fragile direct clicks with robust wait-then-click patterns, and added fallback selectors for dashboard deletion
  • Refactored URL import: Extracted URL tab click logic into dedicated clickUrlImportTab() method with built-in retry logic
  • More resilient JSON parsing: Improved title extraction from JSON editor using regex matching instead of simple text extraction
  • Strengthened page load detection: Enhanced waitForDashboardPage() with Promise.race to handle multiple success conditions (API response, table visibility, or import button)

Style Issues

  • Three instances of console.log/console.warn instead of testLogger (violates project logging standards)

Confidence Score: 4/5

  • This PR is safe to merge with minimal risk - changes are confined to test code with comprehensive stability improvements
  • The PR makes exclusively test-related changes that improve stability without affecting production code. The retry logic and wait conditions are well-implemented with reasonable timeouts. Only minor style issues exist (console statements instead of testLogger), which don't affect functionality.
  • All files are well-structured, but dashboard-import.js:139 and dashboard-import.spec.js:15 should update console statements to use testLogger for consistency

Important Files Changed

File Analysis

Filename Score Overview
tests/ui-testing/pages/dashboardPages/dashboard-import.js 4/5 Added comprehensive retry logic, wait conditions, and error handling for import/deletion operations. Minor style issues with console statements instead of testLogger.
tests/ui-testing/playwright-tests/dashboards/dashboard-import.spec.js 4/5 Improved test stability with better selectors, wait conditions, and refactored URL import logic. Minor style issue with console.log usage.
tests/ui-testing/playwright-tests/dashboards/utils/dashCreation.js 5/5 Enhanced waitForDashboardPage with more robust Promise.race logic and additional fallback conditions. Uses testLogger appropriately.

Sequence Diagram

sequenceDiagram
    participant Test as Test Spec
    participant PM as PageManager
    participant DI as DashboardImport
    participant Page as Playwright Page
    participant UI as UI Elements
    
    Note over Test,UI: Dashboard Import Flow (Enhanced)
    
    Test->>PM: dashboardImport.clickImportDashboard()
    PM->>DI: clickImportDashboard()
    DI->>UI: Wait for import button (15s)
    DI->>UI: Click import button
    DI->>UI: Wait for file tab (10s)
    DI->>UI: Wait for input attached (10s)
    
    alt File Upload
        Test->>DI: uploadDashboardFile(path)
        DI->>UI: Wait for file tab visible (10s)
        DI->>UI: Click file tab
        loop Retry up to 3 times
            DI->>UI: setInputFiles(path)
            alt Success
                DI->>UI: Break retry loop
            else Failure
                DI->>UI: Wait for input attached (5s)
            end
        end
        DI->>UI: Wait for JSON editor visible (10s)
        DI->>UI: Wait for view lines visible (10s)
    else URL Import
        Test->>DI: clickUrlImportTab()
        loop Retry up to 5 times
            DI->>UI: Wait for URL tab (30s)
            DI->>UI: Click URL tab
            DI->>UI: Wait for URL input visible (10s)
            alt Success
                DI->>UI: Break retry loop
            else Failure
                DI->>UI: Wait for tab visible (5s)
            end
        end
        Test->>Page: Fill URL input
        Test->>Page: Wait for JSON content
    end
    
    Test->>DI: clickImportButton()
    DI->>UI: Wait for import button
    DI->>UI: Click import button
    
    Test->>Page: waitForDashboardPage()
    Page->>UI: Wait for URL navigation (30s)
    Page->>UI: Wait for page stabilization (1s)
    alt Promise.race (first to complete)
        Page->>UI: Wait for API response (30s)
        Page->>UI: Wait for dashboard table (30s)
        Page->>UI: Wait for import button (30s)
    end
    Page->>UI: Final stabilization wait (1s)
    
    Test->>DI: deleteImportedDashboard(prefix, name)
    DI->>UI: Wait for dashboard table (20s)
    DI->>UI: Wait for first row (15s)
    
    loop Retry up to 5 times to find row
        DI->>UI: Wait for dashboard row (4s)
        alt Row found
            DI->>UI: Break retry loop
        else Not found
            alt Final retry
                DI->>UI: Fallback to cell-based selector
                DI->>UI: Click cell
                DI->>UI: Click delete button
                DI->>UI: Click confirm
            else Continue retry
                DI->>UI: Wait for DOM loaded
            end
        end
    end
    
    loop Retry up to 3 times to delete
        DI->>UI: Hover over row
        DI->>UI: Wait for delete button (10s)
        DI->>UI: Click delete button
        DI->>UI: Wait for confirm button (5s)
        alt Success
            DI->>UI: Break retry loop
        else Failure
            DI->>UI: Wait for row visible (5s)
        end
    end
    
    DI->>UI: Click confirm button (10s)
    
    alt Verify deletion (fallback chain)
        DI->>UI: Wait for row detached (10s)
    else Fallback 1
        DI->>UI: Wait for networkidle (5s)
    else Fallback 2
        DI->>UI: Wait for row hidden (5s)
    else Fallback 3
        DI->>DI: Log warning via console
    end
Loading

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Additional Comments (1)

  1. tests/ui-testing/playwright-tests/dashboards/dashboard-import.spec.js, line 15 (link)

    style: Use testLogger.info() instead of console.log() for consistency with the project's logging framework

    Context Used: Context from dashboard - Use a logging framework instead of print statements for debugging in tests to allow better control o... (source)

3 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile

@ktx-akshay ktx-akshay force-pushed the stabilize-dashboard-test-suite branch from a55d65b to b9ca689 Compare December 3, 2025 12:39
@ktx-akshay ktx-akshay merged commit 32cc06a into main Dec 3, 2025
47 of 54 checks passed
@ktx-akshay ktx-akshay deleted the stabilize-dashboard-test-suite branch December 3, 2025 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants