From fe82c7ad7c03126c1985144c1ba6a63e9ce5b1fc Mon Sep 17 00:00:00 2001
From: Rhys Arkins <rhys@keylocation.sg>
Date: Wed, 28 Jun 2017 19:37:08 +0200
Subject: [PATCH] Skip repositories with no package.json (#378)

* Skip repositories with no package.json

Closes #376

* Fix await

* Refactor file file check

* Update tests
---
 lib/api/github.js                          |  5 ---
 lib/api/gitlab.js                          |  4 +-
 lib/workers/repository/index.js            | 23 +++++++++--
 test/api/__snapshots__/github.spec.js.snap |  2 +-
 test/api/github.spec.js                    |  4 +-
 test/api/gitlab.spec.js                    |  4 +-
 test/workers/repository/index.spec.js      | 48 ++++++++++++++--------
 7 files changed, 57 insertions(+), 33 deletions(-)

diff --git a/lib/api/github.js b/lib/api/github.js
index 8bb9d241ed..3596cfb3f1 100644
--- a/lib/api/github.js
+++ b/lib/api/github.js
@@ -169,11 +169,6 @@ async function findFilePaths(fileName) {
     `search/code?q=repo:${config.repoName}+filename:${fileName}`
   );
   const exactMatches = res.body.items.filter(item => item.name === fileName);
-  if (exactMatches.length === 0) {
-    // This may happen if the repository is a fork, as forks are not indexed unless they have more stars than the original repo
-    logger.warn('Could not find any package.json files');
-    return ['package.json'];
-  }
   // GitHub seems to return files in the root with a leading `/`
   // which then breaks things later on down the line
   return exactMatches.map(item => item.path.replace(/^\//, ''));
diff --git a/lib/api/gitlab.js b/lib/api/gitlab.js
index aae1e79ef4..f4bc93052a 100644
--- a/lib/api/gitlab.js
+++ b/lib/api/gitlab.js
@@ -110,9 +110,9 @@ async function initRepo(repoName, token, endpoint, repoLogger) {
 // Search
 
 // Returns an array of file paths in current repo matching the fileName
-async function findFilePaths(fileName) {
+async function findFilePaths() {
   logger.debug("Can't find multiple package.json files in GitLab");
-  return [fileName];
+  return [];
 }
 
 // Branch
diff --git a/lib/workers/repository/index.js b/lib/workers/repository/index.js
index e9644b614e..d79901095c 100644
--- a/lib/workers/repository/index.js
+++ b/lib/workers/repository/index.js
@@ -15,13 +15,28 @@ async function renovateRepository(packageFileConfig) {
   try {
     config = await apis.initApis(config);
     config = await apis.mergeRenovateJson(config);
+    if (config.packageFiles.length === 0) {
+      config.logger.debug('Detecting package files');
+      config = await apis.detectPackageFiles(config);
+      if (config.packageFiles.length === 0) {
+        if (!config.hasRenovateJson) {
+          config.logger.debug('Checking if repository has a package.json');
+          const pJson = await config.api.getFileJson('package.json');
+          if (!pJson) {
+            config.logger.info('Repository has no package.json');
+            return;
+          }
+        }
+        config.packageFiles.push('package.json');
+      }
+    }
     config.repoIsOnboarded = await onboarding.getOnboardingStatus(config);
     if (!config.repoIsOnboarded) {
+      const packageFiles = config.packageFiles;
       config = await apis.mergeRenovateJson(config, 'renovate/configure');
-    }
-    const hasConfiguredPackageFiles = config.packageFiles.length > 0;
-    if (!hasConfiguredPackageFiles) {
-      config = await apis.detectPackageFiles(config);
+      if (config.packageFiles.length === 0) {
+        config.packageFiles = packageFiles;
+      }
     }
     const allUpgrades = await upgrades.determineRepoUpgrades(config);
     const branchUpgrades = await upgrades.groupUpgradesByBranch(
diff --git a/test/api/__snapshots__/github.spec.js.snap b/test/api/__snapshots__/github.spec.js.snap
index 44cbd4da0d..00c4200ef8 100644
--- a/test/api/__snapshots__/github.spec.js.snap
+++ b/test/api/__snapshots__/github.spec.js.snap
@@ -316,7 +316,7 @@ Array [
 ]
 `;
 
-exports[`api/github findFilePaths(fileName) should return default value if none found 1`] = `
+exports[`api/github findFilePaths(fileName) should return empty array if none found 1`] = `
 Array [
   Array [
     "repos/some/repo",
diff --git a/test/api/github.spec.js b/test/api/github.spec.js
index 4c88e44931..82ffce64f1 100644
--- a/test/api/github.spec.js
+++ b/test/api/github.spec.js
@@ -298,7 +298,7 @@ describe('api/github', () => {
     });
   });
   describe('findFilePaths(fileName)', () => {
-    it('should return default value if none found', async () => {
+    it('should return empty array if none found', async () => {
       await initRepo('some/repo', 'token');
       ghGot.mockImplementationOnce(() => ({
         body: {
@@ -307,7 +307,7 @@ describe('api/github', () => {
       }));
       const files = await github.findFilePaths('package.json');
       expect(ghGot.mock.calls).toMatchSnapshot();
-      expect(files).toMatchObject(['package.json']);
+      expect(files.length).toBe(0);
     });
     it('should return the files matching the fileName', async () => {
       await initRepo('some/repo', 'token');
diff --git a/test/api/gitlab.spec.js b/test/api/gitlab.spec.js
index f8091cfc42..e3142fe5a5 100644
--- a/test/api/gitlab.spec.js
+++ b/test/api/gitlab.spec.js
@@ -176,10 +176,10 @@ describe('api/gitlab', () => {
     });
   });
   describe('findFilePaths(fileName)', () => {
-    it('should return the fileName', async () => {
+    it('should return empty array', async () => {
       await initRepo('some/repo', 'token');
       const files = await gitlab.findFilePaths('package.json');
-      expect(files).toEqual(['package.json']);
+      expect(files.length).toBe(0);
     });
   });
   describe('branchExists(branchName)', () => {
diff --git a/test/workers/repository/index.spec.js b/test/workers/repository/index.spec.js
index 130db81f97..bc021750c0 100644
--- a/test/workers/repository/index.spec.js
+++ b/test/workers/repository/index.spec.js
@@ -9,7 +9,7 @@ const logger = require('../../_fixtures/logger');
 
 apis.initApis = jest.fn(input => input);
 apis.mergeRenovateJson = jest.fn(input => input);
-apis.detectPackageFiles = jest.fn(input => input);
+apis.detectPackageFiles = jest.fn();
 
 describe('workers/repository', () => {
   describe('renovateRepository', () => {
@@ -17,22 +17,45 @@ describe('workers/repository', () => {
     beforeEach(() => {
       onboarding.getOnboardingStatus = jest.fn();
       onboarding.ensurePr = jest.fn();
-      upgrades.groupUpgradesByBranch = jest.fn();
+      upgrades.determineRepoUpgrades = jest.fn(() => []);
+      upgrades.groupUpgradesByBranch = jest.fn(() => ({}));
       branchWorker.updateBranch = jest.fn();
       config = {
+        api: {
+          getFileJson: jest.fn(),
+        },
         logger,
         packageFiles: [],
       };
     });
-    it('returns early if repo is not onboarded', async () => {
-      onboarding.getOnboardingStatus.mockReturnValueOnce(false);
+    it('skips repository if no package.json', async () => {
+      apis.detectPackageFiles.mockImplementationOnce(input =>
+        Object.assign(input, { packageFiles: [] })
+      );
       await repositoryWorker.renovateRepository(config);
+      expect(onboarding.getOnboardingStatus.mock.calls.length).toBe(0);
+      expect(config.logger.error.mock.calls.length).toBe(0);
     });
-    it('detects package files if none configured', async () => {
-      onboarding.getOnboardingStatus.mockReturnValueOnce(true);
+    it('does not skip repository if package.json', async () => {
+      apis.detectPackageFiles.mockImplementationOnce(input =>
+        Object.assign(input, { packageFiles: [] })
+      );
+      config.api.getFileJson = jest.fn(() => ({ a: 1 }));
+      apis.mergeRenovateJson.mockImplementationOnce(input =>
+        Object.assign(input, { packageFiles: [] })
+      );
+      apis.mergeRenovateJson.mockImplementationOnce(input =>
+        Object.assign(input, { packageFiles: [] })
+      );
       await repositoryWorker.renovateRepository(config);
+      expect(onboarding.getOnboardingStatus.mock.calls.length).toBe(1);
+      expect(branchWorker.updateBranch.mock.calls.length).toBe(0);
+      expect(onboarding.ensurePr.mock.calls.length).toBe(1);
+      expect(config.logger.error.mock.calls.length).toBe(0);
     });
     it('calls branchWorker', async () => {
+      config.packageFiles = ['package.json'];
+      config.hasRenovateJson = true;
       onboarding.getOnboardingStatus.mockReturnValueOnce(true);
       upgrades.groupUpgradesByBranch.mockReturnValueOnce({
         foo: {},
@@ -41,23 +64,14 @@ describe('workers/repository', () => {
       });
       await repositoryWorker.renovateRepository(config);
       expect(branchWorker.updateBranch.mock.calls.length).toBe(3);
-    });
-    it('calls ensurePr', async () => {
-      onboarding.getOnboardingStatus.mockReturnValueOnce(false);
-      upgrades.groupUpgradesByBranch.mockReturnValueOnce({
-        foo: {},
-        bar: {},
-        baz: {},
-      });
-      await repositoryWorker.renovateRepository(config);
-      expect(branchWorker.updateBranch.mock.calls.length).toBe(0);
-      expect(onboarding.ensurePr.mock.calls.length).toBe(1);
+      expect(config.logger.error.mock.calls.length).toBe(0);
     });
     it('swallows errors', async () => {
       apis.initApis.mockImplementationOnce(() => {
         throw new Error('bad init');
       });
       await repositoryWorker.renovateRepository(config);
+      expect(config.logger.error.mock.calls.length).toBe(1);
     });
   });
 });
-- 
GitLab