mirror of
https://github.com/GreptimeTeam/greptimedb.git
synced 2026-08-18 20:18:30 +00:00
fix(ci): render query regression bot comment as compact table plus threshold details (#8774)
Move case/storage thresholds out of the main summary table into a collapsible <details> block below it, so measured cases render one row per query instead of an extra Query=N/A storage row that stretched the table. Drop the backtick code span around the status cell (the escaping in text() turned it into a visible backtick) and unify multi-threshold separators on '; '. - query-regression-comment.cjs: classifyThresholds shared helper, syntheticThresholdDetail for case-level thresholds, kind marker on synthetic rows, renderThresholdDetails emitting unescaped details tags with text()-escaped entry content, plain-text status cell - query-regression-comment.test.cjs: updated separator/kind assertions, new test covering details-block rendering and plain-text status Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
This commit is contained in:
@@ -92,7 +92,7 @@ function thresholdStatus(thresholds, query) {
|
||||
const hits = (Array.isArray(thresholds) ? thresholds : [])
|
||||
.filter(item => query === undefined || (hasScopedQuery(item) && String(item.query) === query))
|
||||
.map(formatThreshold);
|
||||
return hits.length > 0 ? hits.join(', ') : 'N/A';
|
||||
return hits.length > 0 ? hits.join('; ') : 'N/A';
|
||||
}
|
||||
|
||||
function hasScopedQuery(threshold) {
|
||||
@@ -115,7 +115,7 @@ function formatThreshold(threshold) {
|
||||
return `${name}${scope.length > 0 ? ` [${scope.join(', ')}]` : ''}: ${status}${reason}`;
|
||||
}
|
||||
|
||||
function syntheticThresholdStatus(thresholds, measurementNames) {
|
||||
function classifyThresholds(thresholds, measurementNames) {
|
||||
const unscoped = [];
|
||||
const unmatched = new Map();
|
||||
for (const threshold of Array.isArray(thresholds) ? thresholds : []) {
|
||||
@@ -130,7 +130,11 @@ function syntheticThresholdStatus(thresholds, measurementNames) {
|
||||
unmatched.set(query, entries);
|
||||
}
|
||||
}
|
||||
return { unscoped, unmatched };
|
||||
}
|
||||
|
||||
function syntheticThresholdStatus(thresholds, measurementNames) {
|
||||
const { unscoped, unmatched } = classifyThresholds(thresholds, measurementNames);
|
||||
const parts = [];
|
||||
if (unscoped.length > 0) {
|
||||
parts.push(`case/storage threshold: ${thresholdStatus(unscoped)}`);
|
||||
@@ -141,6 +145,21 @@ function syntheticThresholdStatus(thresholds, measurementNames) {
|
||||
return parts.length > 0 ? parts.join('; ') : 'N/A';
|
||||
}
|
||||
|
||||
// Case-level (query-unscoped) thresholds formatted for the collapsible details
|
||||
// block below the summary table: individual items, `; `-separated, without the
|
||||
// `case/storage threshold:` label (the section heading already says that).
|
||||
function syntheticThresholdDetail(thresholds, measurementNames) {
|
||||
const { unscoped, unmatched } = classifyThresholds(thresholds, measurementNames);
|
||||
const items = [];
|
||||
if (unscoped.length > 0) {
|
||||
items.push(thresholdStatus(unscoped));
|
||||
}
|
||||
for (const query of Array.from(unmatched.keys()).sort()) {
|
||||
items.push(`unmatched query ${query}: ${thresholdStatus(unmatched.get(query))}`);
|
||||
}
|
||||
return items.length > 0 ? items.join('; ') : 'N/A';
|
||||
}
|
||||
|
||||
function joinDetails(...details) {
|
||||
const present = details.filter(detail => detail && detail !== 'N/A');
|
||||
return present.length > 0 ? present.join('; ') : 'N/A';
|
||||
@@ -229,7 +248,7 @@ function collectReportRows(report, reportPath) {
|
||||
),
|
||||
};
|
||||
});
|
||||
const syntheticThresholds = syntheticThresholdStatus(thresholds, measurementNames);
|
||||
const syntheticThresholds = syntheticThresholdDetail(thresholds, measurementNames);
|
||||
if (syntheticThresholds !== 'N/A') {
|
||||
rows.push({
|
||||
caseName: name,
|
||||
@@ -239,22 +258,46 @@ function collectReportRows(report, reportPath) {
|
||||
candidateMedian: 'N/A',
|
||||
regression: 'N/A',
|
||||
threshold: syntheticThresholds,
|
||||
kind: 'case-thresholds',
|
||||
});
|
||||
}
|
||||
return rows;
|
||||
}
|
||||
|
||||
function renderThresholdDetails(rows) {
|
||||
const entries = rows.filter(row => row.kind === 'case-thresholds');
|
||||
if (entries.length === 0) {
|
||||
return '';
|
||||
}
|
||||
// The <details>/<summary> tags are emitted literally; only the entry content
|
||||
// goes through text() (so `[ ] ( ) |` etc. stay escaped while the tags render).
|
||||
const lines = [
|
||||
'<details><summary>Case / storage thresholds</summary>',
|
||||
'',
|
||||
];
|
||||
for (const row of entries) {
|
||||
lines.push(`- ${text(row.caseName)}: ${text(row.threshold)}`);
|
||||
}
|
||||
lines.push('</details>');
|
||||
return lines.join('\n');
|
||||
}
|
||||
|
||||
function renderSummaryTable(rows) {
|
||||
const lines = [
|
||||
'| Case | Query | Case status | Base median ms | Candidate median ms | Regression | Threshold |',
|
||||
'| --- | --- | --- | ---: | ---: | ---: | --- |',
|
||||
];
|
||||
for (const row of rows) {
|
||||
if (row.kind === 'case-thresholds') {
|
||||
continue;
|
||||
}
|
||||
lines.push(
|
||||
`| ${text(row.caseName)} | ${text(row.query)} | ${statusEmoji(row.status)} \`${text(row.status)}\` | ${text(row.baseMedian)} | ${text(row.candidateMedian)} | ${text(row.regression)} | ${text(row.threshold)} |`
|
||||
`| ${text(row.caseName)} | ${text(row.query)} | ${statusEmoji(row.status)} ${text(row.status)} | ${text(row.baseMedian)} | ${text(row.candidateMedian)} | ${text(row.regression)} | ${text(row.threshold)} |`
|
||||
);
|
||||
}
|
||||
return lines.join('\n');
|
||||
const details = renderThresholdDetails(rows);
|
||||
const table = lines.join('\n');
|
||||
return details === '' ? table : `${table}\n\n${details}`;
|
||||
}
|
||||
|
||||
module.exports = async function validateQueryRegressionComment({ github, context, core }) {
|
||||
|
||||
@@ -187,11 +187,11 @@ test('sorts the base and candidate query union and aggregates scoped thresholds'
|
||||
assert.equal(rows[0].threshold, 'base measurement missing');
|
||||
assert.equal(
|
||||
rows[1].threshold,
|
||||
'candidate measurement missing; p95 [target=base]: warn, absolute [target=candidate, encoding=plain]: pass'
|
||||
'candidate measurement missing; p95 [target=base]: warn; absolute [target=candidate, encoding=plain]: pass'
|
||||
);
|
||||
});
|
||||
|
||||
test('preserves unscoped and unmatched thresholds in a synthetic N/A row', () => {
|
||||
test('keeps unscoped and unmatched thresholds in a case-thresholds detail row', () => {
|
||||
const rows = collectReportRows(report('thresholds', {
|
||||
base: [{ name: 'measured', latency_ms_median: 10 }],
|
||||
candidate: [{ name: 'measured', latency_ms_median: 11 }],
|
||||
@@ -214,9 +214,10 @@ test('preserves unscoped and unmatched thresholds in a synthetic N/A row', () =>
|
||||
assert.equal(rows[0].query, 'measured');
|
||||
assert.equal(rows[0].threshold, 'query limit [target=base]: passed');
|
||||
assert.equal(rows[1].query, 'N/A');
|
||||
assert.equal(rows[1].kind, 'case-thresholds');
|
||||
assert.equal(
|
||||
rows[1].threshold,
|
||||
'case/storage threshold: min_files [target=base]: passed, min_files [target=candidate]: failed, encoding limit [target=candidate, encoding=plain]: failed; unmatched query not-measured: orphaned limit [target=base, encoding=json]: failed (reason: measurement unavailable)'
|
||||
'min_files [target=base]: passed; min_files [target=candidate]: failed; encoding limit [target=candidate, encoding=plain]: failed; unmatched query not-measured: orphaned limit [target=base, encoding=json]: failed (reason: measurement unavailable)'
|
||||
);
|
||||
});
|
||||
|
||||
@@ -239,12 +240,50 @@ test('keeps unscoped thresholds out of undefined and null query rows', () => {
|
||||
assert.equal(byQuery.get('undefined').threshold, 'N/A');
|
||||
assert.equal(byQuery.get('null').threshold, 'N/A');
|
||||
assert.equal(rows.filter(row => row.query === 'N/A').length, 1);
|
||||
assert.equal(byQuery.get('N/A').kind, 'case-thresholds');
|
||||
assert.equal(
|
||||
byQuery.get('N/A').threshold,
|
||||
'case/storage threshold: min_files [target=base]: passed, min_files [target=candidate]: failed'
|
||||
'min_files [target=base]: passed; min_files [target=candidate]: failed'
|
||||
);
|
||||
});
|
||||
|
||||
test('renders case/storage thresholds in a collapsible details block below the table', () => {
|
||||
const rows = [
|
||||
...collectReportRows(report('first', {
|
||||
base: [{ name: 'q1', latency_ms_median: 10 }],
|
||||
candidate: [{ name: 'q1', latency_ms_median: 11 }],
|
||||
}, [
|
||||
{ query: 'q1', threshold: 'query limit', target: 'base', status: 'passed' },
|
||||
{ threshold: 'min_files', target: 'base', status: 'passed' },
|
||||
{ threshold: 'min_files', target: 'candidate', status: 'failed' },
|
||||
]), '/reports/first/query-regression-report.json'),
|
||||
...collectReportRows({
|
||||
case: { name: 'broken' },
|
||||
status: 'failed',
|
||||
error: 'connection refused',
|
||||
}, '/reports/broken/query-regression-report.json'),
|
||||
];
|
||||
|
||||
const rendered = renderSummaryTable(rows);
|
||||
const detailsIndex = rendered.indexOf('<details>');
|
||||
const table = rendered.slice(0, detailsIndex);
|
||||
const details = rendered.slice(detailsIndex);
|
||||
|
||||
// Query rows and abnormal N/A rows stay in the main table.
|
||||
assert.match(table, /\| first \| q1 \| ✅ ok \|/);
|
||||
assert.match(table, /\| broken \| N\/A \| ❌ failed \|/);
|
||||
// Storage thresholds are not main-table rows.
|
||||
assert.doesNotMatch(table, /^\| first \| N\/A \|/m);
|
||||
assert.doesNotMatch(table, /min_files/);
|
||||
// Status column is plain text without code-span backticks.
|
||||
assert.doesNotMatch(table, /`/);
|
||||
// Details block keeps its tags unescaped and its items '; '-separated.
|
||||
assert.ok(details.startsWith('<details><summary>Case / storage thresholds</summary>'));
|
||||
assert.ok(details.includes('- first: min_files \\[target=base\\]: passed; min_files \\[target=candidate\\]: failed'));
|
||||
assert.ok(details.endsWith('</details>'));
|
||||
assert.ok(!rendered.includes('<details>'));
|
||||
});
|
||||
|
||||
test('escapes Markdown table content, including bare carriage returns', () => {
|
||||
const table = renderSummaryTable([{
|
||||
caseName: 'safe\r| injected |\n<!-- hidden -->@user <tag>',
|
||||
|
||||
Reference in New Issue
Block a user