From 069431a55323748b75af89a68867a208aecf52d0 Mon Sep 17 00:00:00 2001
From: Phil Hughes <me@iamphill.com>
Date: Fri, 6 Apr 2018 11:36:03 +0100
Subject: Added tests for IDE router

We have a lot of logic inside of the router without any tests. This
fixes that by adding tests for each different route that we handle.
---
 .../ide/components/ide_project_branches_tree.vue   |  37 ++---
 app/assets/javascripts/ide/ide_router.js           |  57 +++++---
 app/assets/javascripts/ide/services/index.js       |   4 +-
 .../javascripts/ide/stores/actions/project.js      | 100 +++++++++-----
 app/assets/javascripts/ide/stores/actions/tree.js  |   3 +-
 spec/javascripts/ide/ide_router_spec.js            | 150 +++++++++++++++++++++
 spec/javascripts/ide/mock_data.js                  |  11 ++
 spec/javascripts/test_bundle.js                    |  20 +--
 8 files changed, 292 insertions(+), 90 deletions(-)
 create mode 100644 spec/javascripts/ide/ide_router_spec.js
 create mode 100644 spec/javascripts/ide/mock_data.js

diff --git a/app/assets/javascripts/ide/components/ide_project_branches_tree.vue b/app/assets/javascripts/ide/components/ide_project_branches_tree.vue
index eb2749e6151..ac3e71c19ac 100644
--- a/app/assets/javascripts/ide/components/ide_project_branches_tree.vue
+++ b/app/assets/javascripts/ide/components/ide_project_branches_tree.vue
@@ -1,25 +1,25 @@
 <script>
-  import icon from '~/vue_shared/components/icon.vue';
-  import repoTree from './ide_repo_tree.vue';
-  import newDropdown from './new_dropdown/index.vue';
+import icon from '~/vue_shared/components/icon.vue';
+import repoTree from './ide_repo_tree.vue';
+import newDropdown from './new_dropdown/index.vue';
 
-  export default {
-    components: {
-      repoTree,
-      icon,
-      newDropdown,
+export default {
+  components: {
+    repoTree,
+    icon,
+    newDropdown,
+  },
+  props: {
+    projectId: {
+      type: String,
+      required: true,
     },
-    props: {
-      projectId: {
-        type: String,
-        required: true,
-      },
-      branch: {
-        type: Object,
-        required: true,
-      },
+    branch: {
+      type: Object,
+      required: true,
     },
-  };
+  },
+};
 </script>
 
 <template>
@@ -41,6 +41,7 @@
       </div>
     </div>
     <repo-tree
+      v-if="branch.tree"
       :tree="branch.tree"
     />
   </div>
diff --git a/app/assets/javascripts/ide/ide_router.js b/app/assets/javascripts/ide/ide_router.js
index 20983666b4a..e87fa2a117b 100644
--- a/app/assets/javascripts/ide/ide_router.js
+++ b/app/assets/javascripts/ide/ide_router.js
@@ -39,6 +39,10 @@ const router = new VueRouter({
       path: '/project/:namespace/:project',
       component: EmptyRouterComponent,
       children: [
+        {
+          path: ':targetmode/:branch',
+          component: EmptyRouterComponent,
+        },
         {
           path: ':targetmode/:branch/*',
           component: EmptyRouterComponent,
@@ -63,30 +67,36 @@ router.beforeEach((to, from, next) => {
         const fullProjectId = `${to.params.namespace}/${to.params.project}`;
 
         if (to.params.branch) {
-          store.dispatch('getBranchData', {
-            projectId: fullProjectId,
-            branchId: to.params.branch,
-          });
-
           store
-            .dispatch('getFiles', {
+            .dispatch('getBranchData', {
               projectId: fullProjectId,
               branchId: to.params.branch,
             })
-            .then(() => {
-              if (to.params[0]) {
-                const path =
-                  to.params[0].slice(-1) === '/' ? to.params[0].slice(0, -1) : to.params[0];
-                const treeEntryKey = Object.keys(store.state.entries).find(
-                  key => key === path && !store.state.entries[key].pending,
-                );
-                const treeEntry = store.state.entries[treeEntryKey];
-
-                if (treeEntry) {
-                  store.dispatch('handleTreeEntryAction', treeEntry);
-                }
-              }
-            })
+            .then(() =>
+              store
+                .dispatch('getFiles', {
+                  projectId: fullProjectId,
+                  branchId: to.params.branch,
+                })
+                .then(() => {
+                  if (to.params[0]) {
+                    const path =
+                      to.params[0].slice(-1) === '/' ? to.params[0].slice(0, -1) : to.params[0];
+                    const treeEntryKey = Object.keys(store.state.entries).find(
+                      key => key === path && !store.state.entries[key].pending,
+                    );
+                    const treeEntry = store.state.entries[treeEntryKey];
+
+                    if (treeEntry) {
+                      return store.dispatch('handleTreeEntryAction', treeEntry).then(next);
+                    }
+
+                    return next();
+                  }
+
+                  return next();
+                }),
+            )
             .catch(e => {
               flash(
                 'Error while loading the branch files. Please try again.',
@@ -148,10 +158,13 @@ router.beforeEach((to, from, next) => {
                 }
               });
             })
+            .then(next)
             .catch(e => {
               flash('Error while loading the merge request. Please try again.');
               throw e;
             });
+        } else {
+          next();
         }
       })
       .catch(e => {
@@ -165,9 +178,9 @@ router.beforeEach((to, from, next) => {
         );
         throw e;
       });
+  } else {
+    next();
   }
-
-  next();
 });
 
 export default router;
diff --git a/app/assets/javascripts/ide/services/index.js b/app/assets/javascripts/ide/services/index.js
index a12e637616a..b8d23b00a82 100644
--- a/app/assets/javascripts/ide/services/index.js
+++ b/app/assets/javascripts/ide/services/index.js
@@ -1,4 +1,5 @@
 import Vue from 'vue';
+import axios from '~/lib/utils/axios_utils';
 import VueResource from 'vue-resource';
 import Api from '~/api';
 
@@ -68,8 +69,7 @@ export default {
     });
   },
   getFiles(projectUrl, branchId) {
-    const url = `${projectUrl}/files/${branchId}`;
-    return Vue.http.get(url, {
+    return axios.get(`${projectUrl}/files/${branchId}`, {
       params: {
         format: 'json',
       },
diff --git a/app/assets/javascripts/ide/stores/actions/project.js b/app/assets/javascripts/ide/stores/actions/project.js
index b3882cb8d21..e20a53fadc7 100644
--- a/app/assets/javascripts/ide/stores/actions/project.js
+++ b/app/assets/javascripts/ide/stores/actions/project.js
@@ -5,45 +5,71 @@ import * as types from '../mutation_types';
 export const getProjectData = (
   { commit, state, dispatch },
   { namespace, projectId, force = false } = {},
-) => new Promise((resolve, reject) => {
-  if (!state.projects[`${namespace}/${projectId}`] || force) {
-    commit(types.TOGGLE_LOADING, { entry: state });
-    service.getProjectData(namespace, projectId)
-    .then(res => res.data)
-    .then((data) => {
+) =>
+  new Promise((resolve, reject) => {
+    if (!state.projects[`${namespace}/${projectId}`] || force) {
       commit(types.TOGGLE_LOADING, { entry: state });
-      commit(types.SET_PROJECT, { projectPath: `${namespace}/${projectId}`, project: data });
-      if (!state.currentProjectId) commit(types.SET_CURRENT_PROJECT, `${namespace}/${projectId}`);
-      resolve(data);
-    })
-    .catch(() => {
-      flash('Error loading project data. Please try again.', 'alert', document, null, false, true);
-      reject(new Error(`Project not loaded ${namespace}/${projectId}`));
-    });
-  } else {
-    resolve(state.projects[`${namespace}/${projectId}`]);
-  }
-});
+      service
+        .getProjectData(namespace, projectId)
+        .then(res => res.data)
+        .then(data => {
+          commit(types.TOGGLE_LOADING, { entry: state });
+          commit(types.SET_PROJECT, { projectPath: `${namespace}/${projectId}`, project: data });
+          if (!state.currentProjectId)
+            commit(types.SET_CURRENT_PROJECT, `${namespace}/${projectId}`);
+          resolve(data);
+        })
+        .catch(() => {
+          flash(
+            'Error loading project data. Please try again.',
+            'alert',
+            document,
+            null,
+            false,
+            true,
+          );
+          reject(new Error(`Project not loaded ${namespace}/${projectId}`));
+        });
+    } else {
+      resolve(state.projects[`${namespace}/${projectId}`]);
+    }
+  });
 
 export const getBranchData = (
   { commit, state, dispatch },
   { projectId, branchId, force = false } = {},
-) => new Promise((resolve, reject) => {
-  if ((typeof state.projects[`${projectId}`] === 'undefined' ||
-        !state.projects[`${projectId}`].branches[branchId])
-        || force) {
-    service.getBranchData(`${projectId}`, branchId)
-      .then(({ data }) => {
-        const { id } = data.commit;
-        commit(types.SET_BRANCH, { projectPath: `${projectId}`, branchName: branchId, branch: data });
-        commit(types.SET_BRANCH_WORKING_REFERENCE, { projectId, branchId, reference: id });
-        resolve(data);
-      })
-      .catch(() => {
-        flash('Error loading branch data. Please try again.', 'alert', document, null, false, true);
-        reject(new Error(`Branch not loaded - ${projectId}/${branchId}`));
-      });
-  } else {
-    resolve(state.projects[`${projectId}`].branches[branchId]);
-  }
-});
+) =>
+  new Promise((resolve, reject) => {
+    if (
+      typeof state.projects[`${projectId}`] === 'undefined' ||
+      !state.projects[`${projectId}`].branches[branchId] ||
+      force
+    ) {
+      service
+        .getBranchData(`${projectId}`, branchId)
+        .then(({ data }) => {
+          const { id } = data.commit;
+          commit(types.SET_BRANCH, {
+            projectPath: `${projectId}`,
+            branchName: branchId,
+            branch: data,
+          });
+          commit(types.SET_BRANCH_WORKING_REFERENCE, { projectId, branchId, reference: id });
+          resolve(data);
+        })
+        .catch(e => {
+          flash(
+            'Error loading branch data. Please try again.',
+            'alert',
+            document,
+            null,
+            false,
+            true,
+          );
+          reject(new Error(`Branch not loaded - ${projectId}/${branchId}`));
+          throw e;
+        });
+    } else {
+      resolve(state.projects[`${projectId}`].branches[branchId]);
+    }
+  });
diff --git a/app/assets/javascripts/ide/stores/actions/tree.js b/app/assets/javascripts/ide/stores/actions/tree.js
index 6536be04f0a..44fc7ded25f 100644
--- a/app/assets/javascripts/ide/stores/actions/tree.js
+++ b/app/assets/javascripts/ide/stores/actions/tree.js
@@ -57,8 +57,7 @@ export const getFiles = ({ state, commit, dispatch }, { projectId, branchId } =
 
       service
         .getFiles(selectedProject.web_url, branchId)
-        .then(res => res.json())
-        .then(data => {
+        .then(({ data }) => {
           const worker = new FilesDecoratorWorker();
           worker.addEventListener('message', e => {
             const { entries, treeList } = e.data;
diff --git a/spec/javascripts/ide/ide_router_spec.js b/spec/javascripts/ide/ide_router_spec.js
new file mode 100644
index 00000000000..d496048bcb1
--- /dev/null
+++ b/spec/javascripts/ide/ide_router_spec.js
@@ -0,0 +1,150 @@
+import MockAdapter from 'axios-mock-adapter';
+import axios from '~/lib/utils/axios_utils';
+import router from '~/ide/ide_router';
+import store from '~/ide/stores';
+import { resetStore } from './helpers';
+import { project, branch, files } from './mock_data';
+
+describe('IDE router', () => {
+  const originaPathName = location.pathname;
+  let mock;
+
+  beforeEach(() => {
+    spyOn(history, 'pushState');
+    spyOn(store, 'dispatch').and.callThrough();
+
+    mock = new MockAdapter(axios);
+
+    mock.onGet('/api/v4/projects/namespace-123%2Fproject-123').reply(200, project);
+    mock
+      .onGet('/api/v4/projects/namespace-123%2Fproject-123/repository/branches/master')
+      .reply(200, branch);
+    mock.onGet('/namespace-123/project-123/files/master').reply(200, files);
+
+    history.replaceState({}, '', router.options.base);
+  });
+
+  afterEach(done => {
+    mock.restore();
+
+    resetStore(store);
+
+    router.push('/project', done);
+  });
+
+  afterAll(() => {
+    history.replaceState({}, '', originaPathName);
+  });
+
+  describe('project path', () => {
+    it('loads project data', done => {
+      router.push('/project/namespace-123/project-123/', () => {
+        expect(store.dispatch).toHaveBeenCalledWith('getProjectData', {
+          namespace: 'namespace-123',
+          projectId: 'project-123',
+        });
+
+        done();
+      });
+    });
+
+    it('loads project data without trailing slash', done => {
+      router.push('/project/namespace-123/project-123', () => {
+        expect(store.dispatch).toHaveBeenCalledWith('getProjectData', {
+          namespace: 'namespace-123',
+          projectId: 'project-123',
+        });
+
+        done();
+      });
+    });
+  });
+
+  describe('branch data', () => {
+    it('loads branch data', done => {
+      router.push('/project/namespace-123/project-123/edit/master/', () => {
+        expect(store.dispatch.calls.count()).toBe(3);
+        expect(store.dispatch.calls.argsFor(1)).toEqual([
+          'getBranchData',
+          {
+            projectId: 'namespace-123/project-123',
+            branchId: 'master',
+          },
+        ]);
+
+        done();
+      });
+    });
+
+    it('loads branch data without trailing slash', done => {
+      router.push('/project/namespace-123/project-123/edit/master', () => {
+        expect(store.dispatch.calls.count()).toBe(3);
+        expect(store.dispatch.calls.argsFor(1)).toEqual([
+          'getBranchData',
+          {
+            projectId: 'namespace-123/project-123',
+            branchId: 'master',
+          },
+        ]);
+
+        done();
+      });
+    });
+
+    it('loads files for branch', done => {
+      router.push('/project/namespace-123/project-123/edit/master/', () => {
+        expect(store.dispatch.calls.argsFor(2)).toEqual([
+          'getFiles',
+          {
+            projectId: 'namespace-123/project-123',
+            branchId: 'master',
+          },
+        ]);
+
+        done();
+      });
+    });
+  });
+
+  describe('setting folder open', () => {
+    it('calls handleTreeEntryAction with folder', done => {
+      router.push('/project/namespace-123/project-123/edit/master/folder', () => {
+        expect(store.dispatch.calls.argsFor(3)).toEqual([
+          'handleTreeEntryAction',
+          jasmine.anything(),
+        ]);
+        expect(store.dispatch.calls.argsFor(3)[1].path).toBe('folder');
+
+        done();
+      });
+    });
+
+    it('calls handleTreeEntryAction with folder with trailing slash', done => {
+      router.push('/project/namespace-123/project-123/edit/master/folder/', () => {
+        expect(store.dispatch.calls.argsFor(3)).toEqual([
+          'handleTreeEntryAction',
+          jasmine.anything(),
+        ]);
+        expect(store.dispatch.calls.argsFor(3)[1].path).toBe('folder');
+
+        done();
+      });
+    });
+
+    it('does not call handleTreeEntryAction when file is pending', done => {
+      router.push('/project/namespace-123/project-123/edit/master/folder', () => {
+        store.dispatch.calls.reset();
+        store.state.entries['folder/index.js'].pending = true;
+
+        router.push('/project/namespace-123/project-123/edit/master/folder/index.js', () => {
+          expect(store.dispatch.calls.argsFor(3)).not.toEqual([
+            'handleTreeEntryAction',
+            jasmine.anything(),
+          ]);
+
+          done();
+        });
+      });
+    });
+  });
+});
diff --git a/spec/javascripts/ide/mock_data.js b/spec/javascripts/ide/mock_data.js
new file mode 100644
index 00000000000..9303bec6757
--- /dev/null
+++ b/spec/javascripts/ide/mock_data.js
@@ -0,0 +1,11 @@
+export const project = {
+  web_url: '/namespace-123/project-123',
+};
+
+export const branch = {
+  commit: {
+    id: '123',
+  },
+};
+
+export const files = ['folder/index.js'];
diff --git a/spec/javascripts/test_bundle.js b/spec/javascripts/test_bundle.js
index 1bcfdfe72b6..26892b5cec0 100644
--- a/spec/javascripts/test_bundle.js
+++ b/spec/javascripts/test_bundle.js
@@ -19,7 +19,7 @@ Vue.config.warnHandler = (msg, vm, trace) => {
 };
 
 let hasVueErrors = false;
-Vue.config.errorHandler = function (err) {
+Vue.config.errorHandler = function(err) {
   hasVueErrors = true;
   fail(err);
 };
@@ -38,10 +38,12 @@ window.gl = window.gl || {};
 window.gl.TEST_HOST = 'http://test.host';
 window.gon = window.gon || {};
 window.gon.test_env = true;
+window.gon.relative_url_root = '';
+gon.api_version = 'v4';
 
 let hasUnhandledPromiseRejections = false;
 
-window.addEventListener('unhandledrejection', (event) => {
+window.addEventListener('unhandledrejection', event => {
   hasUnhandledPromiseRejections = true;
   console.error('Unhandled promise rejection:');
   console.error(event.reason.stack || event.reason);
@@ -66,13 +68,13 @@ const axiosDefaultAdapter = getDefaultAdapter();
 
 // render all of our tests
 const testsContext = require.context('.', true, /_spec$/);
-testsContext.keys().forEach(function (path) {
+testsContext.keys().forEach(function(path) {
   try {
     testsContext(path);
   } catch (err) {
     console.error('[ERROR] Unable to load spec: ', path);
-    describe('Test bundle', function () {
-      it(`includes '${path}'`, function () {
+    describe('Test bundle', function() {
+      it(`includes '${path}'`, function() {
         expect(err).toBeNull();
       });
     });
@@ -80,7 +82,7 @@ testsContext.keys().forEach(function (path) {
 });
 
 describe('test errors', () => {
-  beforeAll((done) => {
+  beforeAll(done => {
     if (hasUnhandledPromiseRejections || hasVueWarnings || hasVueErrors) {
       setTimeout(done, 1000);
     } else {
@@ -144,18 +146,18 @@ if (process.env.BABEL_ENV === 'coverage') {
     './issue_show/index.js',
   ];
 
-  describe('Uncovered files', function () {
+  describe('Uncovered files', function() {
     const sourceFiles = require.context('~', true, /\.js$/);
 
     $.holdReady(true);
 
-    sourceFiles.keys().forEach(function (path) {
+    sourceFiles.keys().forEach(function(path) {
       // ignore if there is a matching spec file
       if (testsContext.keys().indexOf(`${path.replace(/\.js$/, '')}_spec`) > -1) {
         return;
       }
 
-      it(`includes '${path}'`, function () {
+      it(`includes '${path}'`, function() {
         try {
           sourceFiles(path);
         } catch (err) {
-- 
cgit v1.2.1