Rewrite router tests for GROUP_LIST in a more robust way
A major issue with the existing tests is that they don't test the URL
parsing and matching at all. They just test the handler methods. That
creates a huge gap in testing and prevents safe refactorings.
We want to refactor all route matching and handling in a similar way as
change 356680 shows it for one route. So it makes sense to write the
tests in a way that the input is a URL and the output is a view state.
I will follow up with other tests once this change is approved and
submitted.
Release-Notes: skip
Change-Id: I7ed36dd147fac2d500331da7aa0384c6aeee8c7b
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
index 168f334..00cb938 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router.ts
@@ -1158,7 +1158,7 @@
const state: AdminViewState = {
view: GerritView.ADMIN,
adminView: AdminChildView.GROUPS,
- offset: ctx.params[1] || 0,
+ offset: ctx.params[1] ?? '0',
filter: null,
openCreateModal: ctx.hash === 'create',
};
@@ -1173,6 +1173,7 @@
adminView: AdminChildView.GROUPS,
offset: ctx.params['offset'],
filter: ctx.params['filter'],
+ openCreateModal: false,
};
// Note that router model view must be updated before view models.
this.setState(state);
@@ -1183,7 +1184,9 @@
const state: AdminViewState = {
view: GerritView.ADMIN,
adminView: AdminChildView.GROUPS,
+ offset: ctx.params[1] ?? '0',
filter: ctx.params['filter'] || null,
+ openCreateModal: false,
};
// Note that router model view must be updated before view models.
this.setState(state);
diff --git a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts
index 98272b4..af44739 100644
--- a/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts
+++ b/polygerrit-ui/app/elements/core/gr-router/gr-router_test.ts
@@ -11,6 +11,7 @@
stubRestApi,
addListenerForTest,
waitEventLoop,
+ waitUntilCalled,
} from '../../../test/test-utils';
import {GrRouter, routerToken, _testOnly_RoutePattern} from './gr-router';
import {GerritView} from '../../../services/router/router-model';
@@ -25,7 +26,7 @@
} from '../../../types/common';
import {AppElementParams} from '../../gr-app-types';
import {assert} from '@open-wc/testing';
-import {AdminChildView} from '../../../models/views/admin';
+import {AdminChildView, AdminViewState} from '../../../models/views/admin';
import {RepoDetailView} from '../../../models/views/repo';
import {GroupDetailView} from '../../../models/views/group';
import {ChangeChildView, ChangeViewState} from '../../../models/views/change';
@@ -38,6 +39,7 @@
createRevision,
} from '../../../test/test-data-generators';
import {ParsedChangeInfo} from '../../../types/types';
+import {ViewState} from '../../../models/views/base';
suite('gr-router tests', () => {
let router: GrRouter;
@@ -291,6 +293,19 @@
assert.deepEqual(setStateStub.lastCall.args[0], params);
}
+ async function checkUrlToState<T extends ViewState>(url: string, state: T) {
+ setStateStub.reset();
+ router.page.show(url);
+ await waitUntilCalled(setStateStub, 'setState');
+ assert.deepEqual(setStateStub.lastCall.firstArg, state);
+ }
+
+ async function checkUrlNotMatched(url: string) {
+ handlePassThroughRoute.reset();
+ router.page.show(url);
+ await waitUntilCalled(handlePassThroughRoute, 'handlePassThroughRoute');
+ }
+
function createPageContext(): PageContext {
return {
canonicalPath: '',
@@ -306,6 +321,7 @@
redirectStub = sinon.stub(router, 'redirect');
setStateStub = sinon.stub(router, 'setState');
handlePassThroughRoute = sinon.stub(router, 'handlePassThroughRoute');
+ router.startRouter();
});
test('handleLegacyProjectDashboardRoute', () => {
@@ -737,55 +753,55 @@
});
});
- test('handleGroupListOffsetRoute', () => {
- const ctx = createPageContext();
- assertctxToParams(ctx, 'handleGroupListOffsetRoute', {
+ test('list of groups', async () => {
+ const defaultState: AdminViewState = {
view: GerritView.ADMIN,
adminView: AdminChildView.GROUPS,
- offset: 0,
- filter: null,
+ offset: '0',
openCreateModal: false,
- });
-
- ctx.params[1] = '42';
- assertctxToParams(ctx, 'handleGroupListOffsetRoute', {
- view: GerritView.ADMIN,
- adminView: AdminChildView.GROUPS,
- offset: '42',
filter: null,
- openCreateModal: false,
- });
+ };
- ctx.hash = 'create';
- assertctxToParams(ctx, 'handleGroupListOffsetRoute', {
- view: GerritView.ADMIN,
- adminView: AdminChildView.GROUPS,
- offset: '42',
- filter: null,
+ await checkUrlToState('/admin/groups', defaultState);
+ await checkUrlToState('/admin/groups/', defaultState);
+ await checkUrlToState('/admin/groups#create', {
+ ...defaultState,
openCreateModal: true,
});
- });
-
- test('handleGroupListFilterOffsetRoute', () => {
- const ctx = {
- ...createPageContext(),
- params: {filter: 'foo', offset: '42'},
- };
- assertctxToParams(ctx, 'handleGroupListFilterOffsetRoute', {
- view: GerritView.ADMIN,
- adminView: AdminChildView.GROUPS,
- offset: '42',
- filter: 'foo',
+ await checkUrlToState('/admin/groups,123', {
+ ...defaultState,
+ offset: '123',
});
- });
-
- test('handleGroupListFilterRoute', () => {
- const ctx = {...createPageContext(), params: {filter: 'foo'}};
- assertctxToParams(ctx, 'handleGroupListFilterRoute', {
- view: GerritView.ADMIN,
- adminView: AdminChildView.GROUPS,
- filter: 'foo',
+ await checkUrlToState('/admin/groups,123#create', {
+ ...defaultState,
+ offset: '123',
+ openCreateModal: true,
});
+
+ await checkUrlToState('/admin/groups/q/filter:asdf', {
+ ...defaultState,
+ filter: 'asdf',
+ });
+ await checkUrlToState('/admin/groups/q/filter:asdf,123', {
+ ...defaultState,
+ filter: 'asdf',
+ offset: '123',
+ });
+ // #create is ignored when filtering
+ await checkUrlToState('/admin/groups/q/filter:asdf,123#create', {
+ ...defaultState,
+ filter: 'asdf',
+ offset: '123',
+ });
+ // filter is decoded (twice)
+ await checkUrlToState(
+ '/admin/groups/q/filter:XX%20XX%2520XX%252FXX%3FXX',
+ {...defaultState, filter: 'XX XX XX/XX?XX'}
+ );
+
+ // Slash must be double encoded in `filter` param.
+ await checkUrlNotMatched('/admin/groups/q/filter:asdf/qwer,11');
+ await checkUrlNotMatched('/admin/groups/q/filter:asdf%2Fqwer,11');
});
test('handleGroupRoute', () => {
