Skip to content

Conversation

@nmgarza5
Copy link
Contributor

@nmgarza5 nmgarza5 commented Oct 25, 2025

Description

This change enables recording trace files for failing playwright tests in CI. These trace files are great for debugging failed tests. https://playwright.dev/docs/trace-viewer

How Has This Been Tested?

ci

Additional Options

  • [Optional] Override Linear Check

@nmgarza5 nmgarza5 requested a review from a team as a code owner October 25, 2025 05:08
@vercel
Copy link

vercel bot commented Oct 25, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
internal-search Ready Ready Preview Comment Oct 25, 2025 5:15am

greptile-apps[bot]

This comment was marked as outdated.

Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

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

No issues found across 2 files

@nmgarza5 nmgarza5 force-pushed the nikg/e2e/record-traces branch from cac16bc to 5663920 Compare October 25, 2025 05:12
@nmgarza5
Copy link
Contributor Author

@greptile

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.

Greptile Overview

Greptile Summary

This PR enables Playwright trace recording for failed tests in CI, which will significantly improve debugging capabilities for test failures. The implementation is straightforward and follows Playwright best practices.

Key Changes:

  • Added trace: "retain-on-failure" to Playwright config to capture trace files only when tests fail
  • Configured outputDir: "test-results" to ensure traces are saved in the correct location
  • Updated GitHub workflow artifact upload to include trace files with updated comment
  • Minor whitespace cleanup in workflow file (trailing spaces removed)

The test-results directory is already properly gitignored, and the artifact retention is set to 30 days, which is appropriate for debugging purposes.

Confidence Score: 5/5

  • This PR is safe to merge with no risk - it only adds test debugging capabilities
  • The changes are minimal, well-scoped, and follow Playwright's official documentation. The trace recording only activates on test failures, adding no overhead to passing tests. The configuration is already supported by the existing gitignore rules, and the workflow changes are purely additive.
  • No files require special attention

Important Files Changed

File Analysis

Filename Score Overview
web/playwright.config.ts 5/5 Added trace recording on test failure with proper output directory configuration - clean implementation
.github/workflows/pr-playwright-tests.yml 5/5 Updated artifact upload comment and path to include trace files - whitespace cleanup included

Sequence Diagram

sequenceDiagram
    participant GH as GitHub Actions
    participant PW as Playwright Test Runner
    participant FS as File System
    participant Artifact as Artifact Storage

    GH->>GH: Start Docker containers
    GH->>GH: Install Playwright & dependencies
    GH->>PW: Run tests with config
    
    alt Test Passes
        PW->>PW: Execute test
        PW->>GH: Report success (no trace)
    else Test Fails
        PW->>PW: Execute test
        PW->>FS: Record trace to test-results/
        PW->>GH: Report failure
    end
    
    GH->>FS: Check test-results/ directory
    FS->>GH: Return test results + trace.zip files
    GH->>Artifact: Upload all files to artifact storage
    Artifact->>Artifact: Retain for 30 days
Loading

2 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

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