From d237c6c6702990b5b80eddb2608cd5a20e099eef Mon Sep 17 00:00:00 2001
From: Rhys Arkins <rhys@arkins.net>
Date: Tue, 6 Mar 2018 20:00:10 +0100
Subject: [PATCH] feat: add comment to PRs if config validation fails

---
 lib/workers/repository/validate.js       | 18 +++++++++++++++---
 test/workers/repository/validate.spec.js | 10 ++++++++++
 2 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/lib/workers/repository/validate.js b/lib/workers/repository/validate.js
index 0b2d7dbf8b..b3dedb5616 100644
--- a/lib/workers/repository/validate.js
+++ b/lib/workers/repository/validate.js
@@ -38,7 +38,10 @@ async function validatePrs(config) {
         try {
           parsed = JSON.parse(content);
         } catch (err) {
-          validations.push('Cannot parse ' + file);
+          validations.push({
+            file,
+            message: 'Invalid JSON',
+          });
         }
         if (parsed) {
           const { errors } = migrateAndValidate(
@@ -49,7 +52,10 @@ async function validatePrs(config) {
           );
           if (errors && errors.length) {
             validations = validations.concat(
-              errors.map(error => error.message)
+              errors.map(error => ({
+                file,
+                message: error.message,
+              }))
             );
           }
         }
@@ -57,12 +63,18 @@ async function validatePrs(config) {
       // if the PR has renovate files then we set a status no matter what
       let status;
       let description;
+      const subject = 'Renovate Configuration Errors';
       if (validations.length) {
-        description = validations.join(', ').substring(0, 140); // GitHub limit
+        const content = validations
+          .map(v => `\`${v.file}\`: ${v.message}`)
+          .join('\n\n');
+        await platform.ensureComment(pr.number, subject, content);
         status = 'failure';
+        description = 'Renovate config validation failed'; // GitHub limit
       } else {
         description = 'Renovate config is valid';
         status = 'success';
+        await platform.ensureCommentRemoval(pr.number, subject);
       }
       logger.info({ status, description }, 'Setting PR validation status');
       const context = 'renovate/validate';
diff --git a/test/workers/repository/validate.spec.js b/test/workers/repository/validate.spec.js
index 320896b43c..4345d7d8df 100644
--- a/test/workers/repository/validate.spec.js
+++ b/test/workers/repository/validate.spec.js
@@ -16,6 +16,8 @@ describe('workers/repository/validate', () => {
       ]);
       await validate.validatePrs({});
       expect(platform.setBranchStatus.mock.calls).toHaveLength(0);
+      expect(platform.ensureComment.mock.calls).toHaveLength(0);
+      expect(platform.ensureCommentRemoval.mock.calls).toHaveLength(0);
     });
     it('returns if no matching files', async () => {
       platform.getPrList.mockReturnValueOnce([
@@ -28,6 +30,8 @@ describe('workers/repository/validate', () => {
       platform.getPrFiles.mockReturnValueOnce(['readme.md']);
       await validate.validatePrs({});
       expect(platform.setBranchStatus.mock.calls).toHaveLength(0);
+      expect(platform.ensureComment.mock.calls).toHaveLength(0);
+      expect(platform.ensureCommentRemoval.mock.calls).toHaveLength(0);
     });
     it('validates failures if cannot parse', async () => {
       platform.getPrList.mockReturnValueOnce([
@@ -42,6 +46,8 @@ describe('workers/repository/validate', () => {
       await validate.validatePrs({});
       expect(platform.setBranchStatus.mock.calls).toHaveLength(1);
       expect(platform.setBranchStatus.mock.calls[0][3]).toEqual('failure');
+      expect(platform.ensureComment.mock.calls).toHaveLength(1);
+      expect(platform.ensureCommentRemoval.mock.calls).toHaveLength(0);
     });
     it('validates failures if config validation fails', async () => {
       platform.getPrList.mockReturnValueOnce([
@@ -56,6 +62,8 @@ describe('workers/repository/validate', () => {
       await validate.validatePrs({});
       expect(platform.setBranchStatus.mock.calls).toHaveLength(1);
       expect(platform.setBranchStatus.mock.calls[0][3]).toEqual('failure');
+      expect(platform.ensureComment.mock.calls).toHaveLength(1);
+      expect(platform.ensureCommentRemoval.mock.calls).toHaveLength(0);
     });
     it('validates successfully', async () => {
       platform.getPrList.mockReturnValueOnce([
@@ -70,6 +78,8 @@ describe('workers/repository/validate', () => {
       await validate.validatePrs({});
       expect(platform.setBranchStatus.mock.calls).toHaveLength(1);
       expect(platform.setBranchStatus.mock.calls[0][3]).toEqual('success');
+      expect(platform.ensureComment.mock.calls).toHaveLength(0);
+      expect(platform.ensureCommentRemoval.mock.calls).toHaveLength(1);
     });
   });
 });
-- 
GitLab