Merge pull request #1070 from intuit/all-contributors-bug

All contributors bug
intuit · Mar 19, 2020 · e3ad36f · e3ad36f
2 parents 4cc43c0 + e1943fa
commit e3ad36f
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 19 deletions.
diff --git a/.all-contributorsrc b/.all-contributorsrc
@@ -217,6 +217,17 @@
         "code",
         "doc"
       ]
+    },
+    {
+      "login": "hborawski",
+      "name": "Harris Borawski",
+      "avatar_url": "https://avatars1.githubusercontent.com/u/1325154?v=4",
+      "profile": "https://github.com/hborawski",
+      "contributions": [
+        "doc",
+        "test",
+        "code"
+      ]
     }
   ],
   "commitConvention": "none"

diff --git a/README.md b/README.md
@@ -212,6 +212,9 @@ Thanks goes to these wonderful people ([emoji key](https://github.com/kentcdodds
     <td align="center"><a href="http://www.jeremiahzucker.com"><img src="https://avatars1.githubusercontent.com/u/9255651?v=4" width="100px;" alt=""/><br /><sub><b>Jeremiah Zucker</b></sub></a><br /><a href="https://github.com/intuit/auto/commits?author=sugarmanz" title="Documentation">📖</a></td>
     <td align="center"><a href="https://www.unconfinedsoftware.com/"><img src="https://avatars2.githubusercontent.com/u/13172697?v=4" width="100px;" alt=""/><br /><sub><b>Brandon Miller</b></sub></a><br /><a href="https://github.com/intuit/auto/commits?author=unknownerror404" title="Code">💻</a> <a href="https://github.com/intuit/auto/commits?author=unknownerror404" title="Documentation">📖</a></td>
   </tr>
+  <tr>
+    <td align="center"><a href="https://github.com/hborawski"><img src="https://avatars1.githubusercontent.com/u/1325154?v=4" width="100px;" alt=""/><br /><sub><b>Harris Borawski</b></sub></a><br /><a href="https://github.com/intuit/auto/commits?author=hborawski" title="Documentation">📖</a> <a href="https://github.com/intuit/auto/commits?author=hborawski" title="Tests">⚠️</a> <a href="https://github.com/intuit/auto/commits?author=hborawski" title="Code">💻</a></td>
+  </tr>
 </table>
 
 <!-- markdownlint-enable -->

diff --git a/plugins/all-contributors/__tests__/all-contributors.test.ts b/plugins/all-contributors/__tests__/all-contributors.test.ts
@@ -12,7 +12,7 @@ const gitShow = jest.fn();
 const getLernaPackages = jest.fn();
 
 getLernaPackages.mockReturnValue(Promise.resolve([]));
-gitShow.mockReturnValue('');
+gitShow.mockReturnValue(Promise.resolve(''));
 exec.mockReturnValue('');
 
 jest.mock('child_process');
@@ -133,7 +133,7 @@ describe('All Contributors Plugin', () => {
           subject: 'Do the thing',
           hash: '123',
           labels: [],
-          files: [],
+          files: ['src/index.ts'],
           authors: [{ username: 'Jeff', hash: '123' }]
         }
       ]
@@ -266,6 +266,42 @@ describe('All Contributors Plugin', () => {
     );
   });
 
+  test('should include sub-commit contributors', async () => {
+    const releasedLabel = new AllContributors();
+    const autoHooks = makeHooks();
+    mockRead(
+      '{ "contributors": [ { "login": "Jeff", "contributions": ["infra"] } ] }'
+    );
+
+    releasedLabel.apply({ hooks: autoHooks, logger: dummyLog() } as Auto.Auto);
+
+    await autoHooks.afterAddToChangelog.promise({
+      bump: Auto.SEMVER.patch,
+      currentVersion: '0.0.0',
+      lastRelease: '0.0.0',
+      releaseNotes: '',
+      commits: [
+        {
+          subject: 'Do the thing',
+          hash: '123',
+          labels: [],
+          files: ['src/index.ts'],
+          authors: [
+            { username: 'Jeff', hash: '123' },
+            { username: 'Adam', hash: '456' }
+          ]
+        }
+      ]
+    });
+
+    expect(exec.mock.calls[0][0]).toBe(
+      'npx all-contributors-cli add Jeff infra,code'
+    );
+    expect(exec.mock.calls[1][0]).toBe(
+      'npx all-contributors-cli add Adam code'
+    );
+  });
+
   test('should not update if no new contribution types', async () => {
     const releasedLabel = new AllContributors();
     const autoHooks = makeHooks();

diff --git a/plugins/all-contributors/src/index.ts b/plugins/all-contributors/src/index.ts
@@ -124,21 +124,14 @@ export default class AllContributorsPlugin implements IPlugin {
     auto.hooks.afterAddToChangelog.tapPromise(
       this.name,
       async ({ commits }) => {
-        await this.updateContributors(auto, commits);
         const rootDir = process.cwd();
-
-        let packages: {
-          /** name of the package to manage contributors in */
-          name: string;
-          /** path to the package */
-          path: string;
-        }[];
+        // Always do the root package
+        let packages = [{ path: rootDir, name: 'root-package' }];
 
         try {
-          packages = await getLernaPackages();
-        } catch (error) {
-          packages = [{ path: rootDir, name: 'single-package' }];
-        }
+          // Try to get sub-packages
+          packages = [...packages, ...(await getLernaPackages())];
+        } catch (error) {}
 
         // Cannot run git operations in parallel
         await packages.reduce(async (last, { name, path }) => {
@@ -229,15 +222,11 @@ export default class AllContributorsPlugin implements IPlugin {
           return isMatch;
         })
         .forEach(contribution => {
-          authors.forEach(({ username, hash }) => {
+          authors.forEach(({ username }) => {
             if (!username) {
               return;
             }
 
-            if (commit.hash !== hash) {
-              return;
-            }
-
             if (!authorContributions[username]) {
               authorContributions[username] = new Set();
             }