Merge branch 'winh-geo-nodes-tests-timing' into 'master'

Fix timing in geo_nodes/components/app_spec.js

Closes #7012

See merge request gitlab-org/gitlab-ee!6661

ee/app/assets/javascripts/geo_nodes/components/app.vue

@@ -83,7 +83,7 @@ export default {
       });
     },
     fetchGeoNodes() {
-      this.service
+      return this.service
         .getGeoNodes()
         .then(res => res.data)
         .then(nodes => {
@@ -92,9 +92,7 @@ export default {
         })
         .catch(() => {
           this.isLoading = false;
-          Flash(
-            s__('GeoNodes|Something went wrong while fetching nodes'),
-          );
+          Flash(s__('GeoNodes|Something went wrong while fetching nodes'));
         });
     },
     fetchNodeDetails(node) {
@@ -109,10 +107,7 @@ export default {
             primaryRevision: primaryNodeVersion.revision,
           });
           this.store.setNodeDetails(nodeId, updatedNodeDetails);
-          eventHub.$emit(
-            'nodeDetailsLoaded',
-            this.store.getNodeDetails(nodeId),
-          );
+          eventHub.$emit('nodeDetailsLoaded', this.store.getNodeDetails(nodeId));
         })
         .catch(err => {
           if (err.response && err.response.data) {
@@ -124,10 +119,7 @@ export default {
               sync_status_unavailable: true,
               storage_shards_match: null,
             });
-            eventHub.$emit(
-              'nodeDetailsLoaded',
-              this.store.getNodeDetails(nodeId),
-            );
+            eventHub.$emit('nodeDetailsLoaded', this.store.getNodeDetails(nodeId));
           } else {
             eventHub.$emit('nodeDetailsLoadFailed', nodeId, err);
           }
@@ -135,14 +127,11 @@ export default {
     },
     repairNode(targetNode) {
       this.setNodeActionStatus(targetNode, true);
-      this.service
+      return this.service
         .repairNode(targetNode)
         .then(() => {
           this.setNodeActionStatus(targetNode, false);
-          Flash(
-            s__('GeoNodes|Node Authentication was successfully repaired.'),
-            'notice',
-          );
+          Flash(s__('GeoNodes|Node Authentication was successfully repaired.'), 'notice');
         })
         .catch(() => {
           this.setNodeActionStatus(targetNode, false);
@@ -151,7 +140,7 @@ export default {
     },
     toggleNode(targetNode) {
       this.setNodeActionStatus(targetNode, true);
-      this.service
+      return this.service
         .toggleNode(targetNode)
         .then(res => res.data)
         .then(node => {
@@ -162,14 +151,12 @@ export default {
         })
         .catch(() => {
           this.setNodeActionStatus(targetNode, false);
-          Flash(
-            s__('GeoNodes|Something went wrong while changing node status'),
-          );
+          Flash(s__('GeoNodes|Something went wrong while changing node status'));
         });
     },
     removeNode(targetNode) {
       this.setNodeActionStatus(targetNode, true);
-      this.service
+      return this.service
         .removeNode(targetNode)
         .then(() => {
           this.store.removeNode(targetNode);

ee/spec/javascripts/geo_nodes/components/app_spec.js

@@ -8,7 +8,13 @@ import GeoNodesStore from 'ee/geo_nodes/store/geo_nodes_store';
 import GeoNodesService from 'ee/geo_nodes/service/geo_nodes_service';
 import mountComponent from 'spec/helpers/vue_mount_component_helper';
 import { NODE_ACTIONS } from 'ee/geo_nodes/constants';
-import { PRIMARY_VERSION, NODE_DETAILS_PATH, mockNodes, mockNode, rawMockNodeDetails } from '../mock_data';
+import {
+  PRIMARY_VERSION,
+  NODE_DETAILS_PATH,
+  mockNodes,
+  mockNode,
+  rawMockNodeDetails,
+} from '../mock_data';
 
 const createComponent = () => {
   const Component = Vue.extend(appComponent);
@@ -86,187 +92,244 @@ describe('AppComponent', () => {
     });
 
     describe('fetchGeoNodes', () => {
-      it('calls service.getGeoNodes and sets response to the store on success', (done) => {
+      it('calls service.getGeoNodes and sets response to the store on success', done => {
         spyOn(vm.store, 'setNodes');
 
-        vm.fetchGeoNodes();
-        setTimeout(() => {
+        vm
+          .fetchGeoNodes()
+          .then(() => {
             expect(vm.store.setNodes).toHaveBeenCalledWith(mockNodes);
             expect(vm.isLoading).toBe(false);
-          done();
-        }, 0);
+          })
+          .then(done)
+          .catch(done.fail);
       });
 
-      it('sets error flag and message on failure', (done) => {
+      it('sets error flag and message on failure', done => {
         response = 'Something went wrong';
         statusCode = 500;
 
-        vm.fetchGeoNodes();
-        setTimeout(() => {
+        vm
+          .fetchGeoNodes()
+          .then(() => {
             expect(vm.isLoading).toBe(false);
-          expect(document.querySelector('.flash-text').innerText.trim()).toBe('Something went wrong while fetching nodes');
-          done();
-        }, 0);
+            expect(document.querySelector('.flash-text').innerText.trim()).toBe(
+              'Something went wrong while fetching nodes',
+            );
+          })
+          .then(done)
+          .catch(done.fail);
       });
     });
 
     describe('fetchNodeDetails', () => {
-      it('calls service.getGeoNodeDetails and sets response to the store on success', (done) => {
+      it('calls service.getGeoNodeDetails and sets response to the store on success', done => {
         mock.onGet(mockNode.statusPath).reply(200, rawMockNodeDetails);
         spyOn(vm.service, 'getGeoNodeDetails').and.callThrough();
 
-        vm.fetchNodeDetails(mockNode);
-        setTimeout(() => {
+        vm
+          .fetchNodeDetails(mockNode)
+          .then(() => {
             expect(vm.service.getGeoNodeDetails).toHaveBeenCalled();
             expect(Object.keys(vm.store.state.nodeDetails).length).not.toBe(0);
             expect(vm.store.state.nodeDetails['1']).toBeDefined();
-          done();
-        }, 0);
+          })
+          .then(done)
+          .catch(done.fail);
       });
 
-      it('emits `nodeDetailsLoaded` event with fake nodeDetails object on 404 failure', (done) => {
+      it('emits `nodeDetailsLoaded` event with fake nodeDetails object on 404 failure', done => {
         spyOn(eventHub, '$emit');
         mock.onGet(mockNode.statusPath).reply(404, {});
         spyOn(vm.service, 'getGeoNodeDetails').and.callThrough();
 
-        vm.fetchNodeDetails(mockNode);
-        setTimeout(() => {
+        vm
+          .fetchNodeDetails(mockNode)
+          .then(() => {
             expect(eventHub.$emit).toHaveBeenCalledWith('nodeDetailsLoaded', jasmine.any(Object));
             const nodeDetails = vm.store.state.nodeDetails['1'];
             expect(nodeDetails).toBeDefined();
             expect(nodeDetails.syncStatusUnavailable).toBe(true);
             expect(nodeDetails.health).toBe('Request failed with status code 404');
-          done();
-        }, 0);
+          })
+          .then(done)
+          .catch(done.fail);
       });
 
-      it('emits `nodeDetailsLoaded` event with fake nodeDetails object on 500 failure', (done) => {
+      it('emits `nodeDetailsLoaded` event with fake nodeDetails object on 500 failure', done => {
         spyOn(eventHub, '$emit');
         mock.onGet(mockNode.statusPath).reply(500, {});
         spyOn(vm.service, 'getGeoNodeDetails').and.callThrough();
 
-        vm.fetchNodeDetails(mockNode);
-        setTimeout(() => {
+        vm
+          .fetchNodeDetails(mockNode)
+          .then(() => {
             expect(eventHub.$emit).toHaveBeenCalledWith('nodeDetailsLoaded', jasmine.any(Object));
             const nodeDetails = vm.store.state.nodeDetails['1'];
             expect(nodeDetails).toBeDefined();
             expect(nodeDetails.syncStatusUnavailable).toBe(true);
             expect(nodeDetails.health).toBe('Request failed with status code 500');
-          done();
-        }, 0);
+          })
+          .then(done)
+          .catch(done.fail);
       });
 
-      it('emits `nodeDetailsLoadFailed` event on failure when there is no response', (done) => {
+      it('emits `nodeDetailsLoadFailed` event on failure when there is no response', done => {
         spyOn(eventHub, '$emit');
         mock.onGet(mockNode.statusPath).reply(500, null);
         spyOn(vm.service, 'getGeoNodeDetails').and.callThrough();
 
-        vm.fetchNodeDetails(mockNode);
-        setTimeout(() => {
-          expect(eventHub.$emit).toHaveBeenCalledWith('nodeDetailsLoadFailed', mockNode.id, jasmine.any(Object));
+        vm
+          .fetchNodeDetails(mockNode)
+          .then(() => {
+            expect(eventHub.$emit).toHaveBeenCalledWith(
+              'nodeDetailsLoadFailed',
+              mockNode.id,
+              jasmine.any(Object),
+            );
             done();
-        }, 0);
+          })
+          .then(done)
+          .catch(done.fail);
       });
     });
 
     describe('repairNode', () => {
-      it('calls service.repairNode and shows success Flash message on request success', (done) => {
+      it('calls service.repairNode and shows success Flash message on request success', done => {
         const node = { ...mockNode };
-        mock.onPost(node.repairPath).reply(200);
+        mock.onPost(node.repairPath).reply(() => {
+          expect(node.nodeActionActive).toBe(true);
+          return [200];
+        });
         spyOn(vm.service, 'repairNode').and.callThrough();
 
-        vm.repairNode(node);
-        expect(node.nodeActionActive).toBe(true);
-        setTimeout(() => {
+        vm
+          .repairNode(node)
+          .then(() => {
             expect(vm.service.repairNode).toHaveBeenCalledWith(node);
-          expect(document.querySelector('.flash-text').innerText.trim()).toBe('Node Authentication was successfully repaired.');
+            expect(document.querySelector('.flash-text').innerText.trim()).toBe(
+              'Node Authentication was successfully repaired.',
+            );
             expect(node.nodeActionActive).toBe(false);
-          done();
-        }, 0);
+          })
+          .then(done)
+          .catch(done.fail);
       });
 
-      it('calls service.repairNode and shows failure Flash message on request failure', (done) => {
+      it('calls service.repairNode and shows failure Flash message on request failure', done => {
         const node = { ...mockNode };
-        mock.onPost(node.repairPath).reply(500);
+        mock.onPost(node.repairPath).reply(() => {
+          expect(node.nodeActionActive).toBe(true);
+          return [500];
+        });
         spyOn(vm.service, 'repairNode').and.callThrough();
 
-        vm.repairNode(node);
-        setTimeout(() => {
+        vm
+          .repairNode(node)
+          .then(() => {
             expect(vm.service.repairNode).toHaveBeenCalledWith(node);
-          expect(document.querySelector('.flash-text').innerText.trim()).toBe('Something went wrong while repairing node');
+            expect(document.querySelector('.flash-text').innerText.trim()).toBe(
+              'Something went wrong while repairing node',
+            );
             expect(node.nodeActionActive).toBe(false);
-          done();
-        }, 0);
+          })
+          .then(done)
+          .catch(done.fail);
       });
     });
 
     describe('toggleNode', () => {
-      it('calls service.toggleNode for enabling node and updates toggle button on request success', (done) => {
+      it('calls service.toggleNode for enabling node and updates toggle button on request success', done => {
         const node = { ...mockNode };
-        mock.onPut(node.basePath).reply(200, {
+        mock.onPut(node.basePath).reply(() => {
+          expect(node.nodeActionActive).toBe(true);
+          return [
+            200,
+            {
               enabled: true,
+            },
+          ];
         });
         spyOn(vm.service, 'toggleNode').and.callThrough();
         node.enabled = false;
 
-        vm.toggleNode(node);
-        expect(node.nodeActionActive).toBe(true);
-        setTimeout(() => {
+        vm
+          .toggleNode(node)
+          .then(() => {
             expect(vm.service.toggleNode).toHaveBeenCalledWith(node);
             expect(node.enabled).toBe(true);
             expect(node.nodeActionActive).toBe(false);
-          done();
-        }, 0);
+          })
+          .then(done)
+          .catch(done.fail);
       });
 
-      it('calls service.toggleNode and shows Flash error on request failure', (done) => {
+      it('calls service.toggleNode and shows Flash error on request failure', done => {
         const node = { ...mockNode };
-        mock.onPut(node.basePath).reply(500);
+        mock.onPut(node.basePath).reply(() => {
+          expect(node.nodeActionActive).toBe(true);
+          return [500];
+        });
         spyOn(vm.service, 'toggleNode').and.callThrough();
         node.enabled = false;
 
-        vm.toggleNode(node);
-        expect(node.nodeActionActive).toBe(true);
-        setTimeout(() => {
+        vm
+          .toggleNode(node)
+          .then(() => {
             expect(vm.service.toggleNode).toHaveBeenCalledWith(node);
-          expect(document.querySelector('.flash-text').innerText.trim()).toBe('Something went wrong while changing node status');
+            expect(document.querySelector('.flash-text').innerText.trim()).toBe(
+              'Something went wrong while changing node status',
+            );
             expect(node.nodeActionActive).toBe(false);
-          done();
-        }, 0);
+          })
+          .then(done)
+          .catch(done.fail);
       });
     });
 
     describe('removeNode', () => {
-      it('calls service.removeNode for removing node and shows Flash message on request success', (done) => {
+      it('calls service.removeNode for removing node and shows Flash message on request success', done => {
         const node = { ...mockNode };
-        mock.onDelete(node.basePath).reply(200);
+        mock.onDelete(node.basePath).reply(() => {
+          expect(node.nodeActionActive).toBe(true);
+          return [200];
+        });
         spyOn(vm.service, 'removeNode').and.callThrough();
         spyOn(vm.store, 'removeNode').and.stub();
 
-        vm.removeNode(node);
-        expect(node.nodeActionActive).toBe(true);
-        setTimeout(() => {
+        vm
+          .removeNode(node)
+          .then(() => {
             expect(vm.service.removeNode).toHaveBeenCalledWith(node);
             expect(vm.store.removeNode).toHaveBeenCalledWith(node);
-          expect(document.querySelector('.flash-text').innerText.trim()).toBe('Node was successfully removed.');
-          done();
-        }, 0);
+            expect(document.querySelector('.flash-text').innerText.trim()).toBe(
+              'Node was successfully removed.',
+            );
+          })
+          .then(done)
+          .catch(done.fail);
       });
 
-      it('calls service.removeNode and shows Flash message on request failure', (done) => {
+      it('calls service.removeNode and shows Flash message on request failure', done => {
         const node = { ...mockNode };
-        mock.onDelete(node.basePath).reply(500);
+        mock.onDelete(node.basePath).reply(() => {
+          expect(node.nodeActionActive).toBe(true);
+          return [500];
+        });
         spyOn(vm.service, 'removeNode').and.callThrough();
         spyOn(vm.store, 'removeNode').and.stub();
 
-        vm.removeNode(node);
-        expect(node.nodeActionActive).toBe(true);
-        setTimeout(() => {
+        vm
+          .removeNode(node)
+          .then(() => {
             expect(vm.service.removeNode).toHaveBeenCalledWith(node);
             expect(vm.store.removeNode).not.toHaveBeenCalled();
-          expect(document.querySelector('.flash-text').innerText.trim()).toBe('Something went wrong while removing node');
-          done();
-        }, 0);
+            expect(document.querySelector('.flash-text').innerText.trim()).toBe(
+              'Something went wrong while removing node',
+            );
+          })
+          .then(done)
+          .catch(done.fail);
       });
     });
 
@@ -399,7 +462,9 @@ describe('AppComponent', () => {
 
     it('renders loading animation when `isLoading` is true', () => {
       vm.isLoading = true;
-      expect(vm.$el.querySelectorAll('.loading-animation.prepend-top-20.append-bottom-20').length).not.toBe(0);
+      expect(
+        vm.$el.querySelectorAll('.loading-animation.prepend-top-20.append-bottom-20').length,
+      ).not.toBe(0);
     });
   });
 });