fix: Manifest.fromConfig can find latest release version without comp…

…onent (#1123)
googleapis · Nov 26, 2021 · 0aeb67b · 0aeb67b
1 parent bf9aacd
commit 0aeb67b
Show file tree

Hide file tree

Showing 3 changed files with 77 additions and 13 deletions.
diff --git a/src/manifest.ts b/src/manifest.ts
@@ -298,7 +298,7 @@ export class Manifest {
     const latestVersion = await latestReleaseVersion(
       github,
       targetBranch,
-      component
+      config.includeComponentInTag ? component : ''
     );
     if (latestVersion) {
       releasedVersions[path] = latestVersion;
@@ -789,8 +789,7 @@ export class Manifest {
       const strategiesByPath = await this.getStrategiesByPath();
       for (const path in this.repositoryConfig) {
         const strategy = strategiesByPath[path];
-        const component =
-          strategy.component || (await strategy.getDefaultComponent()) || '';
+        const component = (await strategy.getComponent()) || '';
         if (this._pathsByComponent[component]) {
           logger.warn(
             `Multiple paths for ${component}: ${this._pathsByComponent[component]}, ${path}`

diff --git a/src/strategy.ts b/src/strategy.ts
@@ -115,6 +115,9 @@ export abstract class Strategy {
   ): Promise<Update[]>;
 
   async getComponent(): Promise<string | undefined> {
+    if (!this.includeComponentInTag) {
+      return '';
+    }
     return this.component || (await this.getDefaultComponent());
   }
 
@@ -290,7 +293,12 @@ export abstract class Strategy {
     mergedPullRequest: PullRequest
   ): Promise<Release | undefined> {
     if (this.skipGitHubRelease) {
-      return undefined;
+      logger.info('Release skipped from strategy config');
+      return;
+    }
+    if (!mergedPullRequest.sha) {
+      logger.error('Pull request should have been merged');
+      return;
     }
 
     const pullRequestTitle =
@@ -300,18 +308,18 @@ export abstract class Strategy {
         MANIFEST_PULL_REQUEST_TITLE_PATTERN
       );
     if (!pullRequestTitle) {
-      throw new Error(`Bad pull request title: '${mergedPullRequest.title}'`);
+      logger.error(`Bad pull request title: '${mergedPullRequest.title}'`);
+      return;
     }
     const branchName = BranchName.parse(mergedPullRequest.headBranchName);
     if (!branchName) {
-      throw new Error(`Bad branch name: ${mergedPullRequest.headBranchName}`);
-    }
-    if (!mergedPullRequest.sha) {
-      throw new Error('Pull request should have been merged');
+      logger.error(`Bad branch name: ${mergedPullRequest.headBranchName}`);
+      return;
     }
     const pullRequestBody = PullRequestBody.parse(mergedPullRequest.body);
     if (!pullRequestBody) {
-      throw new Error('could not parse pull request body as a release PR');
+      logger.error('Could not parse pull request body as a release PR');
+      return;
     }
     const component = await this.getComponent();
     logger.info('component:', component);
@@ -331,7 +339,8 @@ export abstract class Strategy {
     }
     const version = pullRequestTitle.getVersion() || releaseData?.version;
     if (!version) {
-      throw new Error('Pull request should have included version');
+      logger.error('Pull request should have included version');
+      return;
     }
 
     const tag = new TagName(

diff --git a/test/manifest.ts b/test/manifest.ts
@@ -180,6 +180,62 @@ describe('Manifest', () => {
       expect(Object.keys(manifest.repositoryConfig)).lengthOf(1);
       expect(Object.keys(manifest.releasedVersions)).lengthOf(1);
     });
+    it('finds previous release without tag', async () => {
+      mockCommits(github, [
+        {
+          sha: 'abc123',
+          message: 'some commit message',
+          files: [],
+          pullRequest: {
+            headBranchName: 'release-please/branches/main',
+            baseBranchName: 'main',
+            number: 123,
+            title: 'chore: release 1.2.3',
+            body: '',
+            labels: [],
+            files: [],
+          },
+        },
+      ]);
+
+      const manifest = await Manifest.fromConfig(github, 'target-branch', {
+        releaseType: 'simple',
+        bumpMinorPreMajor: true,
+        bumpPatchForMinorPreMajor: true,
+        component: 'foobar',
+        includeComponentInTag: false,
+      });
+      expect(Object.keys(manifest.repositoryConfig)).lengthOf(1);
+      expect(Object.keys(manifest.releasedVersions)).lengthOf(1);
+    });
+    it('finds previous release with tag', async () => {
+      mockCommits(github, [
+        {
+          sha: 'abc123',
+          message: 'some commit message',
+          files: [],
+          pullRequest: {
+            headBranchName: 'release-please/branches/main/components/foobar',
+            baseBranchName: 'main',
+            number: 123,
+            title: 'chore: release foobar 1.2.3',
+            body: '',
+            labels: [],
+            files: [],
+          },
+        },
+      ]);
+
+      const manifest = await Manifest.fromConfig(github, 'target-branch', {
+        releaseType: 'simple',
+        bumpMinorPreMajor: true,
+        bumpPatchForMinorPreMajor: true,
+        component: 'foobar',
+        includeComponentInTag: true,
+      });
+      expect(Object.keys(manifest.repositoryConfig)).lengthOf(1);
+      expect(Object.keys(manifest.releasedVersions)).lengthOf(1);
+    });
   });
 
   describe('buildPullRequests', () => {
@@ -1876,8 +1932,8 @@ describe('Manifest', () => {
             headBranchName: 'release-please/branches/main',
             baseBranchName: 'main',
             number: 1234,
-            title: 'chore: release main',
-            body: pullRequestBody('release-notes/single-manifest.txt'),
+            title: 'chore(main): release v1.3.1',
+            body: pullRequestBody('release-notes/single.txt'),
             labels: ['autorelease: pending'],
             files: [],
             sha: 'abc123',