Skip to content

Commit

Permalink
Debugging run
Browse files Browse the repository at this point in the history
  • Loading branch information
@philrenaud
philrenaud committed Dec 5, 2024
1 parent 65fe72a commit 7e6e8c6
Show file tree
Hide file tree
Showing 2 changed files with 227 additions and 39 deletions.
175 changes: 156 additions & 19 deletions .github/workflows/test-ui.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,14 +127,19 @@ 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'
uses: actions/upload-artifact@b4b15b8c7c6ac21ea08fcf65892d2ee8f75cf882 # v4.4.3
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:
Expand All @@ -143,34 +148,163 @@ 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
});
// 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, 'combined-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 {
const files = fs.readdirSync(tempDir);
console.log(`Warning: No test results JSON found in ${artifact.name}. Found files:`, files);
console.log(`Warning: No combined-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
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 +313,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
91 changes: 71 additions & 20 deletions scripts/analyze-ui-test-times.js
View file
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,32 +19,73 @@ 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;
historicalAverages.set(test.name, current + test.duration);
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(),
Expand All @@ -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
});

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

Expand Down

0 comments on commit 7e6e8c6

Please sign in to comment.