fix(flex): before validate invocation of checks should only happen fo…

…r relevant events specified in `when` clause (#169)

* test: add tests for beforeValidate and afterValidate

* fix(flex): ensure that event in context is taken into consideration before invoking beforeValidate

* refactor(flex): added back comment that was accidentally removed.

__tests__/flex.test.js

@@ -1,5 +1,78 @@
 const executor = require('../lib/flex')
 const Helper = require('../__fixtures__/helper')
+const { Action } = require('../lib/actions/action')
+
+describe('Test beforeValidate and afterValidate invocations', async () => {
+  let context
+  let registry = { validators: new Map(), actions: new Map() }
+  let action
+  let config = `
+    version: 2
+    mergeable:
+      - when: pull_request.*
+        validate:
+          - do: title
+            must_exclude:
+              regex: wip|work in progress|do not merge
+              message: 'a custom message'
+          - do: label
+            must_exclude:
+              regex: wip|work in progress
+  `
+  let configWithMultiple = config + `
+      - when: pull_request_review.submitted
+        validate:
+          - do: milestone
+            no_empty:
+              enabled: true
+  `
+
+  beforeEach(() => {
+    process.env.MERGEABLE_VERSION = 'flex'
+    context = Helper.mockContext('title')
+    Helper.mockConfigWithContext(context, config)
+
+    action = new Action()
+    action.beforeValidate = jest.fn()
+    action.afterValidate = jest.fn()
+    action.supportedEvents = ['pull_request.opened', 'pull_request.edited', 'pull_request_review.submitted']
+    registry.actions.set('checks', action)
+  })
+
+  test('when event is in configuration', async () => {
+    context.event = 'pull_request'
+    context.payload.action = 'opened'
+    await executor(context, registry)
+    expect(action.beforeValidate.mock.calls.length).toBe(1)
+    expect(action.afterValidate.mock.calls.length).toBe(1)
+  })
+
+  test('when event is not in configuration', async () => {
+    context.event = 'pull_request_review'
+    context.payload.action = 'submitted'
+    await executor(context, registry)
+    expect(action.beforeValidate.mock.calls.length).toBe(0)
+    expect(action.afterValidate.mock.calls.length).toBe(0)
+  })
+
+  test('when event is in configuration with multiple whens', async () => {
+    Helper.mockConfigWithContext(context, configWithMultiple)
+    context.event = 'pull_request_review'
+    context.payload.action = 'submitted'
+    await executor(context, registry)
+    expect(action.beforeValidate.mock.calls.length).toBe(1)
+    expect(action.afterValidate.mock.calls.length).toBe(1)
+  })
+
+  test('when event is NOT in configuration with multiple whens', async () => {
+    Helper.mockConfigWithContext(context, configWithMultiple)
+    context.event = 'pull_request_review'
+    context.payload.action = 'commented'
+    await executor(context, registry)
+    expect(action.beforeValidate.mock.calls.length).toBe(0)
+    expect(action.afterValidate.mock.calls.length).toBe(0)
+  })
+})
 
 describe('#executor', () => {
   beforeEach(() => {

lib/flex.js

@@ -52,7 +52,7 @@ const formatErrorSummary = (errors) => {
 
 const processWorkflow = async (context, registry, config) => {
   // go through the settings and register all the validators
-  await config.registerValidatorsAndActions(registry)
+  config.registerValidatorsAndActions(registry)
 
   // do pre validation actions
   await processPreActions(context, registry, config)
@@ -81,9 +81,14 @@ const getValiatorPromises = (context, registry, rule) => {
 // call all action classes' beforeValidate, regardless of whether they are in failure or pass situation
 const processPreActions = async (context, registry, config) => {
   let promises = []
-  registry.actions.forEach(action => {
-    if (action.isEventSupported(`${context.event}.${context.payload.action}`)) {
-      promises.push(action.beforeValidate({ context }))
+
+  config.settings.forEach(rule => {
+    if (isEventInContext(rule.when, context)) {
+      registry.actions.forEach(action => {
+        if (action.isEventSupported(`${context.event}.${context.payload.action}`)) {
+          promises.push(action.beforeValidate({ context }))
+        }
+      })
     }
   })