Lior Vernia has posted comments on this change. Change subject: webadmin,userportal: Fix client-side sort issue ......................................................................
Patch Set 3: Code-Review+2 (1 comment) Please note additional comment. http://gerrit.ovirt.org/#/c/28392/3/frontend/webadmin/modules/uicommonweb/src/test/java/org/ovirt/engine/ui/uicommonweb/models/SortedListModelTest.java File frontend/webadmin/modules/uicommonweb/src/test/java/org/ovirt/engine/ui/uicommonweb/models/SortedListModelTest.java: Line 135: Line 136: expected = Arrays.asList(ITEM_1_1, ITEM_1_2, ITEM_2_1, ITEM_2_2); Line 137: Line 138: testedModel.setComparator(makeValueComparator()); Line 139: sorted = testedModel.sortItems(sorted); I would expect this to work even without calling sortItems() again, i.e. that setComparator() triggers the sorting by itself (this is actually a relevant comment for the two other calls to setComparator() above). I understand that this is a change of behavior for SortedListModel, and that's why I'm approving this patch (I know it also blocks a bunch of other patches), but as far as I'm concerned this test is currently tweaked to pass rather than test what it should really test. Please consider fixing it (and the implementation of setComparator() accordingly) in the future. Line 140: assertArrayEquals(sorted.toArray(), expected.toArray()); Line 141: } Line 142: -- To view, visit http://gerrit.ovirt.org/28392 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I437e9dd7ff2afdb2d87fb0ba6c17dcb081cd965d Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Vojtech Szocs <vsz...@redhat.com> Gerrit-Reviewer: Alexander Wels <aw...@redhat.com> Gerrit-Reviewer: Daniel Erez <de...@redhat.com> Gerrit-Reviewer: Einav Cohen <eco...@redhat.com> Gerrit-Reviewer: Frank Kobzik <fkob...@redhat.com> Gerrit-Reviewer: Lior Vernia <lver...@redhat.com> Gerrit-Reviewer: Vojtech Szocs <vsz...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
