Merge "RevisionIT: Fix flaky test"
diff --git a/Documentation/metrics.txt b/Documentation/metrics.txt
index 0bbf4fb..36b3473 100644
--- a/Documentation/metrics.txt
+++ b/Documentation/metrics.txt
@@ -118,6 +118,15 @@
** `type`:
type of push (create/replace, autoclose, normal)
* `receivecommits/timeout`: rate of push timeouts
+* `receivecommits/ps_revision_missing`: errors due to patch set revision missing
+* `receivecommits/push_count`: number of pushes
+** `kind`:
+ The push kind (direct vs. magic).
+** `project`:
+ The name of the project for which the push is done.
+** `type`:
+ The type of the update (CREATE, UPDATE, CREATE/UPDATE, UPDATE_NONFASTFORWARD,
+ DELETE).
=== Process
diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index f7f58fc..ec1a5c8 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -102,7 +102,9 @@
import com.google.gerrit.extensions.validators.CommentValidationFailure;
import com.google.gerrit.extensions.validators.CommentValidator;
import com.google.gerrit.metrics.Counter0;
+import com.google.gerrit.metrics.Counter3;
import com.google.gerrit.metrics.Description;
+import com.google.gerrit.metrics.Field;
import com.google.gerrit.metrics.MetricMaker;
import com.google.gerrit.server.CancellationMetrics;
import com.google.gerrit.server.ChangeUtil;
@@ -323,6 +325,7 @@
@Singleton
private static class Metrics {
private final Counter0 psRevisionMissing;
+ private final Counter3<String, String, String> pushCount;
@Inject
Metrics(MetricMaker metricMaker) {
@@ -330,6 +333,23 @@
metricMaker.newCounter(
"receivecommits/ps_revision_missing",
new Description("errors due to patch set revision missing"));
+ pushCount =
+ metricMaker.newCounter(
+ "receivecommits/push_count",
+ new Description("number of pushes"),
+ Field.ofString("kind", (metadataBuilder, fieldValue) -> {})
+ .description("The push kind (direct vs. magic).")
+ .build(),
+ Field.ofString(
+ "project",
+ (metadataBuilder, fieldValue) -> metadataBuilder.projectName(fieldValue))
+ .description("The name of the project for which the push is done.")
+ .build(),
+ Field.ofString("type", (metadataBuilder, fieldValue) -> {})
+ .description(
+ "The type of the update (CREATE, UPDATE, CREATE/UPDATE,"
+ + " UPDATE_NONFASTFORWARD, DELETE).")
+ .build());
}
}
@@ -727,6 +747,13 @@
return;
}
+ if (!magicCommands.isEmpty()) {
+ metrics.pushCount.increment("magic", project.getName(), getUpdateType(magicCommands));
+ }
+ if (!regularCommands.isEmpty()) {
+ metrics.pushCount.increment("direct", project.getName(), getUpdateType(regularCommands));
+ }
+
try {
if (!regularCommands.isEmpty()) {
handleRegularCommands(regularCommands, progress);
@@ -777,6 +804,15 @@
lazy(() -> commands.stream().map(ReceiveCommits::commandToString).collect(joining(","))));
}
+ private String getUpdateType(List<ReceiveCommand> commands) {
+ return commands.stream()
+ .map(ReceiveCommand::getType)
+ .map(ReceiveCommand.Type::name)
+ .distinct()
+ .sorted()
+ .collect(joining("/"));
+ }
+
private void sendErrorMessages() {
if (!errors.isEmpty()) {
logger.atFine().log("Handling error conditions: %s", errors.keySet());
diff --git a/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.ts b/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.ts
index 4147cc0..a496be5 100644
--- a/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.ts
+++ b/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores.ts
@@ -16,9 +16,8 @@
*/
import '../gr-label-score-row/gr-label-score-row';
import '../../../styles/shared-styles';
-import {PolymerElement} from '@polymer/polymer/polymer-element';
-import {htmlTemplate} from './gr-label-scores_html';
-import {customElement, property} from '@polymer/decorators';
+import {LitElement, css, html} from 'lit';
+import {customElement, property} from 'lit/decorators';
import {hasOwnProperty} from '../../../utils/common-util';
import {
LabelNameToValueMap,
@@ -33,21 +32,14 @@
Label,
LabelValuesMap,
} from '../gr-label-score-row/gr-label-score-row';
-import {PolymerDeepPropertyChange} from '@polymer/polymer/interfaces';
import {appContext} from '../../../services/app-context';
import {labelCompare} from '../../../utils/label-util';
import {Execution} from '../../../constants/reporting';
+import {ChangeStatus} from '../../../constants/constants';
@customElement('gr-label-scores')
-export class GrLabelScores extends PolymerElement {
- static get template() {
- return htmlTemplate;
- }
-
- @property({type: Array, computed: '_computeLabels(change.labels.*, account)'})
- _labels: Label[] = [];
-
- @property({type: Object, observer: '_computeColumns'})
+export class GrLabelScores extends LitElement {
+ @property({type: Object})
permittedLabels?: LabelNameToValueMap;
@property({type: Object})
@@ -56,11 +48,63 @@
@property({type: Object})
account?: AccountInfo;
- @property({type: Object})
- _labelValues?: LabelValuesMap;
-
private readonly reporting = appContext.reportingService;
+ static override get styles() {
+ return [
+ css`
+ .scoresTable {
+ display: table;
+ width: 100%;
+ }
+ .mergedMessage,
+ .abandonedMessage {
+ font-style: italic;
+ text-align: center;
+ width: 100%;
+ }
+ gr-label-score-row:hover {
+ background-color: var(--hover-background-color);
+ }
+ gr-label-score-row {
+ display: table-row;
+ }
+ gr-label-score-row.no-access {
+ display: none;
+ }
+ `,
+ ];
+ }
+
+ override render() {
+ const labels = this._computeLabels();
+ const labelValues = this._computeColumns();
+ return html`<div class="scoresTable">
+ ${labels.map(
+ label => html`<gr-label-score-row
+ class="${this.computeLabelAccessClass(label.name)}"
+ .label="${label}"
+ .name="${label.name}"
+ .labels="${this.change?.labels}"
+ .permittedLabels="${this.permittedLabels}"
+ .labelValues="${labelValues}"
+ ></gr-label-score-row>`
+ )}
+ </div>
+ <div
+ class="mergedMessage"
+ ?hidden=${this.change?.status !== ChangeStatus.MERGED}
+ >
+ Because this change has been merged, votes may not be decreased.
+ </div>
+ <div
+ class="abandonedMessage"
+ ?hidden=${this.change?.status !== ChangeStatus.ABANDONED}
+ >
+ Because this change has been abandoned, you cannot vote.
+ </div>`;
+ }
+
getLabelValues(includeDefaults = true): LabelNameToValuesMap {
const labels: LabelNameToValuesMap = {};
if (this.shadowRoot === null || !this.change) {
@@ -79,7 +123,7 @@
if (selectedVal === undefined) continue;
- const defValNum = this._getDefaultValue(this.change.labels, label);
+ const defValNum = this.getDefaultValue(label);
if (includeDefaults || selectedVal !== defValNum) {
labels[label] = selectedVal;
}
@@ -87,7 +131,7 @@
return labels;
}
- _getStringLabelValue(
+ private getStringLabelValue(
labels: LabelNameToInfoMap,
labelName: string,
numberValue?: number
@@ -108,25 +152,26 @@
return stringVal;
}
- _getDefaultValue(labels?: LabelNameToInfoMap, labelName?: string) {
+ private getDefaultValue(labelName?: string) {
+ const labels = this.change?.labels;
if (!labelName || !labels?.[labelName]) return undefined;
const labelInfo = labels[labelName] as DetailedLabelInfo;
return labelInfo.default_value;
}
- _getVoteForAccount(
- labels: LabelNameToInfoMap | undefined,
- labelName: string,
- account?: AccountInfo
- ): string | null {
+ _getVoteForAccount(labelName: string): string | null {
+ const labels = this.change?.labels;
if (!labels) return null;
const votes = labels[labelName] as DetailedLabelInfo;
if (votes.all && votes.all.length > 0) {
for (let i = 0; i < votes.all.length; i++) {
- // TODO(TS): Replace == with === and check code can assign string to _account_id instead of number
- // eslint-disable-next-line eqeqeq
- if (account && votes.all[i]._account_id == account._account_id) {
- return this._getStringLabelValue(
+ if (
+ this.account &&
+ // TODO(TS): Replace == with === and check code can assign string to _account_id instead of number
+ // eslint-disable-next-line eqeqeq
+ votes.all[i]._account_id == this.account._account_id
+ ) {
+ return this.getStringLabelValue(
labels,
labelName,
votes.all[i].value
@@ -137,32 +182,26 @@
return null;
}
- _computeLabels(
- labelRecord: PolymerDeepPropertyChange<
- LabelNameToInfoMap,
- LabelNameToInfoMap
- >,
- account?: AccountInfo
- ): Label[] {
- if (!account) return [];
- if (!labelRecord?.base) return [];
- const labelsObj = labelRecord.base;
+ _computeLabels(): Label[] {
+ if (!this.account) return [];
+ const labelsObj = this.change?.labels;
+ if (!labelsObj) return [];
return Object.keys(labelsObj)
.sort(labelCompare)
.map(key => {
return {
name: key,
- value: this._getVoteForAccount(labelsObj, key, this.account),
+ value: this._getVoteForAccount(key),
};
});
}
- _computeColumns(permittedLabels?: LabelNameToValueMap) {
- if (!permittedLabels) return;
- const labels = Object.keys(permittedLabels);
+ _computeColumns() {
+ if (!this.permittedLabels) return;
+ const labels = Object.keys(this.permittedLabels);
const values: Set<number> = new Set();
for (const label of labels) {
- for (const value of permittedLabels[label]) {
+ for (const value of this.permittedLabels[label]) {
values.add(Number(value));
}
}
@@ -173,23 +212,14 @@
for (let i = 0; i < orderedValues.length; i++) {
labelValues[orderedValues[i]] = i;
}
- this._labelValues = labelValues;
+ return labelValues;
}
- _changeIsMerged(changeStatus: string) {
- return changeStatus === 'MERGED';
- }
+ private computeLabelAccessClass(label?: string) {
+ if (!this.permittedLabels || !label) return '';
- _computeLabelAccessClass(
- label?: string,
- permittedLabels?: LabelNameToValueMap
- ) {
- if (!permittedLabels || !label) {
- return '';
- }
-
- return hasOwnProperty(permittedLabels, label) &&
- permittedLabels[label].length
+ return hasOwnProperty(this.permittedLabels, label) &&
+ this.permittedLabels[label].length
? 'access'
: 'no-access';
}
diff --git a/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores_html.ts b/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores_html.ts
deleted file mode 100644
index 7b1fb7f..0000000
--- a/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores_html.ts
+++ /dev/null
@@ -1,55 +0,0 @@
-/**
- * @license
- * Copyright (C) 2020 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- * http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-import {html} from '@polymer/polymer/lib/utils/html-tag';
-
-export const htmlTemplate = html`
- <style include="shared-styles">
- .scoresTable {
- display: table;
- width: 100%;
- }
- .mergedMessage {
- font-style: italic;
- text-align: center;
- width: 100%;
- }
- gr-label-score-row:hover {
- background-color: var(--hover-background-color);
- }
- gr-label-score-row {
- display: table-row;
- }
- gr-label-score-row.no-access {
- display: none;
- }
- </style>
- <div class="scoresTable">
- <template is="dom-repeat" items="[[_labels]]" as="label">
- <gr-label-score-row
- class$="[[_computeLabelAccessClass(label.name, permittedLabels)]]"
- label="[[label]]"
- name="[[label.name]]"
- labels="[[change.labels]]"
- permitted-labels="[[permittedLabels]]"
- label-values="[[_labelValues]]"
- ></gr-label-score-row>
- </template>
- </div>
- <div class="mergedMessage" hidden$="[[!_changeIsMerged(change.status)]]">
- Because this change has been merged, votes may not be decreased.
- </div>
-`;
diff --git a/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores_test.ts b/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores_test.ts
index 58fe189..cfecf91 100644
--- a/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores_test.ts
+++ b/polygerrit-ui/app/elements/change/gr-label-scores/gr-label-scores_test.ts
@@ -17,7 +17,7 @@
import '../../../test/common-test-setup-karma';
import './gr-label-scores';
-import {queryAndAssert, stubRestApi} from '../../../test/test-utils';
+import {isHidden, queryAndAssert, stubRestApi} from '../../../test/test-utils';
import {GrLabelScores} from './gr-label-scores';
import {AccountId} from '../../../types/common';
import {GrLabelScoreRow} from '../gr-label-score-row/gr-label-score-row';
@@ -25,6 +25,7 @@
createAccountWithId,
createChange,
} from '../../../test/test-data-generators';
+import {ChangeStatus} from '../../../constants/constants';
const basicFixture = fixtureFromElement('gr-label-scores');
@@ -116,19 +117,12 @@
test('_getVoteForAccount', () => {
const labelName = 'Code-Review';
- assert.strictEqual(
- element._getVoteForAccount(
- element.change!.labels,
- labelName,
- element.account
- ),
- '+1'
- );
+ assert.strictEqual(element._getVoteForAccount(labelName), '+1');
});
test('_computeColumns', () => {
- element._computeColumns(element.permittedLabels);
- assert.deepEqual(element._labelValues, {
+ const labelValues = element._computeColumns();
+ assert.deepEqual(labelValues, {
'-2': 0,
'-1': 1,
'0': 2,
@@ -137,31 +131,8 @@
});
});
- test('_computeLabelAccessClass undefined case', () => {
- assert.strictEqual(
- element._computeLabelAccessClass(undefined, undefined),
- ''
- );
- assert.strictEqual(element._computeLabelAccessClass('', undefined), '');
- assert.strictEqual(element._computeLabelAccessClass(undefined, {}), '');
- });
-
- test('_computeLabelAccessClass has access', () => {
- assert.strictEqual(
- element._computeLabelAccessClass('foo', {foo: ['']}),
- 'access'
- );
- });
-
- test('_computeLabelAccessClass no access', () => {
- assert.strictEqual(
- element._computeLabelAccessClass('zap', {foo: ['']}),
- 'no-access'
- );
- });
-
- test('changes in label score are reflected in _labels', () => {
- element.change = {
+ test('changes in label score are reflected in _labels', async () => {
+ const change = {
...createChange(),
labels: {
'Code-Review': {
@@ -186,17 +157,62 @@
},
},
};
- assert.deepEqual(element._labels, [
+ element.change = change;
+ await flush();
+ let labels = element._computeLabels();
+ assert.deepEqual(labels, [
{name: 'Code-Review', value: null},
{name: 'Verified', value: null},
]);
- element.set(
- ['change', 'labels', 'Verified', 'all'],
- [{_account_id: accountId, value: 1}]
- );
- assert.deepEqual(element._labels, [
+ element.change = {
+ ...change,
+ labels: {
+ ...change.labels,
+ Verified: {
+ ...change.labels.Verified,
+ all: [
+ {
+ _account_id: accountId,
+ value: 1,
+ },
+ ],
+ },
+ },
+ };
+ await flush();
+ labels = element._computeLabels();
+ assert.deepEqual(labels, [
{name: 'Code-Review', value: null},
{name: 'Verified', value: '+1'},
]);
});
+ suite('message', () => {
+ test('shown when change is abandoned', async () => {
+ element.change = {
+ ...createChange(),
+ status: ChangeStatus.ABANDONED,
+ };
+ await flush();
+ assert.isFalse(isHidden(queryAndAssert(element, '.abandonedMessage')));
+ assert.isTrue(isHidden(queryAndAssert(element, '.mergedMessage')));
+ });
+ test('shown when change is merged', async () => {
+ element.change = {
+ ...createChange(),
+ status: ChangeStatus.MERGED,
+ };
+ await flush();
+ assert.isFalse(isHidden(queryAndAssert(element, '.mergedMessage')));
+ assert.isTrue(isHidden(queryAndAssert(element, '.abandonedMessage')));
+ });
+ test('do not show for new', async () => {
+ element.change = {
+ ...createChange(),
+ status: ChangeStatus.NEW,
+ };
+ await flush();
+ assert.isTrue(isHidden(queryAndAssert(element, '.mergedMessage')));
+ assert.isTrue(isHidden(queryAndAssert(element, '.abandonedMessage')));
+ });
+ });
});
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/token-highlight-layer_test.ts b/polygerrit-ui/app/elements/diff/gr-diff-builder/token-highlight-layer_test.ts
index 2cb08d6..4f44665 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/token-highlight-layer_test.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/token-highlight-layer_test.ts
@@ -21,7 +21,7 @@
import {GrDiffLine, GrDiffLineType} from '../gr-diff/gr-diff-line.js';
import {HOVER_DELAY_MS, TokenHighlightLayer} from './token-highlight-layer';
import {GrAnnotation} from '../gr-diff-highlight/gr-annotation';
-import {html, render} from 'lit-html';
+import {html, render} from 'lit';
import {_testOnly_allTasks} from '../../../utils/async-util';
import {queryAndAssert} from '../../../test/test-utils';
diff --git a/polygerrit-ui/app/styles/gr-menu-page-styles.ts b/polygerrit-ui/app/styles/gr-menu-page-styles.ts
index 0471a4e..5f58571 100644
--- a/polygerrit-ui/app/styles/gr-menu-page-styles.ts
+++ b/polygerrit-ui/app/styles/gr-menu-page-styles.ts
@@ -14,7 +14,7 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
-import {css} from 'lit-element';
+import {css} from 'lit';
export const menuPageStyles = css`
:host {
