Better check for empty configs in checkAll

Author: joshdales

__tests__/fixtures/any_and_all.yml

@@ -0,0 +1,6 @@
+tests:
+  - any:
+      - head-branch: ['^tests/', '^test/']
+      - changed-files: ['tests/**/*']
+  - all:
+      - changed-files: ['!tests/requirements.txt']

__tests__/main.test.ts

@@ -17,7 +17,8 @@ const getPullMock = jest.spyOn(gh.rest.pulls, 'get');
 const yamlFixtures = {
   'branches.yml': fs.readFileSync('__tests__/fixtures/branches.yml'),
   'only_pdfs.yml': fs.readFileSync('__tests__/fixtures/only_pdfs.yml'),
-  'not_supported.yml': fs.readFileSync('__tests__/fixtures/not_supported.yml')
+  'not_supported.yml': fs.readFileSync('__tests__/fixtures/not_supported.yml'),
+  'any_and_all.yml': fs.readFileSync('__tests__/fixtures/any_and_all.yml')
 };
 
 afterAll(() => jest.restoreAllMocks());
@@ -167,19 +168,28 @@ describe('run', () => {
     });
   });
 
-  it('can support multiple branches by providing an array', async () => {
-    github.context.payload.pull_request!.head = {ref: 'array/123'};
-    usingLabelerConfigYaml('branches.yml');
+  it('adds a label when matching any and all patterns are provided', async () => {
+    usingLabelerConfigYaml('any_and_all.yml');
+    mockGitHubResponseChangedFiles('tests/test.ts');
     await run();
 
     expect(addLabelsMock).toHaveBeenCalledTimes(1);
     expect(addLabelsMock).toHaveBeenCalledWith({
       owner: 'monalisa',
       repo: 'helloworld',
       issue_number: 123,
-      labels: ['array-branch']
+      labels: ['tests']
     });
   });
+
+  it('does not add a label when not all any and all patterns are matched', async () => {
+    usingLabelerConfigYaml('any_and_all.yml');
+    mockGitHubResponseChangedFiles('tests/requirements.txt');
+    await run();
+
+    expect(addLabelsMock).toHaveBeenCalledTimes(0);
+    expect(removeLabelMock).toHaveBeenCalledTimes(0);
+  });
 });
 
 function usingLabelerConfigYaml(fixtureName: keyof typeof yamlFixtures): void {

dist/index.js

@@ -439,7 +439,8 @@ function checkMatch(changedFiles, matchConfig) {
 // equivalent to "Array.some()" but expanded for debugging and clarity
 function checkAny(matchConfigs, changedFiles) {
     core.debug(`  checking "any" patterns`);
-    if (!matchConfigs.length) {
+    if (!matchConfigs.length ||
+        !matchConfigs.some(configOption => Object.keys(configOption).length)) {
         core.debug(`  no "any" patterns to check`);
         return false;
     }
@@ -468,8 +469,7 @@ exports.checkAny = checkAny;
 function checkAll(matchConfigs, changedFiles) {
     core.debug(`  checking "all" patterns`);
     if (!matchConfigs.length ||
-        // Make sure that all the configs have keys that we can check for
-        !matchConfigs.some(configOption => ALLOWED_CONFIG_KEYS.includes(Object.keys(configOption)[0]))) {
+        !matchConfigs.some(configOption => Object.keys(configOption).length)) {
         core.debug(`  no "all" patterns to check`);
         return false;
     }

src/labeler.ts

@@ -225,7 +225,10 @@ export function checkAny(
   changedFiles: string[]
 ): boolean {
   core.debug(`  checking "any" patterns`);
-  if (!matchConfigs.length) {
+  if (
+    !matchConfigs.length ||
+    !matchConfigs.some(configOption => Object.keys(configOption).length)
+  ) {
     core.debug(`  no "any" patterns to check`);
     return false;
   }
@@ -262,10 +265,7 @@ export function checkAll(
   core.debug(`  checking "all" patterns`);
   if (
     !matchConfigs.length ||
-    // Make sure that all the configs have keys that we can check for
-    !matchConfigs.some(configOption =>
-      ALLOWED_CONFIG_KEYS.includes(Object.keys(configOption)[0])
-    )
+    !matchConfigs.some(configOption => Object.keys(configOption).length)
   ) {
     core.debug(`  no "all" patterns to check`);
     return false;