Skip to content

Commit

Permalink
Debugging run
Browse files Browse the repository at this point in the history
  • Loading branch information
philrenaud committed Dec 4, 2024
1 parent 65fe72a commit 31789fa
Show file tree
Hide file tree
Showing 2 changed files with 167 additions and 35 deletions.
147 changes: 128 additions & 19 deletions .github/workflows/test-ui.yml
Original file line number Diff line number Diff line change
Expand Up @@ -143,34 +143,140 @@ 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

Check warning

Code scanning / GitHub Actions Scanner

Missing pinned commit hash for GitHub Actions configuration Warning test

found external action "actions/github-script@v7" without pinned version hash
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

Check warning

Code scanning / GitHub Actions Scanner

Missing pinned commit hash for GitHub Actions configuration Warning test

found external action "actions/github-script@v7" without pinned version hash
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
with:
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`;
Expand All @@ -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,
Expand Down
55 changes: 39 additions & 16 deletions scripts/analyze-ui-test-times.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 => {
Expand All @@ -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 = {
Expand All @@ -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
});

Expand All @@ -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)
);

Expand Down

0 comments on commit 31789fa

Please sign in to comment.