diff --git a/.github/workflows/test-ui.yml b/.github/workflows/test-ui.yml index a52c0e7b61c..d71d801fd6a 100644 --- a/.github/workflows/test-ui.yml +++ b/.github/workflows/test-ui.yml @@ -127,6 +127,11 @@ jobs: name: test-results-${{ github.sha }} path: ui/combined-test-results.json retention-days: 90 + - name: Delete partition test results + if: github.event_name == 'push' && github.ref == 'refs/heads/main' || github.event_name == 'pull_request' + uses: geekyeggo/delete-artifact@f275313e70c08f6120db482d7a6b98377786765b # v5.1.0 + with: + name: test-results-* - name: Upload Current PR results if: github.event_name == 'pull_request' @@ -134,7 +139,7 @@ jobs: with: name: pr-test-results-${{ github.sha }} # Prefix with "pr-" to avoid comparing with main during analyze step path: ui/combined-test-results.json - retention-days: 7 + retention-days: 1 - name: finalize env: @@ -143,25 +148,154 @@ 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 + }); + + // Log out the names of each artifact + console.log('Available artifacts:'); + artifacts.data.artifacts.forEach(artifact => { + console.log(`- ${artifact.name}`); + }); + + const testArtifacts = artifacts.data.artifacts.filter(artifact => + artifact.name.startsWith('pr-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 }); + + try { + // 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`) + ); + console.log(`Successfully processed ${artifact.name}`); + } else { + console.log(`Warning: No test-results.json found in ${artifact.name}`); + } + } finally { + // Always clean up temp directory + if (fs.existsSync(tempDir)) { + fs.rmSync(tempDir, { recursive: true, force: true }); + } + } + } catch (error) { + console.log(`Error processing ${artifact.name}:`, error.message); + // Continue with next artifact + } + } + + # 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" + cd historical-results + echo -e "\nContents of each file (first 10 lines):" + for file in *.json; do + if [ -f "$file" ]; then + echo -e "\n=== $file ===" + head -n 10 "$file" + fi + done + - 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 +303,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 +311,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..42b0364b0ce 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,21 +19,47 @@ 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(); - - // 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}`) - ); - - if (historical.summary.failed === 0) { +// 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 historicalDir = 'historical-results'; +const historicalFiles = fs.readdirSync(historicalDir) + .filter(file => file.endsWith('.json')); + +console.log(`[analyze-test-times] Found ${historicalFiles.length} JSON files`); + + // Debug: Show content of first file + if (historicalFiles.length > 0) { + const firstFile = fs.readFileSync(path.join(historicalDir, historicalFiles[0]), 'utf8'); + console.log('\n[analyze-test-times] First file content sample:'); + console.log(firstFile.substring(0, 500) + '...'); + console.log('\n[analyze-test-times] First file parsed:'); + const parsed = JSON.parse(firstFile); + console.log(JSON.stringify(parsed, null, 2).substring(0, 500) + '...'); + } + +historicalFiles.forEach((file, index) => { + console.log(`[analyze-test-times] Reading ${file} (${index + 1} of ${historicalFiles.length})...`); + try { + const historical = JSON.parse(fs.readFileSync(path.join(historicalDir, file), 'utf8')); + + // Debug output + console.log(`[analyze-test-times] File ${file}:`); + console.log(` - Has summary: ${!!historical.summary}`); + if (historical.summary) { + console.log(` - Failed tests: ${historical.summary.failed}`); + console.log(` - Total tests: ${historical.summary.total}`); + } + console.log(` - Has tests array: ${!!historical.tests}`); + if (historical.tests) { + console.log(` - Number of tests: ${historical.tests.length}`); + } + + if (historical.summary && historical.summary.failed === 0) { historical.tests.forEach(test => { const current = historicalAverages.get(test.name) || 0; const count = historicalCounts.get(test.name) || 0; @@ -40,10 +67,25 @@ async function analyzeTestTimes() { historicalCounts.set(test.name, count + 1); }); } else { - console.log(`[analyze-test-times] Skipping ${file} because it has failed tests`); + console.log(`[analyze-test-times] Skipping ${file} because it has failed tests or invalid format`); } - }); + } catch (error) { + console.log(`[analyze-test-times] Error processing ${file}:`, error.message); + } +}); +// Debug output after processing +console.log('\n[analyze-test-times] Processing complete'); +console.log(`Total unique tests found: ${historicalAverages.size}`); +if (historicalAverages.size > 0) { + console.log('Sample of processed tests:'); + let i = 0; + for (const [name, total] of historicalAverages) { + if (i++ >= 3) break; + const count = historicalCounts.get(name); + console.log(`- ${name}: ${total}ms total, ${count} samples`); + } +} // Calculate averages and compare const analysis = { timestamp: new Date().toISOString(), @@ -56,20 +98,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 +137,7 @@ async function analyzeTestTimes() { // Write analysis results fs.writeFileSync( - '../ui/test-time-analysis.json', + 'test-time-analysis.json', JSON.stringify(analysis, null, 2) );