Merge "Expand INFO check chips, if there are no errors and running runs"
diff --git a/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts b/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
index b726292..77a51db 100644
--- a/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary.ts
@@ -70,6 +70,8 @@
const DETAILS_QUOTA: Map<RunStatus | Category, number> = new Map();
DETAILS_QUOTA.set(Category.ERROR, 7);
DETAILS_QUOTA.set(Category.WARNING, 2);
+DETAILS_QUOTA.set(Category.INFO, 2);
+DETAILS_QUOTA.set(Category.SUCCESS, 2);
DETAILS_QUOTA.set(RunStatus.RUNNING, 2);
@customElement('gr-change-summary')
@@ -417,8 +419,27 @@
if (hasResultsOf(run, category)) return true;
return category === Category.SUCCESS && hasCompletedWithoutResults(run);
});
+ const hasRunning = this.runs.some(isRunningOrScheduled);
+ const hasWarning = this.runs.some(run =>
+ hasResultsOf(run, Category.WARNING)
+ );
+ const hasError = this.runs.some(run => hasResultsOf(run, Category.ERROR));
const count = (run: CheckRun) => getResultsOf(run, category);
- if (category === Category.SUCCESS || category === Category.INFO) {
+
+ // Sometimes INFO and SUCCESS results should not consume much UI space and
+ // not grab any attention, e.g. when there are errors. Then let's
+ // aggressively collapse them into one small chip. But if INFO and SUCCESS
+ // is all we have, then make use of the one line we have and show expanded
+ // chips.
+ if (
+ category === Category.SUCCESS &&
+ (hasRunning || hasError || hasWarning || runs.length > 3)
+ ) {
+ return this.renderChecksChipsCollapsed(runs, category, count);
+ } else if (
+ category === Category.INFO &&
+ (hasRunning || hasError || runs.length > 3)
+ ) {
return this.renderChecksChipsCollapsed(runs, category, count);
}
return this.renderChecksChipsExpanded(runs, category);
diff --git a/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary_test.ts b/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary_test.ts
index 05036ab..3c5371f 100644
--- a/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-summary/gr-change-summary_test.ts
@@ -6,13 +6,15 @@
import '../../../test/common-test-setup';
import {fixture, html, assert} from '@open-wc/testing';
import {GrChangeSummary} from './gr-change-summary';
-import {queryAndAssert} from '../../../utils/common-util';
+import {queryAll, queryAndAssert} from '../../../utils/common-util';
import {fakeRun0} from '../../../models/checks/checks-fakes';
import {
createAccountWithEmail,
+ createCheckResult,
createComment,
createCommentThread,
createDraft,
+ createRun,
} from '../../../test/test-data-generators';
import {stubFlags} from '../../../test/test-utils';
import {Timestamp} from '../../../api/rest-api';
@@ -22,6 +24,9 @@
CommentsModel,
commentsModelToken,
} from '../../../models/comments/comments-model';
+import {GrChecksChip} from './gr-checks-chip';
+import {CheckRun} from '../../../models/checks/checks-model';
+import {Category, RunStatus} from '../../../api/checks';
suite('gr-change-summary test', () => {
let element: GrChangeSummary;
@@ -117,6 +122,73 @@
);
});
+ suite('checks summary', () => {
+ const checkSummary = async (runs: CheckRun[], texts: string[]) => {
+ element.runs = runs;
+ element.showChecksSummary = true;
+ await element.updateComplete;
+ const chips = queryAll<GrChecksChip>(element, 'gr-checks-chip') ?? [];
+ assert.deepEqual(
+ [...chips].map(c => `${c.statusOrCategory} ${c.text}`),
+ texts
+ );
+ };
+
+ test('single success', async () => {
+ checkSummary([createRun()], ['SUCCESS test-name']);
+ });
+
+ test('single running', async () => {
+ checkSummary(
+ [createRun({status: RunStatus.RUNNING})],
+ ['RUNNING test-name']
+ );
+ });
+
+ test('single info', async () => {
+ checkSummary(
+ [
+ createRun({
+ status: RunStatus.COMPLETED,
+ results: [createCheckResult({category: Category.INFO})],
+ }),
+ ],
+ ['INFO test-name']
+ );
+ });
+
+ test('single of each collapses INFO and SUCCESS', async () => {
+ checkSummary(
+ [
+ createRun({status: RunStatus.RUNNING}),
+ createRun({
+ status: RunStatus.COMPLETED,
+ results: [createCheckResult({category: Category.SUCCESS})],
+ }),
+ createRun({
+ status: RunStatus.COMPLETED,
+ results: [createCheckResult({category: Category.INFO})],
+ }),
+ createRun({
+ status: RunStatus.COMPLETED,
+ results: [createCheckResult({category: Category.WARNING})],
+ }),
+ createRun({
+ status: RunStatus.COMPLETED,
+ results: [createCheckResult({category: Category.ERROR})],
+ }),
+ ],
+ [
+ 'ERROR test-name',
+ 'WARNING test-name',
+ 'INFO 1',
+ 'SUCCESS 1',
+ 'RUNNING test-name',
+ ]
+ );
+ });
+ });
+
test('renders mentions summary', async () => {
stubFlags('isEnabled').returns(true);
// recreate element so that flag protected subscriptions are added
diff --git a/polygerrit-ui/app/test/test-data-generators.ts b/polygerrit-ui/app/test/test-data-generators.ts
index b0f0dc0..05e4df7 100644
--- a/polygerrit-ui/app/test/test-data-generators.ts
+++ b/polygerrit-ui/app/test/test-data-generators.ts
@@ -110,7 +110,7 @@
SubmitRequirementResultInfo,
SubmitRequirementStatus,
} from '../api/rest-api';
-import {CheckResult, RunResult} from '../models/checks/checks-model';
+import {CheckResult, CheckRun, RunResult} from '../models/checks/checks-model';
import {Category, RunStatus} from '../api/checks';
import {DiffInfo} from '../api/diff';
import {SearchViewState} from '../models/views/search';
@@ -1103,6 +1103,19 @@
};
}
+export function createRun(partial: Partial<CheckRun> = {}): CheckRun {
+ return {
+ attemptDetails: [],
+ checkName: 'test-name',
+ internalRunId: 'test-internal-run-id',
+ isLatestAttempt: true,
+ isSingleAttempt: true,
+ pluginName: 'test-plugin-name',
+ status: RunStatus.COMPLETED,
+ ...partial,
+ };
+}
+
export function createRunResult(): RunResult {
return {
attemptDetails: [],
@@ -1119,11 +1132,14 @@
};
}
-export function createCheckResult(): CheckResult {
+export function createCheckResult(
+ partial: Partial<CheckResult> = {}
+): CheckResult {
return {
category: Category.ERROR,
summary: 'error',
internalResultId: 'test-internal-result-id',
+ ...partial,
};
}
