Make commets-service responsible for loading comments based on state
Instead of change-view and diff-view calling `reloadAll` the service is
now subscribing to state changes and will update the comments
independently of the lifecycles of the view components.
Change-Id: I3a0694e51ae4b5b0226ff655ecf524036a3e544d
diff --git a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
index 2a4290a..8a28cb1 100644
--- a/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
+++ b/polygerrit-ui/app/elements/change/gr-change-view/gr-change-view.ts
@@ -2021,23 +2021,6 @@
});
}
- /**
- * Fetches a new changeComment object, and data for all types of comments
- * (comments, robot comments, draft comments) is requested.
- */
- _reloadComments() {
- // We are resetting all comment related properties, because we want to avoid
- // a new change being loaded and then paired with outdated comments.
- this._changeComments = undefined;
- this._commentThreads = undefined;
- this._draftCommentThreads = undefined;
- this._robotCommentThreads = undefined;
- if (!this._changeNum)
- throw new Error('missing required changeNum property');
-
- this.commentsService.reloadAll(this._changeNum, this._patchRange?.patchNum);
- }
-
@observe('_changeComments')
changeCommentsChanged(comments?: ChangeComments) {
if (!comments) return;
@@ -2119,8 +2102,6 @@
});
allDataPromises.push(projectConfigLoaded);
- this._reloadComments();
-
let coreDataPromise;
// If the patch number is specified
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js
index 6facdca..8d75230 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-host/gr-diff-host_test.js
@@ -20,7 +20,6 @@
import {dom} from '@polymer/polymer/lib/legacy/polymer.dom.js';
import {createDefaultDiffPrefs, Side} from '../../../constants/constants.js';
-import {_testOnly_resetState} from '../../../services/comments/comments-model.js';
import {createChange, createComment, createCommentThread} from '../../../test/test-data-generators.js';
import {addListenerForTest, mockPromise, stubRestApi} from '../../../test/test-utils.js';
import {EditPatchSetNum, ParentPatchSetNum} from '../../../types/common.js';
@@ -43,7 +42,6 @@
element.path = 'some/path';
sinon.stub(element.reporting, 'time');
sinon.stub(element.reporting, 'timeEnd');
- _testOnly_resetState();
await flush();
});
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
index 2d47ce3..64186f4 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view.ts
@@ -347,8 +347,6 @@
private readonly userService = appContext.userService;
- private readonly commentsService = appContext.commentsService;
-
private readonly shortcuts = appContext.shortcutsService;
_throttledToggleFileReviewed?: (e: KeyboardEvent) => void;
@@ -1073,8 +1071,6 @@
if (!this._change) promises.push(this._getChangeDetail(this._changeNum));
- if (!this._changeComments) this._loadComments(value.patchNum);
-
promises.push(this._getChangeEdit());
this.$.diffHost.cancel();
@@ -1463,11 +1459,6 @@
return url;
}
- _loadComments(patchSet?: PatchSetNum) {
- assertIsDefined(this._changeNum, '_changeNum');
- return this.commentsService.reloadAll(this._changeNum, patchSet);
- }
-
@observe(
'_changeComments',
'_files.changeFilesByPath',
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
index 46ec5d0..5c04ab0 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-view/gr-diff-view_test.js
@@ -30,7 +30,7 @@
import {EditPatchSetNum} from '../../../types/common.js';
import {CursorMoveResult} from '../../../api/core.js';
import {EventType} from '../../../types/events.js';
-import {_testOnly_resetState, _testOnly_setState} from '../../../services/browser/browser-model.js';
+import {_testOnly_setState} from '../../../services/browser/browser-model.js';
const basicFixture = fixtureFromElement('gr-diff-view');
@@ -69,8 +69,6 @@
stubRestApi('getDiffDrafts').returns(Promise.resolve({}));
stubRestApi('getPortedComments').returns(Promise.resolve({}));
- _testOnly_resetState();
-
element = basicFixture.instantiate();
element._changeNum = '42';
element._path = 'some/path.txt';
@@ -2040,7 +2038,6 @@
Promise.resolve([]));
element = basicFixture.instantiate();
element._changeNum = '42';
- return element._loadComments();
});
test('_getFiles add files with comments without changes', () => {
diff --git a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
index 595de34..9e0027a 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment-thread/gr-comment-thread_test.ts
@@ -50,7 +50,6 @@
stubReporting,
stubRestApi,
} from '../../../test/test-utils';
-import {_testOnly_resetState} from '../../../services/comments/comments-model';
import {SinonStub} from 'sinon';
const basicFixture = fixtureFromElement('gr-comment-thread');
@@ -63,7 +62,6 @@
setup(() => {
stubRestApi('getLoggedIn').returns(Promise.resolve(false));
- _testOnly_resetState();
element = basicFixture.instantiate();
element.patchNum = 3 as PatchSetNum;
element.changeNum = 1 as NumericChangeId;
diff --git a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
index 00edc07..09ac95b 100644
--- a/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
+++ b/polygerrit-ui/app/elements/shared/gr-comment/gr-comment_test.ts
@@ -384,11 +384,7 @@
test('delete comment', async () => {
const stub = stubRestApi('deleteComment').returns(
- Promise.resolve({
- id: '1' as UrlEncodedCommentId,
- updated: '1' as Timestamp,
- ...createComment(),
- })
+ Promise.resolve(createComment())
);
const openSpy = sinon.spy(element.confirmDeleteOverlay!, 'open');
element.changeNum = 42 as NumericChangeId;
@@ -1183,6 +1179,7 @@
test('draft prevent save when disabled', async () => {
const saveStub = sinon.stub(element, 'save').returns(Promise.resolve());
+ sinon.stub(element, '_fireEdit');
element.showActions = true;
element.draft = true;
await flush();
diff --git a/polygerrit-ui/app/services/browser/browser-model.ts b/polygerrit-ui/app/services/browser/browser-model.ts
index db790f6..8cb6575 100644
--- a/polygerrit-ui/app/services/browser/browser-model.ts
+++ b/polygerrit-ui/app/services/browser/browser-model.ts
@@ -33,11 +33,13 @@
const initialState: BrowserState = {};
-// Mutable for testing
-let privateState$ = new BehaviorSubject(initialState);
+const privateState$ = new BehaviorSubject(initialState);
export function _testOnly_resetState() {
- privateState$ = new BehaviorSubject(initialState);
+ // We cannot assign a new subject to privateState$, because all the selectors
+ // have already subscribed to the original subject. So we have to emit the
+ // initial state on the existing subject.
+ privateState$.next({...initialState});
}
export function _testOnly_setState(state: BrowserState) {
diff --git a/polygerrit-ui/app/services/change/change-model.ts b/polygerrit-ui/app/services/change/change-model.ts
index 962ef4d..7df0c22 100644
--- a/polygerrit-ui/app/services/change/change-model.ts
+++ b/polygerrit-ui/app/services/change/change-model.ts
@@ -40,6 +40,13 @@
const privateState$ = new BehaviorSubject(initialState);
+export function _testOnly_resetState() {
+ // We cannot assign a new subject to privateState$, because all the selectors
+ // have already subscribed to the original subject. So we have to emit the
+ // initial state on the existing subject.
+ privateState$.next({...initialState});
+}
+
// Re-exporting as Observable so that you can only subscribe, but not emit.
export const changeState$: Observable<ChangeState> = privateState$;
diff --git a/polygerrit-ui/app/services/checks/checks-model.ts b/polygerrit-ui/app/services/checks/checks-model.ts
index 75c24b6d..6435252 100644
--- a/polygerrit-ui/app/services/checks/checks-model.ts
+++ b/polygerrit-ui/app/services/checks/checks-model.ts
@@ -126,11 +126,13 @@
pluginStateSelected: {},
};
-// Mutable for testing
-let privateState$ = new BehaviorSubject(initialState);
+const privateState$ = new BehaviorSubject(initialState);
export function _testOnly_resetState() {
- privateState$ = new BehaviorSubject(initialState);
+ // We cannot assign a new subject to privateState$, because all the selectors
+ // have already subscribed to the original subject. So we have to emit the
+ // initial state on the existing subject.
+ privateState$.next({...initialState});
}
export function _testOnly_setState(state: ChecksState) {
diff --git a/polygerrit-ui/app/services/checks/checks-model_test.ts b/polygerrit-ui/app/services/checks/checks-model_test.ts
index dbd3f86..0be0451 100644
--- a/polygerrit-ui/app/services/checks/checks-model_test.ts
+++ b/polygerrit-ui/app/services/checks/checks-model_test.ts
@@ -18,7 +18,6 @@
import './checks-model';
import {
_testOnly_getState,
- _testOnly_resetState,
ChecksPatchset,
updateStateSetLoading,
updateStateSetProvider,
@@ -52,7 +51,6 @@
suite('checks-model tests', () => {
test('updateStateSetProvider', () => {
- _testOnly_resetState();
updateStateSetProvider(PLUGIN_NAME, ChecksPatchset.LATEST);
assert.deepEqual(current(), {
pluginName: PLUGIN_NAME,
@@ -65,7 +63,6 @@
});
test('loading and first time load', () => {
- _testOnly_resetState();
updateStateSetProvider(PLUGIN_NAME, ChecksPatchset.LATEST);
assert.isFalse(current().loading);
assert.isTrue(current().firstTimeLoad);
@@ -84,14 +81,12 @@
});
test('updateStateSetResults', () => {
- _testOnly_resetState();
updateStateSetResults(PLUGIN_NAME, RUNS, [], [], ChecksPatchset.LATEST);
assert.lengthOf(current().runs, 1);
assert.lengthOf(current().runs[0].results!, 1);
});
test('updateStateUpdateResult', () => {
- _testOnly_resetState();
updateStateSetResults(PLUGIN_NAME, RUNS, [], [], ChecksPatchset.LATEST);
assert.equal(
current().runs[0].results![0].summary,
diff --git a/polygerrit-ui/app/services/comments/comments-model.ts b/polygerrit-ui/app/services/comments/comments-model.ts
index 850acbc..ef57416 100644
--- a/polygerrit-ui/app/services/comments/comments-model.ts
+++ b/polygerrit-ui/app/services/comments/comments-model.ts
@@ -51,7 +51,10 @@
const privateState$ = new BehaviorSubject(initialState);
export function _testOnly_resetState() {
- privateState$.next(initialState);
+ // We cannot assign a new subject to privateState$, because all the selectors
+ // have already subscribed to the original subject. So we have to emit the
+ // initial state on the existing subject.
+ privateState$.next({...initialState});
}
// Re-exporting as Observable so that you can only subscribe, but not emit.
@@ -65,11 +68,21 @@
privateState$.next(state);
}
+export const comments$ = commentState$.pipe(
+ map(commentState => commentState.comments),
+ distinctUntilChanged()
+);
+
export const drafts$ = commentState$.pipe(
map(commentState => commentState.drafts),
distinctUntilChanged()
);
+export const portedComments$ = commentState$.pipe(
+ map(commentState => commentState.portedComments),
+ distinctUntilChanged()
+);
+
export const discardedDrafts$ = commentState$.pipe(
map(commentState => commentState.discardedDrafts),
distinctUntilChanged()
@@ -95,6 +108,11 @@
privateState$.next(state);
}
+/** Called when the change number changes. Wipes out all data from the state. */
+export function updateStateReset() {
+ publishState({...initialState});
+}
+
export function updateStateComments(comments?: {
[path: string]: CommentInfo[];
}) {
diff --git a/polygerrit-ui/app/services/comments/comments-model_test.ts b/polygerrit-ui/app/services/comments/comments-model_test.ts
index e389254..30fc7cf 100644
--- a/polygerrit-ui/app/services/comments/comments-model_test.ts
+++ b/polygerrit-ui/app/services/comments/comments-model_test.ts
@@ -22,13 +22,11 @@
import {
updateStateDeleteDraft,
_testOnly_getState,
- _testOnly_resetState,
_testOnly_setState,
} from './comments-model';
suite('comments model tests', () => {
test('updateStateDeleteDraft', () => {
- _testOnly_resetState();
const draft = createDraft();
draft.id = '1' as UrlEncodedCommentId;
_testOnly_setState({
diff --git a/polygerrit-ui/app/services/comments/comments-service.ts b/polygerrit-ui/app/services/comments/comments-service.ts
index 2e28ece..5896b52 100644
--- a/polygerrit-ui/app/services/comments/comments-service.ts
+++ b/polygerrit-ui/app/services/comments/comments-service.ts
@@ -31,7 +31,10 @@
updateStatePortedDrafts,
updateStateUndoDiscardedDraft,
discardedDrafts$,
+ updateStateReset,
} from './comments-model';
+import {changeNum$, currentPatchNum$} from '../change/change-model';
+import {combineLatest} from 'rxjs';
export class CommentsService {
private discardedDrafts?: UIDraft[] = [];
@@ -40,21 +43,20 @@
discardedDrafts$.subscribe(
discardedDrafts => (this.discardedDrafts = discardedDrafts)
);
- }
-
- /**
- * Load all comments (with drafts and robot comments) for the given change
- * number. The returned promise resolves when the comments have loaded, but
- * does not yield the comment data.
- */
- // TODO(dhruvsri): listen to changeNum changes or reload event to update
- // automatically
- reloadAll(changeNum: NumericChangeId, patchNum = CURRENT as RevisionId) {
- this.reloadComments(changeNum);
- this.reloadRobotComments(changeNum);
- this.reloadDrafts(changeNum);
- this.reloadPortedComments(changeNum, patchNum);
- this.reloadPortedDrafts(changeNum, patchNum);
+ changeNum$.subscribe(changeNum => {
+ updateStateReset();
+ if (!changeNum) return;
+ this.reloadComments(changeNum);
+ this.reloadRobotComments(changeNum);
+ this.reloadDrafts(changeNum);
+ });
+ combineLatest([changeNum$, currentPatchNum$]).subscribe(
+ ([changeNum, currentPatchNum]) => {
+ if (!changeNum || !currentPatchNum) return;
+ this.reloadPortedComments(changeNum, currentPatchNum);
+ this.reloadPortedDrafts(changeNum, currentPatchNum);
+ }
+ );
}
reloadComments(changeNum: NumericChangeId): Promise<void> {
diff --git a/polygerrit-ui/app/services/comments/comments-service_test.ts b/polygerrit-ui/app/services/comments/comments-service_test.ts
index 0e6e042..a35768e 100644
--- a/polygerrit-ui/app/services/comments/comments-service_test.ts
+++ b/polygerrit-ui/app/services/comments/comments-service_test.ts
@@ -18,109 +18,63 @@
import '../../test/common-test-setup-karma';
import {
createComment,
- createFixSuggestionInfo,
+ createParsedChange,
+ TEST_NUMERIC_CHANGE_ID,
} from '../../test/test-data-generators';
-import {stubRestApi} from '../../test/test-utils';
-import {
- NumericChangeId,
- RobotId,
- RobotRunId,
- Timestamp,
- UrlEncodedCommentId,
-} from '../../types/common';
+import {stubRestApi, waitUntil, waitUntilCalled} from '../../test/test-utils';
import {appContext} from '../app-context';
import {CommentsService} from './comments-service';
+import {updateState as updateChangeState} from '../change/change-model';
+import {
+ GerritView,
+ updateState as updateRouterState,
+} from '../router/router-model';
+import {comments$, portedComments$} from './comments-model';
+import {PathToCommentsInfoMap} from '../../types/common';
suite('change service tests', () => {
- let commentsService: CommentsService;
-
- test('loads logged-out', () => {
- const changeNum = 1234 as NumericChangeId;
- commentsService = new CommentsService(appContext.restApiService);
- stubRestApi('getLoggedIn').returns(Promise.resolve(false));
+ test('loads comments', async () => {
+ new CommentsService(appContext.restApiService);
const diffCommentsSpy = stubRestApi('getDiffComments').returns(
- Promise.resolve({
- 'foo.c': [
- {
- ...createComment(),
- id: '123' as UrlEncodedCommentId,
- message: 'Done',
- updated: '2017-02-08 16:40:49' as Timestamp,
- },
- ],
- })
+ Promise.resolve({'foo.c': [createComment()]})
);
const diffRobotCommentsSpy = stubRestApi('getDiffRobotComments').returns(
- Promise.resolve({
- 'foo.c': [
- {
- ...createComment(),
- id: '321' as UrlEncodedCommentId,
- message: 'Done',
- updated: '2017-02-08 16:40:49' as Timestamp,
- robot_id: 'robot_1' as RobotId,
- robot_run_id: 'run_1' as RobotRunId,
- properties: {},
- fix_suggestions: [
- createFixSuggestionInfo('fix_1'),
- createFixSuggestionInfo('fix_2'),
- ],
- },
- ],
- })
+ Promise.resolve({})
);
const diffDraftsSpy = stubRestApi('getDiffDrafts').returns(
Promise.resolve({})
);
-
- commentsService.reloadAll(changeNum);
- assert.isTrue(diffCommentsSpy.calledWithExactly(changeNum));
- assert.isTrue(diffRobotCommentsSpy.calledWithExactly(changeNum));
- assert.isTrue(diffDraftsSpy.calledWithExactly(changeNum));
- });
-
- test('loads logged-in', () => {
- const changeNum = 1234 as NumericChangeId;
-
- stubRestApi('getLoggedIn').returns(Promise.resolve(true));
- const diffCommentsSpy = stubRestApi('getDiffComments').returns(
- Promise.resolve({
- 'foo.c': [
- {
- ...createComment(),
- id: '123' as UrlEncodedCommentId,
- message: 'Done',
- updated: '2017-02-08 16:40:49' as Timestamp,
- },
- ],
- })
+ const portedCommentsSpy = stubRestApi('getPortedComments').returns(
+ Promise.resolve({'foo.c': [createComment()]})
);
- const diffRobotCommentsSpy = stubRestApi('getDiffRobotComments').returns(
- Promise.resolve({
- 'foo.c': [
- {
- ...createComment(),
- id: '321' as UrlEncodedCommentId,
- message: 'Done',
- updated: '2017-02-08 16:40:49' as Timestamp,
- robot_id: 'robot_1' as RobotId,
- robot_run_id: 'run_1' as RobotRunId,
- properties: {},
- fix_suggestions: [
- createFixSuggestionInfo('fix_1'),
- createFixSuggestionInfo('fix_2'),
- ],
- },
- ],
- })
- );
- const diffDraftsSpy = stubRestApi('getDiffDrafts').returns(
+ const portedDraftsSpy = stubRestApi('getPortedDrafts').returns(
Promise.resolve({})
);
+ let comments: PathToCommentsInfoMap = {};
+ comments$.subscribe(c => (comments = c));
+ let portedComments: PathToCommentsInfoMap = {};
+ portedComments$.subscribe(c => (portedComments = c));
- commentsService.reloadAll(changeNum);
- assert.isTrue(diffCommentsSpy.calledWithExactly(changeNum));
- assert.isTrue(diffRobotCommentsSpy.calledWithExactly(changeNum));
- assert.isTrue(diffDraftsSpy.calledWithExactly(changeNum));
+ updateRouterState(GerritView.CHANGE, TEST_NUMERIC_CHANGE_ID);
+ updateChangeState(createParsedChange());
+
+ await waitUntilCalled(diffCommentsSpy, 'diffCommentsSpy');
+ await waitUntilCalled(diffRobotCommentsSpy, 'diffRobotCommentsSpy');
+ await waitUntilCalled(diffDraftsSpy, 'diffDraftsSpy');
+ await waitUntilCalled(portedCommentsSpy, 'portedCommentsSpy');
+ await waitUntilCalled(portedDraftsSpy, 'portedDraftsSpy');
+ await waitUntil(
+ () => Object.keys(comments).length > 0,
+ 'comment in model not set'
+ );
+ await waitUntil(
+ () => Object.keys(portedComments).length > 0,
+ 'ported comment in model not set'
+ );
+
+ assert.equal(comments['foo.c'].length, 1);
+ assert.equal(comments['foo.c'][0].id, '12345');
+ assert.equal(portedComments['foo.c'].length, 1);
+ assert.equal(portedComments['foo.c'][0].id, '12345');
});
});
diff --git a/polygerrit-ui/app/services/router/router-model.ts b/polygerrit-ui/app/services/router/router-model.ts
index b3cdf9e..ae3d848 100644
--- a/polygerrit-ui/app/services/router/router-model.ts
+++ b/polygerrit-ui/app/services/router/router-model.ts
@@ -47,6 +47,13 @@
const privateState$ = new BehaviorSubject<RouterState>(initialState);
+export function _testOnly_resetState() {
+ // We cannot assign a new subject to privateState$, because all the selectors
+ // have already subscribed to the original subject. So we have to emit the
+ // initial state on the existing subject.
+ privateState$.next({...initialState});
+}
+
// Re-exporting as Observable so that you can only subscribe, but not emit.
export const routerState$: Observable<RouterState> = privateState$;
diff --git a/polygerrit-ui/app/services/user/user-model.ts b/polygerrit-ui/app/services/user/user-model.ts
index 000887c..df307d6 100644
--- a/polygerrit-ui/app/services/user/user-model.ts
+++ b/polygerrit-ui/app/services/user/user-model.ts
@@ -37,11 +37,13 @@
diffPreferences: createDefaultDiffPrefs(),
};
-// Mutable for testing
-let privateState$ = new BehaviorSubject(initialState);
+const privateState$ = new BehaviorSubject(initialState);
export function _testOnly_resetState() {
- privateState$ = new BehaviorSubject(initialState);
+ // We cannot assign a new subject to privateState$, because all the selectors
+ // have already subscribed to the original subject. So we have to emit the
+ // initial state on the existing subject.
+ privateState$.next({...initialState});
}
export function _testOnly_setState(state: UserState) {
diff --git a/polygerrit-ui/app/test/common-test-setup.ts b/polygerrit-ui/app/test/common-test-setup.ts
index bd5504a..05adb41 100644
--- a/polygerrit-ui/app/test/common-test-setup.ts
+++ b/polygerrit-ui/app/test/common-test-setup.ts
@@ -45,6 +45,12 @@
import {updatePreferences} from '../services/user/user-model';
import {createDefaultPreferences} from '../constants/constants';
import {appContext} from '../services/app-context';
+import {_testOnly_resetState as resetBrowserState} from '../services/browser/browser-model';
+import {_testOnly_resetState as resetChangeState} from '../services/change/change-model';
+import {_testOnly_resetState as resetChecksState} from '../services/checks/checks-model';
+import {_testOnly_resetState as resetCommentsState} from '../services/comments/comments-model';
+import {_testOnly_resetState as resetRouterState} from '../services/router/router-model';
+import {_testOnly_resetState as resetUserState} from '../services/user/user-model';
declare global {
interface Window {
@@ -106,6 +112,14 @@
// tests.
initGlobalVariables();
_testOnly_initGerritPluginApi();
+
+ resetBrowserState();
+ resetChangeState();
+ resetChecksState();
+ resetCommentsState();
+ resetRouterState();
+ resetUserState();
+
const shortcuts = appContext.shortcutsService;
assert.isTrue(shortcuts._testOnly_isEmpty());
const selection = document.getSelection();
diff --git a/polygerrit-ui/app/test/test-data-generators.ts b/polygerrit-ui/app/test/test-data-generators.ts
index dd56ce2..91cd2f3 100644
--- a/polygerrit-ui/app/test/test-data-generators.ts
+++ b/polygerrit-ui/app/test/test-data-generators.ts
@@ -95,7 +95,12 @@
import {AppElementChangeViewParams} from '../elements/gr-app-types';
import {CommitInfoWithRequiredCommit} from '../elements/change/gr-change-metadata/gr-change-metadata';
import {WebLinkInfo} from '../types/diff';
-import {createCommentThreads, UIComment, UIDraft} from '../utils/comment-util';
+import {
+ createCommentThreads,
+ UIComment,
+ UIDraft,
+ UIHuman,
+} from '../utils/comment-util';
import {GerritView} from '../services/router/router-model';
import {ChangeComments} from '../elements/diff/gr-comment-api/gr-comment-api';
import {EditRevisionInfo, ParsedChangeInfo} from '../types/types';
@@ -488,7 +493,7 @@
};
}
-export function createComment(): UIComment {
+export function createComment(): UIHuman {
return {
patch_set: 1 as PatchSetNum,
id: '12345' as UrlEncodedCommentId,
diff --git a/polygerrit-ui/app/test/test-utils.ts b/polygerrit-ui/app/test/test-utils.ts
index 4a513f8..63c125e 100644
--- a/polygerrit-ui/app/test/test-utils.ts
+++ b/polygerrit-ui/app/test/test-utils.ts
@@ -19,7 +19,7 @@
import {_testOnly_resetEndpoints} from '../elements/shared/gr-js-api-interface/gr-plugin-endpoints';
import {appContext} from '../services/app-context';
import {RestApiService} from '../services/gr-rest-api/gr-rest-api';
-import {SinonSpy} from 'sinon';
+import {SinonSpy, SinonStub} from 'sinon';
import {StorageService} from '../services/storage/gr-storage';
import {AuthService} from '../services/gr-auth/gr-auth';
import {ReportingService} from '../services/gr-reporting/gr-reporting';
@@ -185,6 +185,7 @@
): Promise<void> {
const start = Date.now();
let sleep = 0;
+ if (predicate()) return Promise.resolve();
return new Promise((resolve, reject) => {
const waiter = () => {
if (predicate()) {
@@ -200,6 +201,10 @@
});
}
+export function waitUntilCalled(stub: SinonStub, name: string) {
+ return waitUntil(() => stub.called, `${name} was not called`);
+}
+
/**
* Promisify an event callback to simplify async...await tests.
*
