Apply patch and update spec per MR comments

- Use wrapper instead of global Vue object
- Use vue utils helper method
- Remove redundant spec tests

spec/frontend/sidebar/components/assignees/assignee_avatar_link_spec.js

@@ -1,10 +1,13 @@
 import { shallowMount } from '@vue/test-utils';
 import { joinPaths } from '~/lib/utils/url_utility';
-import userDataMock from '../../user_data_mock';
+import { TEST_HOST } from 'helpers/test_constants';
 import AssigneeAvatarLink from '~/sidebar/components/assignees/assignee_avatar_link.vue';
+import AssigneeAvatar from '~/sidebar/components/assignees/assignee_avatar.vue';
+import userDataMock from '../../user_data_mock';
 
 const TOOLTIP_PLACEMENT = 'bottom';
-const { name: USER_NAME } = userDataMock();
+const { name: USER_NAME, username: USER_USERNAME } = userDataMock();
+const TEST_ISSUABLE_TYPE = 'merge_request';
 
 describe('AssigneeAvatarLink component', () => {
   let wrapper;
@@ -13,10 +16,10 @@ describe('AssigneeAvatarLink component', () => {
     const propsData = {
       user: userDataMock(),
       showLess: true,
-      rootPath: 'http://localhost:3000/',
+      rootPath: TEST_HOST,
       tooltipPlacement: TOOLTIP_PLACEMENT,
       singleUser: false,
-      issuableType: 'merge_request',
+      issuableType: TEST_ISSUABLE_TYPE,
       ...props,
     };
 
@@ -32,26 +35,24 @@ describe('AssigneeAvatarLink component', () => {
 
   const findTooltipText = () => wrapper.attributes('data-original-title');
 
-  it('user who cannot merge has "cannot merge" in tooltip', () => {
-    createComponent({
-      user: {
-        can_merge: false,
-      },
-    });
-
-    expect(findTooltipText().includes('cannot merge')).toBe(true);
-  });
-
   it('has the root url present in the assigneeUrl method', () => {
     createComponent();
-    const assigneeUrl = joinPaths(
-      `${wrapper.props('rootPath')}`,
-      `${wrapper.props('user').username}`,
-    );
+    const assigneeUrl = joinPaths(TEST_HOST, USER_USERNAME);
 
     expect(wrapper.attributes().href).toEqual(assigneeUrl);
   });
 
+  it('renders assignee avatar', () => {
+    createComponent();
+
+    expect(wrapper.find(AssigneeAvatar).props()).toEqual(
+      expect.objectContaining({
+        issuableType: TEST_ISSUABLE_TYPE,
+        user: userDataMock(),
+      }),
+    );
+  });
+
   describe.each`
     issuableType       | tooltipHasName | canMerge | expected
     ${'merge_request'} | ${true}        | ${true}  | ${USER_NAME}

spec/frontend/sidebar/components/assignees/assignee_avatar_spec.js

@@ -39,7 +39,7 @@ describe('AssigneeAvatar', () => {
   it('does not show warning icon if assignee can merge', () => {
     createComponent();
 
-    expect(wrapper.element.querySelector('.merge-icon')).toBeNull();
+    expect(wrapper.find('.merge-icon').exists()).toBe(false);
   });
 
   it('shows warning icon if assignee cannot merge', () => {
@@ -49,7 +49,7 @@ describe('AssigneeAvatar', () => {
       },
     });
 
-    expect(wrapper.element.querySelector('.merge-icon')).not.toBeNull();
+    expect(wrapper.find('.merge-icon').exists()).toBe(true);
   });
 
   it('does not show warning icon for issuableType = "issue"', () => {
@@ -57,7 +57,7 @@ describe('AssigneeAvatar', () => {
       issuableType: 'issue',
     });
 
-    expect(wrapper.element.querySelector('.merge-icon')).toBeNull();
+    expect(wrapper.find('.merge-icon').exists()).toBe(false);
   });
 
   it.each`

spec/frontend/sidebar/components/assignees/collapsed_assignee_list_spec.js

@@ -77,54 +77,30 @@ describe('CollapsedAssigneeList component', () => {
 
     beforeEach(() => {
       users = UsersMockHelper.createNumberRandomUsers(2);
+
+      createComponent({ users });
     });
 
-    describe('default', () => {
-      beforeEach(() => {
-        createComponent({ users });
-      });
-
-      it('has multiple-users class', () => {
-        expect(wrapper.classes('multiple-users')).toBe(true);
-      });
-
-      it('does not display an avatar count', () => {
-        expect(findAvatarCounter().exists()).toBe(false);
-      });
-
-      it('returns just two collapsed users', () => {
-        expect(findAssignees().length).toBe(2);
-      });
+    it('has multiple-users class', () => {
+      expect(wrapper.classes('multiple-users')).toBe(true);
     });
 
-    it('has correct "cannot merge" tooltip when no user can merge', () => {
-      users[0].can_merge = false;
-      users[1].can_merge = false;
-
-      createComponent({
-        users,
-      });
-
-      expect(getTooltipTitle()).toEqual(`${users[0].name}, ${users[1].name} (no one can merge)`);
+    it('does not display an avatar count', () => {
+      expect(findAvatarCounter().exists()).toBe(false);
     });
 
-    it('does not have "merge" word in tooltip if everyone can merge', () => {
-      users[0].can_merge = true;
-      users[1].can_merge = true;
-
-      createComponent({
-        users,
-      });
-
-      expect(getTooltipTitle()).toEqual(`${users[0].name}, ${users[1].name}`);
+    it('returns just two collapsed users', () => {
+      expect(findAssignees().length).toBe(2);
     });
   });
 
   describe('More than two assignees/users', () => {
     let users;
+    let userNames;
 
     beforeEach(() => {
       users = UsersMockHelper.createNumberRandomUsers(3);
+      userNames = users.map(x => x.name).join(', ');
     });
 
     describe('default', () => {
@@ -142,6 +118,18 @@ describe('CollapsedAssigneeList component', () => {
       });
     });
 
+    it('has corrent "no one can merge" tooltip when no one can merge', () => {
+      users[0].can_merge = false;
+      users[1].can_merge = false;
+      users[2].can_merge = false;
+
+      createComponent({
+        users,
+      });
+
+      expect(getTooltipTitle()).toEqual(`${userNames} (no one can merge)`);
+    });
+
     it('has correct "cannot merge" tooltip when one user can merge', () => {
       users[0].can_merge = true;
       users[1].can_merge = false;
@@ -151,7 +139,7 @@ describe('CollapsedAssigneeList component', () => {
         users,
       });
 
-      expect(getTooltipTitle()).toContain('1/3 can merge');
+      expect(getTooltipTitle()).toEqual(`${userNames} (1/3 can merge)`);
     });
 
     it('has correct "cannot merge" tooltip when more than one user can merge', () => {
@@ -163,7 +151,7 @@ describe('CollapsedAssigneeList component', () => {
         users,
       });
 
-      expect(getTooltipTitle()).toContain('2/3 can merge');
+      expect(getTooltipTitle()).toEqual(`${userNames} (2/3 can merge)`);
     });
 
     it('does not have "merge" in tooltip if everyone can merge', () => {
@@ -175,10 +163,10 @@ describe('CollapsedAssigneeList component', () => {
         users,
       });
 
-      expect(getTooltipTitle()).not.toContain('merge');
+      expect(getTooltipTitle()).toEqual(userNames);
     });
 
-    it('displays the correct avatar count via a computed property if less than default max counter', () => {
+    it('displays the correct avatar count', () => {
       users = UsersMockHelper.createNumberRandomUsers(5);
 
       createComponent({

spec/frontend/sidebar/components/assignees/collapsed_assignee_spec.js

@@ -1,13 +1,18 @@
 import { shallowMount } from '@vue/test-utils';
 import CollapsedAssignee from '~/sidebar/components/assignees/collapsed_assignee.vue';
+import AssigneeAvatar from '~/sidebar/components/assignees/assignee_avatar.vue';
 import userDataMock from '../../user_data_mock';
 
+const TEST_USER = userDataMock();
+const TEST_ISSUABLE_TYPE = 'merge_request';
+
 describe('CollapsedAssignee assignee component', () => {
   let wrapper;
 
   function createComponent(props = {}) {
     const propsData = {
       user: userDataMock(),
+      issuableType: TEST_ISSUABLE_TYPE,
       ...props,
     };
 
@@ -29,6 +34,16 @@ describe('CollapsedAssignee assignee component', () => {
         .find('.author')
         .text()
         .trim(),
-    ).toEqual(wrapper.vm.user.name);
+    ).toEqual(TEST_USER.name);
+  });
+
+  it('has assignee avatar', () => {
+    createComponent();
+
+    expect(wrapper.find(AssigneeAvatar).props()).toEqual({
+      imgSize: 24,
+      user: TEST_USER,
+      issuableType: TEST_ISSUABLE_TYPE,
+    });
   });
 });

spec/frontend/sidebar/components/assignees/uncollapsed_assignee_list_spec.js

@@ -1,4 +1,3 @@
-import Vue from 'vue';
 import { mount } from '@vue/test-utils';
 import UncollapsedAssigneeList from '~/sidebar/components/assignees/uncollapsed_assignee_list.vue';
 import AssigneeAvatarLink from '~/sidebar/components/assignees/assignee_avatar_link.vue';
@@ -56,79 +55,48 @@ describe('UncollapsedAssigneeList component', () => {
     });
   });
 
-  describe('Two or more assignees/users', () => {
-    beforeEach(() => {
-      createComponent({
-        users: UsersMockHelper.createNumberRandomUsers(3),
-      });
-    });
-
-    it('more than one user', () => {
-      expect(wrapper.findAll(AssigneeAvatarLink).length).toBe(3);
-    });
-
-    it('shows the "show-less" assignees label', done => {
-      const users = UsersMockHelper.createNumberRandomUsers(6);
-
-      createComponent({
-        users,
-      });
-
-      expect(wrapper.vm.$el.querySelectorAll('.user-item').length).toEqual(DEFAULT_RENDER_COUNT);
-
-      expect(wrapper.vm.$el.querySelector('.user-list-more')).not.toBe(null);
-      const usersLabelExpectation = users.length - DEFAULT_RENDER_COUNT;
-
-      expect(wrapper.vm.$el.querySelector('.user-list-more .btn-link').innerText.trim()).not.toBe(
-        `+${usersLabelExpectation} more`,
-      );
-      wrapper.vm.toggleShowLess();
-      Vue.nextTick(() => {
-        expect(wrapper.vm.$el.querySelector('.user-list-more .btn-link').innerText.trim()).toBe(
-          '- show less',
-        );
-        done();
-      });
-    });
-
-    it('shows the "show-less" when "n+ more " label is clicked', done => {
-      createComponent({
-        users: UsersMockHelper.createNumberRandomUsers(6),
-      });
-
-      wrapper.vm.$el.querySelector('.user-list-more .btn-link').click();
-      Vue.nextTick(() => {
-        expect(wrapper.vm.$el.querySelector('.user-list-more .btn-link').innerText.trim()).toBe(
-          '- show less',
-        );
-        done();
-      });
-    });
-
-    it('does not show n+ more label when less than render count', () => {
-      expect(findMoreButton().exists()).toBe(false);
-    });
-  });
-
   describe('n+ more label', () => {
-    beforeEach(() => {
-      createComponent({
-        users: UsersMockHelper.createNumberRandomUsers(DEFAULT_RENDER_COUNT + 1),
+    describe('when users count is rendered users', () => {
+      beforeEach(() => {
+        createComponent({
+          users: UsersMockHelper.createNumberRandomUsers(DEFAULT_RENDER_COUNT),
+        });
+      });
+
+      it('does not show more label', () => {
+        expect(findMoreButton().exists()).toBe(false);
       });
     });
 
-    it('shows "+1 more" label', () => {
-      expect(findMoreButton().text()).toBe('+ 1 more');
-      expect(wrapper.findAll(AssigneeAvatarLink).length).toBe(DEFAULT_RENDER_COUNT);
-    });
+    describe('when more than rendered users', () => {
+      beforeEach(() => {
+        createComponent({
+          users: UsersMockHelper.createNumberRandomUsers(DEFAULT_RENDER_COUNT + 1),
+        });
+      });
 
-    it('shows "show less" label', done => {
-      findMoreButton().trigger('click');
+      it('shows "+1 more" label', () => {
+        expect(findMoreButton().text()).toBe('+ 1 more');
+      });
 
-      Vue.nextTick(() => {
-        expect(findMoreButton().text()).toBe('- show less');
-        expect(wrapper.findAll(AssigneeAvatarLink).length).toBe(DEFAULT_RENDER_COUNT + 1);
-        done();
+      it('shows truncated users', () => {
+        expect(wrapper.findAll(AssigneeAvatarLink).length).toBe(DEFAULT_RENDER_COUNT);
+      });
+
+      describe('when more button is clicked', () => {
+        beforeEach(() => {
+          findMoreButton().trigger('click');
+
+          return wrapper.vm.$nextTick();
+        });
+
+        it('shows "show less" label', () => {
+          expect(findMoreButton().text()).toBe('- show less');
+        });
+
+        it('shows all users', () => {
+          expect(wrapper.findAll(AssigneeAvatarLink).length).toBe(DEFAULT_RENDER_COUNT + 1);
+        });
       });
     });
   });

spec/javascripts/sidebar/assignees_spec.js

@@ -224,7 +224,6 @@ describe('Assignee component', () => {
 
       expect(userItems.length).toBe(3);
       expect(userItems[0].dataset.originalTitle).toBe(users[2].name);
-      expect(userItems[0].dataset.originalTitle).not.toBe(users[0].name);
     });
 
     it('passes the sorted assignees to the collapsed-assignee-list', () => {
@@ -244,7 +243,6 @@ describe('Assignee component', () => {
       const collapsedButton = component.$el.querySelector('.sidebar-collapsed-user button');
 
       expect(collapsedButton.innerText.trim()).toBe(users[2].name);
-      expect(collapsedButton.innerText.trim()).not.toBe(users[0].name);
     });
   });
 });

spec/javascripts/sidebar/mock_data.js

@@ -1,13 +1,3 @@
-export const userDataMock = {
-  avatar_url: 'mock_path',
-  id: 1,
-  name: 'Root',
-  state: 'active',
-  username: 'root',
-  web_url: '',
-  can_merge: true,
-};
-
 const RESPONSE_MAP = {
   GET: {
     '/gitlab-org/gitlab-shell/issues/5.json': {