Merge pull request #50 from bobvanderlinden/pr-fix-rule-results

Fix failing rule results not being passed through

src/pull-request-status.ts

@@ -18,28 +18,47 @@ export function getConditionResults (
   )
 }
 
+export function getFirstSuccessfulResults (ruleConfigs: ConditionConfig[], conditions: Conditions, pullRequestInfo: PullRequestInfo): ConditionResults | undefined {
+  for (let ruleConfig of ruleConfigs) {
+    const ruleResult = getConditionResults(ruleConfig, conditions, pullRequestInfo)
+    if (areSuccessfulResults(ruleResult)) {
+      return ruleResult
+    }
+  }
+  return undefined
+}
+
 export function getPullRequestStatus (
   context: HandlerContext,
   conditions: Conditions,
   pullRequestInfo: PullRequestInfo
 ): PullRequestStatus {
   const globalConfig = context.config
   const globalResults = getConditionResults(globalConfig, conditions, pullRequestInfo)
-  if (!getConclusion(globalResults)) {
+
+  // If the global configuration returns failing conditions, then this overrules
+  // any of the results from rules. There is no need to evaluate rules, so
+  // return the global results here.
+  if (!areSuccessfulResults(globalResults)) {
     return globalResults
   }
-  return (globalConfig.rules || []).reduce<undefined | ConditionResults>((result, ruleConfig) => {
-    if (result !== undefined) {
-      return result
-    }
+
+  let fallbackNonSuccessfulResult = undefined
+  for (let ruleConfig of globalConfig.rules) {
     const ruleResult = getConditionResults(ruleConfig, conditions, pullRequestInfo)
-    if (getConclusion(ruleResult)) {
+    if (areSuccessfulResults(ruleResult)) {
       return ruleResult
     }
-    return undefined
-  }, undefined) || globalResults
+
+    // Store the first non-successful result for the return value when
+    // no rule has a successful result.
+    if (fallbackNonSuccessfulResult === undefined) {
+      fallbackNonSuccessfulResult = ruleResult
+    }
+  }
+  return fallbackNonSuccessfulResult || globalResults
 }
 
-export function getConclusion (conditionResults: ConditionResults): boolean {
+export function areSuccessfulResults (conditionResults: ConditionResults): boolean {
   return Object.values(conditionResults).every(conditionResult => conditionResult.status === 'success')
 }

test/integration.test.ts

@@ -145,3 +145,52 @@ it('pending test', async () => {
   expect(github.pullRequests.merge).toHaveBeenCalled()
 
 })
+
+it('to merge when one rule and the global configuration passes', async () => {
+  const config = `
+    rules:
+    - minApprovals:
+        OWNER: 1
+    - requiredLabels:
+      - merge
+  `
+
+  const pullRequestInfo = createPullRequestInfo({
+    reviews: {
+      nodes: [
+        approvedReview({
+          authorAssociation: 'CONTRIBUTOR'
+        })
+      ]
+    },
+    checkRuns: [
+      successCheckRun
+    ],
+    labels: {
+      nodes: [
+        { name: 'merge' }
+      ]
+    }
+  })
+
+  const github = createGithubApiFromPullRequestInfo({
+    pullRequestInfo,
+    config
+  })
+
+  const app = createApplication({
+    appFn: require('../src/index'),
+    logger: createEmptyLogger(),
+    github
+  })
+
+  await app.receive(
+    createPullRequestOpenedEvent({
+      owner: 'bobvanderlinden',
+      repo: 'probot-auto-merge',
+      number: 1
+    })
+  )
+
+  expect(github.pullRequests.merge).toHaveBeenCalled()
+})

test/pull-request-status.test.ts

@@ -128,4 +128,26 @@ describe('pull request status', () => {
 
     expect(results.minimumApprovals.status).toEqual('success')
   })
+
+  it('returns fail when when one rule fails even if global succeeds', () => {
+    const results = getPullRequestStatus(
+      createHandlerContext({
+        config: createConfig({
+          rules: [{
+            requiredLabels: ['merge']
+          }]
+        })
+      }),
+      createConditions({
+        requiredLabels: conditions.requiredLabels
+      }),
+      createPullRequestInfo({
+        reviews: {
+          nodes: []
+        }
+      })
+    )
+
+    expect(results.requiredLabels.status).toEqual('fail')
+  })
 })