From 31789fa6bcd6ed53273b81f40090e3459c0ba7a7 Mon Sep 17 00:00:00 2001 From: Phil Renaud Date: Wed, 4 Dec 2024 10:57:40 -0500 Subject: [PATCH] Debugging run --- .github/workflows/test-ui.yml | 147 +++++++++++++++++++++++++++---- scripts/analyze-ui-test-times.js | 55 ++++++++---- 2 files changed, 167 insertions(+), 35 deletions(-) diff --git a/.github/workflows/test-ui.yml b/.github/workflows/test-ui.yml index a52c0e7b61c..60e493c96a7 100644 --- a/.github/workflows/test-ui.yml +++ b/.github/workflows/test-ui.yml @@ -143,25 +143,133 @@ jobs: run: yarn percy build:finalize analyze-times: - needs: [tests, finalize] + # TODO: temporary comment-out with hardcoded sha + # needs: [tests, finalize] if: github.event_name == 'pull_request' runs-on: ubuntu-latest + defaults: + run: + working-directory: ui + steps: - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - - uses: actions/download-artifact@fa0a91b85d4f404e444e00e005971372dc801d16 # v4.1.8 + + # Debug step to show environment + - name: Debug environment + run: | + echo "GITHUB_SHA: ${{ github.sha }}" + echo "GITHUB_EVENT_NAME: ${{ github.event_name }}" + echo "GITHUB_REF: ${{ github.ref }}" + echo "RUN_ID: ${{ github.run_id }}" + + # Try to list available artifacts first + - name: List artifacts + uses: actions/github-script@v7 with: - name: test-results-${{ github.sha }} + script: | + const artifacts = await github.rest.actions.listWorkflowRunArtifacts({ + owner: context.repo.owner, + repo: context.repo.repo, + // run_id: context.runId + run_id: 12163157778 + }); + console.log('Available artifacts:'); + console.log(JSON.stringify(artifacts.data, null, 2)); + + - name: Download current PR results + uses: actions/download-artifact@fa0a91b85d4f404e444e00e005971372dc801d16 # v4.1.8 + with: + # name: test-results-${{ github.sha }} + name: pr-test-results-fe7ca11e9afc42bc98d79fe521155a37634bd232 # TODO: temporary hardcoded sha from previous run path: ui - + run-id: 12163157778 + github-token: ${{ secrets.GITHUB_TOKEN }} + + # - name: Download historical results + # uses: actions/download-artifact@fa0a91b85d4f404e444e00e005971372dc801d16 # v4.1.8 + # with: + # pattern: test-results-* + # path: historical-results + # merge-multiple: true + + # Download historical results from previous main branch runs - name: Download historical results - uses: actions/download-artifact@fa0a91b85d4f404e444e00e005971372dc801d16 # v4.1.8 + uses: actions/github-script@v7 + env: + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} with: - pattern: test-results-* - path: historical-results - merge-multiple: true - + script: | + const fs = require('fs'); + const path = require('path'); + + const historicalDir = path.join('ui', 'historical-results'); + // Clean up any existing directory + if (fs.existsSync(historicalDir)) { + fs.rmSync(historicalDir, { recursive: true, force: true }); + } + fs.mkdirSync(historicalDir, { recursive: true }); + + const artifacts = await github.rest.actions.listArtifactsForRepo({ + owner: context.repo.owner, + repo: context.repo.repo, + per_page: 100 + }); + + const testArtifacts = artifacts.data.artifacts.filter(artifact => + artifact.name.startsWith('test-results-') + ); + + console.log(`Found ${testArtifacts.length} test result artifacts`); + + for (const artifact of testArtifacts) { + try { + console.log(`Downloading ${artifact.name}`); + + // Create a temporary directory for this artifact + const tempDir = path.join(historicalDir, `temp-${artifact.id}`); + fs.mkdirSync(tempDir, { recursive: true }); + + // Download to temp directory + await exec.exec('gh', [ + 'run', + 'download', + '-n', + artifact.name, + '--repo', + `${context.repo.owner}/${context.repo.repo}`, + '--dir', + tempDir, + artifact.workflow_run.id.toString() + ]); + + // Move the JSON file to the historical directory with a unique name + const jsonFile = path.join(tempDir, 'test-results.json'); + if (fs.existsSync(jsonFile)) { + fs.renameSync( + jsonFile, + path.join(historicalDir, `${artifact.name}-${artifact.id}.json`) + ); + } + + // Clean up temp directory + fs.rmSync(tempDir, { recursive: true, force: true }); + + console.log(`Successfully processed ${artifact.name}`); + } catch (error) { + console.log(`Error processing ${artifact.name}:`, error.message); + } + } + + # Debug what we got + - name: Debug directories + run: | + echo "Current directory structure:" + ls -la + printf "\nHistorical results directory:\n" + ls -la historical-results || echo "historical-results directory not found" + - name: Analyze test times - run: node scripts/analyze-test-times.js + run: node ../scripts/analyze-ui-test-times.js - name: Comment PR uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 @@ -169,8 +277,6 @@ jobs: script: | const fs = require('fs'); const analysis = JSON.parse(fs.readFileSync('ui/test-time-analysis.json')); - - console.log('=== analysis', analysis); let body = `### Test Time Analysis\n\n`; body += `- Total Tests: ${analysis.summary.totalTests}\n`; @@ -179,15 +285,18 @@ jobs: if (analysis.testComparisons.length > 0) { body += `#### Most Significant Changes:\n\n`; - analysis.testComparisons.slice(0, 5).forEach(comp => { - body += `**${comp.name}**\n`; - body += `- Current: ${comp.currentDuration}ms\n`; - body += `- Historical Avg: ${comp.historicalAverage}ms\n`; - body += `- Change: ${comp.percentDiff.toFixed(1)}%\n\n`; - }); + analysis.testComparisons + .filter(comp => comp.percentDiff != null) // Skip invalid comparisons + .slice(0, 5) + .forEach(comp => { + body += `**${comp.name}**\n`; + body += `- Current: ${comp.currentDuration}ms\n`; + body += `- Historical Avg: ${comp.historicalAverage}ms\n`; + body += `- Change: ${comp.percentDiff?.toFixed(1) || 'N/A'}%\n\n`; + }); } - github.rest.issues.createComment({ + await github.rest.issues.createComment({ issue_number: context.issue.number, owner: context.repo.owner, repo: context.repo.repo, diff --git a/scripts/analyze-ui-test-times.js b/scripts/analyze-ui-test-times.js index bad4c738372..c070d4d0dcf 100644 --- a/scripts/analyze-ui-test-times.js +++ b/scripts/analyze-ui-test-times.js @@ -6,10 +6,11 @@ 'use strict'; const fs = require('fs'); +const path = require('path'); async function analyzeTestTimes() { const currentResults = JSON.parse( - fs.readFileSync('../ui/combined-test-results.json') + fs.readFileSync('combined-test-results.json') ); // Create a map of test names to their durations @@ -18,19 +19,29 @@ async function analyzeTestTimes() { currentTestTimes.set(test.name, test.duration); }); - // Load and process historical results - console.log('[analyze-test-times] Processing historical results...\n'); - const historicalAverages = new Map(); - const historicalCounts = new Map(); +// Load and process historical results +console.log('[analyze-test-times] Processing historical results...\n'); +const historicalAverages = new Map(); +const historicalCounts = new Map(); - // Read each historical result file - console.log('[analyze-test-times] Reading historical results directory...\n'); - const historicalFiles = fs.readdirSync('../historical-results'); - historicalFiles.forEach((file, index) => { - console.log(`[analyze-test-times] Reading ${file} (${index + 1} of ${historicalFiles.length})...`); - const historical = JSON.parse( - fs.readFileSync(`../historical-results/${file}`) - ); +// Read each historical result file +console.log('[analyze-test-times] Reading historical results directory...\n'); +const historicalDir = path.join('ui', 'historical-results'); +const historicalFiles = fs.readdirSync(historicalDir) + .filter(file => file.endsWith('.json')); // Only process JSON files + +historicalFiles.forEach((file, index) => { + console.log(`[analyze-test-times] Reading ${file} (${index + 1} of ${historicalFiles.length})...`); + const filePath = path.join(historicalDir, file); + + // Skip if it's a directory + if (fs.statSync(filePath).isDirectory()) { + console.log(`[analyze-test-times] Skipping directory: ${file}`); + return; + } + + try { + const historical = JSON.parse(fs.readFileSync(filePath, 'utf8')); if (historical.summary.failed === 0) { historical.tests.forEach(test => { @@ -42,7 +53,10 @@ async function analyzeTestTimes() { } else { console.log(`[analyze-test-times] Skipping ${file} because it has failed tests`); } - }); + } catch (error) { + console.log(`[analyze-test-times] Error processing ${file}:`, error.message); + } +}); // Calculate averages and compare const analysis = { @@ -56,20 +70,29 @@ async function analyzeTestTimes() { testComparisons: [] }; + // In the analyzeTestTimes function: + console.log('[analyze-test-times] Comparing current test times with historical averages...'); currentTestTimes.forEach((currentDuration, testName) => { const totalHistorical = historicalAverages.get(testName) || 0; const count = historicalCounts.get(testName) || 1; const historicalAverage = totalHistorical / count; + // Skip tests with no historical data + if (historicalAverage === 0) { + console.log(`[analyze-test-times] Skipping ${testName} - no historical data`); + return; + } + // Consider a test significantly different if it's 25% slower/faster const percentDiff = ((currentDuration - historicalAverage) / historicalAverage) * 100; if (Math.abs(percentDiff) >= 25) { + console.log(`[analyze-test-times] Found significant difference in ${testName}: ${percentDiff.toFixed(1)}% change`); analysis.testComparisons.push({ name: testName, currentDuration, historicalAverage, - percentDiff, + percentDiff, // This will now always be a number samples: count }); @@ -86,7 +109,7 @@ async function analyzeTestTimes() { // Write analysis results fs.writeFileSync( - '../ui/test-time-analysis.json', + 'test-time-analysis.json', JSON.stringify(analysis, null, 2) );