Apply reviewer feedback

Use named arguments on
create component and clean
up use of vm on wrapper.

app/assets/javascripts/jobs/components/manual_variables_form.vue

@@ -117,7 +117,12 @@ export default {
         <div class="table-section section-50" role="rowheader">{{ s__('CiVariables|Value') }}</div>
       </div>
 
-      <div v-for="variable in variables" :key="variable.id" class="gl-responsive-table-row">
+      <div
+        v-for="variable in variables"
+        :key="variable.id"
+        class="gl-responsive-table-row"
+        data-testid="ci-variable-row"
+      >
         <div class="table-section section-50">
           <div class="table-mobile-header" role="rowheader">{{ s__('Pipeline|Key') }}</div>
           <div class="table-mobile-content gl-mr-3">
@@ -126,6 +131,7 @@ export default {
               v-model="variable.key"
               :placeholder="$options.i18n.keyPlaceholder"
               class="ci-variable-body-item form-control"
+              data-testid="ci-variable-key"
             />
           </div>
         </div>
@@ -138,6 +144,7 @@ export default {
               v-model="variable.secret_value"
               :placeholder="$options.i18n.valuePlaceholder"
               class="ci-variable-body-item form-control"
+              data-testid="ci-variable-value"
             />
           </div>
         </div>

spec/frontend/jobs/components/manual_variables_form_spec.js

@@ -21,7 +21,7 @@ describe('Manual Variables Form', () => {
     variablesSettingsUrl: '/settings',
   };
 
-  const createComponent = (props = {}, mountFn = shallowMount) => {
+  const createComponent = ({ props = {}, mountFn = shallowMount } = {}) => {
     store = new Vuex.Store({
       actions: {
         triggerManualJob: jest.fn(),
@@ -30,7 +30,7 @@ describe('Manual Variables Form', () => {
 
     wrapper = extendedWrapper(
       mountFn(localVue.extend(Form), {
-        propsData: props,
+        propsData: { ...requiredProps, ...props },
         localVue,
         store,
       }),
@@ -43,6 +43,9 @@ describe('Manual Variables Form', () => {
   const findTriggerBtn = () => wrapper.findByTestId('trigger-manual-job-btn');
   const findHelpText = () => wrapper.findByTestId('form-help-text');
   const findDeleteVarBtn = () => wrapper.findByTestId('delete-variable-btn');
+  const findCiVariableKey = () => wrapper.findByTestId('ci-variable-key');
+  const findCiVariableValue = () => wrapper.findByTestId('ci-variable-value');
+  const findAllVariables = () => wrapper.findAllByTestId('ci-variable-row');
 
   afterEach(() => {
     wrapper.destroy();
@@ -50,7 +53,7 @@ describe('Manual Variables Form', () => {
 
   describe('shallowMount', () => {
     beforeEach(() => {
-      createComponent(requiredProps);
+      createComponent();
     });
 
     it('renders empty form with correct placeholders', () => {
@@ -70,19 +73,25 @@ describe('Manual Variables Form', () => {
       it('creates a new variable when user types a new key and resets the form', async () => {
         await findInputKey().setValue('new key');
 
-        expect(wrapper.vm.variables).toHaveLength(1);
-        expect(wrapper.vm.variables[0].key).toBe('new key');
+        expect(findAllVariables()).toHaveLength(1);
+        expect(findCiVariableKey().element.value).toBe('new key');
         expect(findInputKey().attributes('value')).toBe(undefined);
       });
 
       it('creates a new variable when user types a new value and resets the form', async () => {
         await findInputValue().setValue('new value');
 
-        expect(wrapper.vm.variables).toHaveLength(1);
-        expect(wrapper.vm.variables[0].secret_value).toBe('new value');
+        expect(findAllVariables()).toHaveLength(1);
+        expect(findCiVariableValue().element.value).toBe('new value');
         expect(findInputValue().attributes('value')).toBe(undefined);
       });
     });
+  });
+
+  describe('mount', () => {
+    beforeEach(() => {
+      createComponent({ mountFn: mount });
+    });
 
     describe('when deleting a variable', () => {
       it('removes the variable row', async () => {
@@ -96,17 +105,15 @@ describe('Manual Variables Form', () => {
           ],
         });
 
-        findDeleteVarBtn().vm.$emit('click');
+        findDeleteVarBtn().trigger('click');
 
-        expect(wrapper.vm.variables).toHaveLength(0);
+        await wrapper.vm.$nextTick();
+
+        expect(findAllVariables()).toHaveLength(0);
       });
     });
-  });
 
-  describe('mount', () => {
     it('trigger button is disabled after trigger action', async () => {
-      createComponent(requiredProps, mount);
-
       expect(findTriggerBtn().props('disabled')).toBe(false);
 
       await findTriggerBtn().trigger('click');