test: fix test failures caused by conflicts with main

Author: kriswest

src/proxy/processors/push-action/checkUserPushPermission.ts

@@ -17,7 +17,17 @@ const exec = async (req: any, action: Action): Promise<Action> => {
   const list = await getUsers({ email: action.userEmail });
 
   if (list.length > 1) {
-    console.warn(`Multiple users found with email address ${userEmail}, using the first only`);
+    console.error(`Multiple users found with email address ${userEmail}, ending`);
+    step.error = true;
+    step.log(
+      `Multiple Users have email <${userEmail}> so we cannot uniquely identify the user, ending`,
+    );
+
+    step.setError(
+      `Your push has been blocked (there are multiple users with email ${action.userEmail})`,
+    );
+    action.addStep(step);
+    return action;
   } else if (list.length == 0) {
     console.error(`No user with email address ${userEmail} found`);
   } else {
@@ -34,8 +44,6 @@ const exec = async (req: any, action: Action): Promise<Action> => {
       `User ${username} <${userEmail}> is not allowed to push on repo ${action.repo}, ending`,
     );
 
-    console.log('setting error');
-
     step.setError(
       `Your push has been blocked (${action.userEmail} ` +
         `is not allowed to push on repo ` +

test/ConfigLoader.test.js

@@ -38,6 +38,10 @@ describe('ConfigLoader', () => {
     });
   });
 
+  after(() => {
+    // restore default config
+  });
+
   describe('loadFromFile', () => {
     it('should load configuration from file', async () => {
       const testConfig = {
@@ -251,7 +255,6 @@ describe('ConfigLoader', () => {
       configLoader = new ConfigLoader(mockConfig);
       configLoader.reloadTimer = setInterval(() => {}, 1000);
       await configLoader.start();
-
       expect(configLoader.reloadTimer).to.be.null;
     });
 
@@ -297,7 +300,6 @@ describe('ConfigLoader', () => {
       configLoader = new ConfigLoader(mockConfig);
       configLoader.reloadTimer = setInterval(() => {}, 1000);
       expect(configLoader.reloadTimer).to.not.be.null;
-
       await configLoader.stop();
       expect(configLoader.reloadTimer).to.be.null;
     });

test/processors/checkUserPushPermission.test.js

@@ -18,12 +18,15 @@ describe('checkUserPushPermission', () => {
     getUsersStub = sinon.stub();
     isUserPushAllowedStub = sinon.stub();
 
-    const checkUserPushPermission = proxyquire('../../src/proxy/processors/push-action/checkUserPushPermission', {
-      '../../../db': {
-        getUsers: getUsersStub,
-        isUserPushAllowed: isUserPushAllowedStub
-      }
-    });
+    const checkUserPushPermission = proxyquire(
+      '../../src/proxy/processors/push-action/checkUserPushPermission',
+      {
+        '../../../db': {
+          getUsers: getUsersStub,
+          isUserPushAllowed: isUserPushAllowedStub,
+        },
+      },
+    );
 
     exec = checkUserPushPermission.exec;
   });
@@ -39,40 +42,43 @@ describe('checkUserPushPermission', () => {
 
     beforeEach(() => {
       req = {};
-      action = new Action(
-        '1234567890',
-        'push',
-        'POST',
-        1234567890,
-        'test/repo.git'
-      );
+      action = new Action('1234567890', 'push', 'POST', 1234567890, 'test/repo.git');
       action.user = 'git-user';
+      action.userEmail = '[email protected]';
       stepSpy = sinon.spy(Step.prototype, 'log');
     });
 
     it('should allow push when user has permission', async () => {
-      getUsersStub.resolves([{ username: 'db-user', gitAccount: 'git-user' }]);
+      getUsersStub.resolves([
+        { username: 'db-user', email: '[email protected]', gitAccount: 'git-user' },
+      ]);
       isUserPushAllowedStub.resolves(true);
 
       const result = await exec(req, action);
 
       expect(result.steps).to.have.lengthOf(1);
       expect(result.steps[0].error).to.be.false;
-      expect(stepSpy.calledWith('User db-user is allowed to push on repo test/repo.git')).to.be.true;
-      expect(logStub.calledWith('User db-user permission on Repo repo : true')).to.be.true;
+      expect(stepSpy.lastCall.args[0]).to.equal(
+        'User db-user <[email protected]> is allowed to push on repo test/repo.git',
+      );
+      expect(logStub.lastCall.args[0]).to.equal(
+        'User db-user <[email protected]> permission on Repo repo : true',
+      );
     });
 
     it('should reject push when user has no permission', async () => {
-      getUsersStub.resolves([{ username: 'db-user', gitAccount: 'git-user' }]);
+      getUsersStub.resolves([
+        { username: 'db-user', email: '[email protected]', gitAccount: 'git-user' },
+      ]);
       isUserPushAllowedStub.resolves(false);
 
       const result = await exec(req, action);
 
       expect(result.steps).to.have.lengthOf(1);
       expect(result.steps[0].error).to.be.true;
-      expect(stepSpy.calledWith('User db-user is not allowed to push on repo test/repo.git, ending')).to.be.true;
-      expect(result.steps[0].errorMessage).to.include('Rejecting push as user git-user');
-      expect(logStub.calledWith('User not allowed to Push')).to.be.true;
+      expect(result.steps[0].errorMessage).to.equal(
+        'Your push has been blocked ([email protected] is not allowed to push on repo test/repo.git)',
+      );
     });
 
     it('should reject push when no user found for git account', async () => {
@@ -82,21 +88,22 @@ describe('checkUserPushPermission', () => {
 
       expect(result.steps).to.have.lengthOf(1);
       expect(result.steps[0].error).to.be.true;
-      expect(stepSpy.calledWith('User git-user is not allowed to push on repo test/repo.git, ending')).to.be.true;
-      expect(result.steps[0].errorMessage).to.include('Rejecting push as user git-user');
+      expect(result.steps[0].errorMessage).to.include('Your push has been blocked');
     });
 
-    it('should handle multiple users for git account by rejecting push', async () => {
+    it('should handle multiple users for git account by rejecting the push', async () => {
       getUsersStub.resolves([
-        { username: 'user1', gitAccount: 'git-user' },
-        { username: 'user2', gitAccount: 'git-user' }
+        { username: 'user1', email: '[email protected]', gitAccount: 'git-user' },
+        { username: 'user2', email: '[email protected]', gitAccount: 'git-user' },
       ]);
 
       const result = await exec(req, action);
 
       expect(result.steps).to.have.lengthOf(1);
       expect(result.steps[0].error).to.be.true;
-      expect(logStub.calledWith('Users for this git account: [{"username":"user1","gitAccount":"git-user"},{"username":"user2","gitAccount":"git-user"}]')).to.be.true;
+      expect(result.steps[0].errorMessage).to.equal(
+        'Your push has been blocked (there are multiple users with email [email protected])',
+      );
     });
   });
 });

test/testDb.test.js

@@ -86,13 +86,6 @@ describe('Database clients', async () => {
     expect(cleanRepos).to.deep.include(TEST_REPO);
   });
 
-  it('should be able to delete a repo', async function () {
-    await db.deleteRepo(TEST_REPO.name);
-    const repos = await db.getRepos();
-    const cleanRepos = cleanResponseData(TEST_REPO, repos);
-    expect(cleanRepos).to.not.deep.include(TEST_REPO);
-  });
-
   it('should be able to filter repos', async function () {
     // uppercase the filter value to confirm db client is lowercasing inputs
     const repos = await db.getRepos({ name: TEST_REPO.name.toUpperCase() });
@@ -116,6 +109,13 @@ describe('Database clients', async () => {
     expect(cleanRepo).to.eql(TEST_REPO);
   });
 
+  it('should be able to delete a repo', async function () {
+    await db.deleteRepo(TEST_REPO.name);
+    const repos = await db.getRepos();
+    const cleanRepos = cleanResponseData(TEST_REPO, repos);
+    expect(cleanRepos).to.not.deep.include(TEST_REPO);
+  });
+
   it('should NOT be able to create a repo with blank project, name or url', async function () {
     // test with a null value
     let threwError = false;