From 9769f5a5dbd7ba4d75793c5797372d9aee45fc0e Mon Sep 17 00:00:00 2001
From: Rhys Arkins <rhys@keylocation.sg>
Date: Fri, 3 Nov 2017 09:25:18 +0100
Subject: [PATCH]  refactor: detectPackageFiles returns packageFiles not config
 (#1092)

---
 lib/manager/index.js                          | 18 ++++-----
 lib/workers/repository/index.js               |  2 +-
 lib/workers/repository/onboarding.js          |  4 +-
 test/manager/__snapshots__/index.spec.js.snap |  3 +-
 test/manager/index.spec.js                    | 21 +++++-----
 test/workers/repository/index.spec.js         | 40 ++++---------------
 test/workers/repository/onboarding.spec.js    |  4 +-
 7 files changed, 30 insertions(+), 62 deletions(-)

diff --git a/lib/manager/index.js b/lib/manager/index.js
index 091c93d46d..b087bb873e 100644
--- a/lib/manager/index.js
+++ b/lib/manager/index.js
@@ -17,36 +17,32 @@ module.exports = {
   getUpdatedPackageFiles,
 };
 
-async function detectPackageFiles(input) {
-  const config = { ...input };
+async function detectPackageFiles(config) {
   const { logger } = config;
+  let packageFiles = [];
   const fileList = (await config.api.getFileList()).filter(
     file =>
       !config.ignorePaths.some(
         ignorePath => file.includes(ignorePath) || minimatch(file, ignorePath)
       )
   );
-  logger.debug({ config }, 'detectPackageFiles');
-  config.types = {};
+  logger.trace({ config }, 'detectPackageFiles');
   const packageJsonFiles = await npmDetect.detectPackageFiles(config, fileList);
   if (packageJsonFiles.length) {
     logger.info({ packageJsonFiles }, 'Found package.json files');
-    config.packageFiles = config.packageFiles.concat(packageJsonFiles);
-    config.types.npm = true;
+    packageFiles = packageFiles.concat(packageJsonFiles);
   }
   const meteorFiles = await meteorDetect.detectPackageFiles(config, fileList);
   if (meteorFiles.length) {
     logger.info({ packageJsonFiles }, 'Found meteor files');
-    config.packageFiles = config.packageFiles.concat(meteorFiles);
-    config.types.meteor = true;
+    packageFiles = packageFiles.concat(meteorFiles);
   }
   const dockerFiles = await dockerDetect.detectPackageFiles(config, fileList);
   if (dockerFiles.length) {
     logger.info({ dockerFiles }, 'Found Dockerfiles');
-    config.packageFiles = config.packageFiles.concat(dockerFiles);
-    config.types.docker = true;
+    packageFiles = packageFiles.concat(dockerFiles);
   }
-  return config;
+  return packageFiles;
 }
 
 async function getPackageUpdates(config) {
diff --git a/lib/workers/repository/index.js b/lib/workers/repository/index.js
index a65e7c28ba..43c1cda55f 100644
--- a/lib/workers/repository/index.js
+++ b/lib/workers/repository/index.js
@@ -75,7 +75,7 @@ async function renovateRepository(repoConfig, token) {
     // Detect package files in default branch if not manually provisioned
     if (config.packageFiles.length === 0) {
       logger.debug('Detecting package files');
-      config = await manager.detectPackageFiles(config);
+      config.packageFiles = await manager.detectPackageFiles(config);
       // If we can't detect any package.json then return
       if (config.packageFiles.length === 0) {
         logger.info('Cannot detect package files');
diff --git a/lib/workers/repository/onboarding.js b/lib/workers/repository/onboarding.js
index f5aa49b55c..24703e45f0 100644
--- a/lib/workers/repository/onboarding.js
+++ b/lib/workers/repository/onboarding.js
@@ -10,10 +10,10 @@ module.exports = {
 };
 
 async function createOnboardingBranch(inputConfig) {
-  let config = { ...inputConfig };
+  const config = { ...inputConfig };
   const { logger } = config;
   logger.debug('Creating onboarding branch');
-  config = await manager.detectPackageFiles(config);
+  config.packageFiles = await manager.detectPackageFiles(config);
   if (config.packageFiles.length === 0) {
     throw new Error('no package files');
   }
diff --git a/test/manager/__snapshots__/index.spec.js.snap b/test/manager/__snapshots__/index.spec.js.snap
index 868dd63ea6..9104f880dd 100644
--- a/test/manager/__snapshots__/index.spec.js.snap
+++ b/test/manager/__snapshots__/index.spec.js.snap
@@ -22,12 +22,11 @@ Array [
 exports[`manager detectPackageFiles(config) ignores node modules 1`] = `
 Array [
   "package.json",
-  "not_node_mldules/backend/package.json",
 ]
 `;
 
 exports[`manager detectPackageFiles(config) ignores node modules 2`] = `undefined`;
 
-exports[`manager detectPackageFiles(config) ignores node modules 3`] = `Array []`;
+exports[`manager detectPackageFiles(config) ignores node modules 3`] = `undefined`;
 
 exports[`manager detectPackageFiles(config) skips meteor package files with no json 1`] = `Array []`;
diff --git a/test/manager/index.spec.js b/test/manager/index.spec.js
index f219d5605c..9b5744ba6f 100644
--- a/test/manager/index.spec.js
+++ b/test/manager/index.spec.js
@@ -28,8 +28,8 @@ describe('manager', () => {
         'backend/package.json',
       ]);
       const res = await manager.detectPackageFiles(config);
-      expect(res.packageFiles).toMatchSnapshot();
-      expect(res.packageFiles).toHaveLength(2);
+      expect(res).toMatchSnapshot();
+      expect(res).toHaveLength(2);
     });
     it('finds meteor package files', async () => {
       config.meteor.enabled = true;
@@ -38,8 +38,8 @@ describe('manager', () => {
       ]); // meteor
       config.api.getFileContent.mockReturnValueOnce('Npm.depends( {} )');
       const res = await manager.detectPackageFiles(config);
-      expect(res.packageFiles).toMatchSnapshot();
-      expect(res.packageFiles).toHaveLength(1);
+      expect(res).toMatchSnapshot();
+      expect(res).toHaveLength(1);
     });
     it('skips meteor package files with no json', async () => {
       config.meteor.enabled = true;
@@ -48,8 +48,8 @@ describe('manager', () => {
       ]); // meteor
       config.api.getFileContent.mockReturnValueOnce('Npm.depends(packages)');
       const res = await manager.detectPackageFiles(config);
-      expect(res.packageFiles).toMatchSnapshot();
-      expect(res.packageFiles).toHaveLength(0);
+      expect(res).toMatchSnapshot();
+      expect(res).toHaveLength(0);
     });
     it('finds Dockerfiles', async () => {
       config.api.getFileList.mockReturnValueOnce([
@@ -63,18 +63,17 @@ describe('manager', () => {
         'ARG foo\nFROM something\nRUN something'
       );
       const res = await manager.detectPackageFiles(config);
-      expect(res.packageFiles).toMatchSnapshot();
-      expect(res.packageFiles).toHaveLength(1);
+      expect(res).toMatchSnapshot();
+      expect(res).toHaveLength(1);
     });
     it('ignores node modules', async () => {
       config.api.getFileList.mockReturnValueOnce([
         'package.json',
         'node_modules/backend/package.json',
-        'not_node_mldules/backend/package.json',
       ]);
       const res = await manager.detectPackageFiles(config);
-      expect(res.packageFiles).toMatchSnapshot();
-      expect(res.packageFiles).toHaveLength(2);
+      expect(res).toMatchSnapshot();
+      expect(res).toHaveLength(1);
       expect(res.foundIgnoredPaths).toMatchSnapshot();
       expect(res.warnings).toMatchSnapshot();
     });
diff --git a/test/workers/repository/index.spec.js b/test/workers/repository/index.spec.js
index abb83c9b69..e5cdff6bb4 100644
--- a/test/workers/repository/index.spec.js
+++ b/test/workers/repository/index.spec.js
@@ -79,46 +79,31 @@ describe('workers/repository', () => {
     it('does not skip repository if its a configured fork', async () => {
       config.isFork = true;
       config.renovateFork = true;
-      manager.detectPackageFiles.mockImplementationOnce(input => ({
-        ...input,
-        ...{ packageFiles: [] },
-      }));
+      manager.detectPackageFiles.mockReturnValueOnce([]);
       await repositoryWorker.renovateRepository(config);
     });
     it('sets custom base branch', async () => {
       config.baseBranch = 'some-branch';
       config.api.branchExists.mockReturnValueOnce(true);
-      manager.detectPackageFiles.mockImplementationOnce(input => ({
-        ...input,
-        ...{ packageFiles: [] },
-      }));
+      manager.detectPackageFiles.mockReturnValueOnce([]);
       await repositoryWorker.renovateRepository(config);
       expect(config.api.setBaseBranch.mock.calls).toHaveLength(1);
     });
     it('errors when missing custom base branch', async () => {
       config.baseBranch = 'some-branch';
       config.api.branchExists.mockReturnValueOnce(false);
-      manager.detectPackageFiles.mockImplementationOnce(input => ({
-        ...input,
-        ...{ packageFiles: [] },
-      }));
+      manager.detectPackageFiles.mockReturnValueOnce([]);
       await repositoryWorker.renovateRepository(config);
       expect(config.api.setBaseBranch.mock.calls).toHaveLength(0);
     });
     it('skips repository if no package.json', async () => {
-      manager.detectPackageFiles.mockImplementationOnce(input => ({
-        ...input,
-        ...{ packageFiles: [] },
-      }));
+      manager.detectPackageFiles.mockReturnValueOnce([]);
       await repositoryWorker.renovateRepository(config);
       expect(apis.resolvePackageFiles.mock.calls.length).toBe(0);
       expect(config.logger.error.mock.calls.length).toBe(0);
     });
     it('does not skip repository if package.json', async () => {
-      manager.detectPackageFiles.mockImplementationOnce(input => ({
-        ...input,
-        ...{ packageFiles: ['package.json'] },
-      }));
+      manager.detectPackageFiles.mockReturnValueOnce(['package.json']);
       config.api.getFileJson = jest.fn(() => ({ a: 1 }));
       apis.mergeRenovateJson.mockImplementationOnce(input => ({
         ...input,
@@ -138,15 +123,9 @@ describe('workers/repository', () => {
       expect(config.logger.error.mock.calls.length).toBe(0);
     });
     it('uses onboarding custom baseBranch', async () => {
-      manager.detectPackageFiles.mockImplementationOnce(input => ({
-        ...input,
-        ...{ packageFiles: ['package.json'] },
-      }));
+      manager.detectPackageFiles.mockReturnValueOnce(['package.json']);
       config.api.getFileJson = jest.fn(() => ({ a: 1 }));
-      apis.mergeRenovateJson.mockImplementationOnce(input => ({
-        ...input,
-        ...{ packageFiles: ['package.json'] },
-      }));
+      manager.detectPackageFiles.mockReturnValueOnce(['package.json']);
       apis.mergeRenovateJson.mockImplementationOnce(input => ({
         ...input,
         ...{ packageFiles: ['package.json'], baseBranch: 'next' },
@@ -162,10 +141,7 @@ describe('workers/repository', () => {
       expect(config.logger.error.mock.calls.length).toBe(0);
     });
     it('errors onboarding custom baseBranch', async () => {
-      manager.detectPackageFiles.mockImplementationOnce(input => ({
-        ...input,
-        ...{ packageFiles: ['package.json'] },
-      }));
+      manager.detectPackageFiles.mockReturnValueOnce(['package.json']);
       config.api.getFileJson = jest.fn(() => ({ a: 1 }));
       apis.mergeRenovateJson.mockImplementationOnce(input => ({
         ...input,
diff --git a/test/workers/repository/onboarding.spec.js b/test/workers/repository/onboarding.spec.js
index 953b53b53e..739219251e 100644
--- a/test/workers/repository/onboarding.spec.js
+++ b/test/workers/repository/onboarding.spec.js
@@ -248,9 +248,7 @@ describe('lib/workers/repository/onboarding', () => {
       expect(config.api.commitFilesToBranch.mock.calls[0]).toMatchSnapshot();
     });
     it('throws if no packageFiles', async () => {
-      manager.detectPackageFiles = jest.fn(input => ({
-        ...input,
-      }));
+      manager.detectPackageFiles = jest.fn(() => []);
       let e;
       try {
         await onboarding.getOnboardingStatus(config);
-- 
GitLab