Rename some configuration parameters for the action (#7)

s/fail_if_missing_approving_reviews/fail_if_cannot_be_merged
s/fail_if_no_area_label/require_area_label
s/fail_if_not_enough_available_approvers_for_area/require_enough_available_approvers_for_area

Also add a couple of extra unit tests.

Signed-off-by: Antonin Bas <[email protected]>

action.yml

@@ -15,16 +15,16 @@ inputs:
     description: 'Repository file which includes the owners for each area label'
     required: true
     default: '.github/AREA-OWNERS'
-  fail_if_missing_approving_reviews:
-    description: 'Fail action if there are not enough approving reviews'
+  fail_if_cannot_be_merged:
+    description: 'Fail action if the PR cannot be merged (e.g. not enough approving reviews)'
     required: false
     default: false
   label_on_success:
-    description: 'Label the PR with this label if there are enough approving reviews'
+    description: 'Label the PR with this label on success, i.e. if it can be merged (enough approving reviews)'
     required: false
     default: ''
-  fail_if_no_area_label:
-    description: 'If the PR is not labelled with any label from the ownership file, fail'
+  require_area_label:
+    description: 'Require that each PR be labelled with at last one label from the ownership file'
     required: false
     default: true
   succeed_if_maintainer_approves:
@@ -39,8 +39,8 @@ inputs:
     description: 'Ignore all PRs not labelled with this label'
     required: false
     default: ''
-  fail_if_not_enough_available_approvers_for_area:
-    description: 'Fail if there are not enough available reviewers (< min_approving_reviews_per_area) defined for a given area'
+  require_enough_available_approvers_for_area:
+    description: 'There must be enough available reviewers (< min_approving_reviews_per_area) defined each area label'
     required: false
     default: false
   support_label_regex:

approval.js

@@ -73,7 +73,7 @@ let canBeMerged = function(labels, author, approvals, config) {
         result = false;
     }
 
-    if (approvalsByArea.size < 1 && config.failIfNoAreaLabel) {
+    if (approvalsByArea.size < 1 && config.requireAreaLabel) {
         console.log(`At least one area label is required for a pull request`);
         result = false;
     }

approval.test.js

@@ -9,7 +9,7 @@ describe('PR can be merged', () => {
     let approvers;
     let minApprovingReviewsTotal;
     let minApprovingReviewsPerArea;
-    let failIfNoAreaLabel;
+    let requireAreaLabel;
     let succeedIfMaintainerApproves;
     let failIfNotEnoughAvailableApproversPerArea;
     let requestReviewsFromMaintainersIfNeeded;
@@ -26,7 +26,7 @@ describe('PR can be merged', () => {
             areaApprovers: areaApprovers,
             areaReviewersRegexList: owners.buildRegexList(areaReviewers),
             areaApproversRegexList: owners.buildRegexList(areaApprovers),
-            failIfNoAreaLabel: failIfNoAreaLabel,
+            requireAreaLabel: requireAreaLabel,
             succeedIfMaintainerApproves: succeedIfMaintainerApproves,
             failIfNotEnoughAvailableApproversPerArea: failIfNotEnoughAvailableApproversPerArea,
             requestReviewsFromMaintainersIfNeeded: requestReviewsFromMaintainersIfNeeded,
@@ -49,7 +49,7 @@ describe('PR can be merged', () => {
         maintainers = [];
         minApprovingReviewsTotal = 2;
         minApprovingReviewsPerArea = 1;
-        failIfNoAreaLabel = true;
+        requireAreaLabel = true;
         succeedIfMaintainerApproves = false;
         requestReviewsFromMaintainersIfNeeded = false;
         failIfNotEnoughAvailableApproversPerArea = false;
@@ -80,7 +80,7 @@ describe('PR can be merged', () => {
 
     test('accept no area label', async () => {
         labels = [];
-        failIfNoAreaLabel = false;
+        requireAreaLabel = false;
         const approvals = new Set(['alice', 'joe']);
         expect(canBeMerged(approvals)).toBeTruthy();
     });

run.js

@@ -34,13 +34,13 @@ let getConfig = function() {
         areaApprovers: areaApprovers,
         areaReviewersRegexList: areaReviewersRegexList,
         areaApproversRegexList: areaApproversRegexList,
-        failIfMissingApprovingReviews: core.getInput('fail_if_missing_approving_reviews'),
+        failIfCannotBeMerged: core.getInput('fail_if_cannot_be_merged'),
         labelOnSuccess: core.getInput('label_on_success'),
-        failIfNoAreaLabel: core.getInput('fail_if_no_area_label'),
+        requireAreaLabel: core.getInput('require_area_label'),
         succeedIfMaintainerApproves: core.getInput('succeed_if_maintainer_approves'),
         requestReviewsFromMaintainersIfNeeded: core.getInput('request_reviews_from_maintainers_if_needed'),
         ignoreIfNotLabelledWith: core.getInput('ignore_if_not_labelled_with'),
-        failIfNotEnoughAvailableApproversPerArea: core.getInput('fail_if_not_enough_available_approvers_for_area'),
+        failIfNotEnoughAvailableApproversPerArea: core.getInput('require_enough_available_approvers_for_area'),
         maintainersAreUniversalApprovers: core.getInput('maintainers_are_universal_approvers'),
    };
 };
@@ -108,7 +108,7 @@ async function run() {
             }
         }
 
-        if (!canBeMerged && config.failIfMissingApprovingReviews) {
+        if (!canBeMerged && config.failIfCannotBeMerged) {
             core.setFailed(`Not enough approving reviews for PR`);
         }
     } catch (error) {

run.test.js

@@ -58,9 +58,9 @@ describe('run', () => {
         });
     });
 
-    function setContextPayload(labels, author) {
+    function setContextPayload(labels, author, draft = false) {
         pullRequest = {
-            draft: false,
+            draft: draft,
             number: pullRequestNumber,
             user: {
                 login: author,
@@ -80,9 +80,9 @@ describe('run', () => {
             min_approving_reviews_total: 2,
             min_approving_reviews_per_area: 1,
             area_ownership_file: 'testdata/owners.yml',
-            fail_if_missing_approving_reviews: true,
+            fail_if_cannot_be_merged: true,
             label_on_success: successLabel,
-            fail_if_no_area_label: true,
+            require_area_label: true,
             succeed_if_maintainer_approves: false,
             request_reviews_from_maintainers_if_neede: true,
             ignore_if_not_labelled_with: '',
@@ -125,6 +125,7 @@ describe('run', () => {
             labels: [successLabel],
         }]);
         expect(mockRemoveLabel.mock.calls.length).toBe(0);
+        expect(core.setFailed.mock.calls.length).toBe(0);
     });
 
     test('remove success label', async () => {
@@ -143,5 +144,25 @@ describe('run', () => {
             issue_number: pullRequestNumber,
             label: successLabel,
         }]);
+        expect(core.setFailed.mock.calls.length).toBe(1);
+    });
+
+    test('missing area label', async () => {
+        setContextPayload(labels, author);
+        await run();
+        expect(core.setFailed.mock.calls.length).toBe(1);
+    });
+
+    test('ignore if draft', async () => {
+        setContextPayload(labels, author, true);
+        await run();
+        expect(core.setFailed.mock.calls.length).toBe(0);
+    });
+
+    test('ignore if not labelled with', async () => {
+        setContextPayload(labels, author);
+        inputs.ignore_if_not_labelled_with = 'review-manager';
+        await run();
+        expect(core.setFailed.mock.calls.length).toBe(0);
     });
 });