Skip to content

Commit c7eb4d2

Browse files
committed
Improve benchmark result validation and reporting
Enhanced the benchmark workflows to handle empty or malformed benchmark-results.json files, provide clearer error messages, and ensure benchmark data is present before posting comments or summaries. Updated the Maven command to use --batch-mode and package phase for more reliable execution.
1 parent c1dac8a commit c7eb4d2

2 files changed

Lines changed: 70 additions & 24 deletions

File tree

.github/workflows/benchmark-comment.yml

Lines changed: 64 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -38,38 +38,75 @@ jobs:
3838
3939
const resultsPath = path.join(process.cwd(), 'benchmark', 'benchmark-results.json');
4040
if (!fs.existsSync(resultsPath)) {
41-
core.info('benchmark-results.json not found in artifact; skipping comment.');
41+
core.setFailed('benchmark-results.json not found in artifact.');
4242
return;
4343
}
4444
45-
const results = JSON.parse(fs.readFileSync(resultsPath, 'utf8'));
46-
const runUrl = context.payload.workflow_run.html_url;
47-
48-
let comment = '## 📊 Benchmark Results\\n\\n';
49-
comment += `Triggered by [workflow run](${runUrl}).\\n\\n`;
50-
comment += '| Benchmark | Score | Error | Unit |\\n';
51-
comment += '|-----------|-------|-------|------|\\n';
52-
53-
let hasRegression = false;
54-
55-
results.forEach(result => {
56-
const score = result.primaryMetric.score;
57-
const error = result.primaryMetric.scoreError;
58-
const unit = result.primaryMetric.scoreUnit;
59-
const benchmark = result.benchmark.split('.').pop();
45+
const raw = fs.readFileSync(resultsPath, 'utf8').trim();
46+
if (!raw) {
47+
core.setFailed('benchmark-results.json is empty');
48+
return;
49+
}
6050
61-
const emoji = score > 50 ? '🔴' : '🟢';
62-
if (score > 50) hasRegression = true;
51+
let parsed;
52+
try {
53+
parsed = JSON.parse(raw);
54+
} catch (error) {
55+
core.setFailed(`Failed to parse benchmark-results.json: ${error.message}`);
56+
throw error;
57+
}
6358
64-
comment += `| ${emoji} ${benchmark} | ${score.toFixed(3)} | ±${error.toFixed(3)} | ${unit} |\\n`;
65-
});
59+
const benchmarks = Array.isArray(parsed)
60+
? parsed
61+
: Array.isArray(parsed?.benchmarks)
62+
? parsed.benchmarks
63+
: [];
6664
67-
if (hasRegression) {
68-
comment += '\\n⚠️ **Performance regression detected**: One or more benchmarks exceeded the 50ms threshold.\\n';
65+
const runUrl = context.payload.workflow_run.html_url;
66+
const commentLines = [
67+
'## 📊 Benchmark Results',
68+
'',
69+
`Triggered by [workflow run](${runUrl}).`,
70+
'',
71+
];
72+
73+
if (benchmarks.length === 0) {
74+
commentLines.push('⚠️ **No benchmark data was produced.** Check the workflow run logs for failures in the benchmark phase.');
6975
} else {
70-
comment += '\\n✅ **All benchmarks passed** performance thresholds.\\n';
76+
commentLines.push('| Benchmark | Score | Error | Unit |');
77+
commentLines.push('|-----------|-------|-------|------|');
78+
79+
let hasRegression = false;
80+
81+
for (const result of benchmarks) {
82+
const score = Number(result?.primaryMetric?.score ?? NaN);
83+
const error = Number(result?.primaryMetric?.scoreError ?? NaN);
84+
const unit = result?.primaryMetric?.scoreUnit ?? 'ops';
85+
const benchmark = (result?.benchmark ?? 'unknown').split('.').pop();
86+
87+
if (!Number.isFinite(score)) {
88+
commentLines.push(`| ⚠️ ${benchmark} | n/a | n/a | ${unit} |`);
89+
core.warning(`Missing score for benchmark ${result?.benchmark ?? 'unknown'}`);
90+
continue;
91+
}
92+
93+
const emoji = score > 50 ? '🔴' : '🟢';
94+
if (score > 50) hasRegression = true;
95+
96+
const errorDisplay = Number.isFinite(error) ? error.toFixed(3) : 'n/a';
97+
commentLines.push(`| ${emoji} ${benchmark} | ${score.toFixed(3)} | ±${errorDisplay} | ${unit} |`);
98+
}
99+
100+
commentLines.push('');
101+
if (hasRegression) {
102+
commentLines.push('⚠️ **Performance regression detected**: One or more benchmarks exceeded the 50ms threshold.');
103+
} else {
104+
commentLines.push('✅ **All benchmarks passed** performance thresholds.');
105+
}
71106
}
72107
108+
const comment = commentLines.join('\n');
109+
73110
const { owner, repo } = context.repo;
74111
const issue_number = pr.number;
75112
@@ -98,3 +135,7 @@ jobs:
98135
body: comment,
99136
});
100137
}
138+
139+
if (benchmarks.length === 0) {
140+
core.setFailed('No benchmark entries were found in benchmark-results.json.');
141+
}

.github/workflows/benchmark.yml

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,17 @@ jobs:
2424

2525
- name: Run benchmarks
2626
run: |
27-
./mvnw clean test-compile exec:java -P benchmark -Djmh.options="-wi 2 -i 3 -f 1 -rf json -rff benchmark-results.json"
27+
./mvnw --batch-mode clean package exec:java -P benchmark -Djmh.options="-wi 2 -i 3 -f 1 -rf json -rff benchmark-results.json"
2828
2929
- name: Process benchmark results
3030
if: always()
3131
run: |
3232
if [ -f benchmark-results.json ]; then
33+
ENTRY_COUNT=$(jq 'length' benchmark-results.json)
34+
if [ "$ENTRY_COUNT" -eq 0 ]; then
35+
echo "::error::benchmark-results.json contained no benchmark entries"
36+
exit 1
37+
fi
3338
echo "## Benchmark Results" >> $GITHUB_STEP_SUMMARY
3439
echo '```' >> $GITHUB_STEP_SUMMARY
3540

0 commit comments

Comments
 (0)