fix: make workspace plugin with separate-pull-request fine (#2310)

* fix merging behaviour in plugin * add test * remove unused test snapshot * update licnese header * considering plugin schema carefully * add unit test * remove `determineMerge` and set default value directly * set default value for `separatePullRequests` in `buildPlugin` --------- Co-authored-by: Jeff Ching <[email protected]>
googleapis · Oct 1, 2024 · 15c75a9 · 15c75a9
1 parent 9df5b71
commit 15c75a9
Show file tree

Hide file tree

Showing 6 changed files with 271 additions and 59 deletions.
diff --git a/__snapshots__/php-manifest.js b/__snapshots__/php-manifest.js
diff --git a/__snapshots__/separate-pull-requests-workspace.js b/__snapshots__/separate-pull-requests-workspace.js
@@ -0,0 +1,51 @@
+exports['Plugin compatibility separate-pull-requests and workspace plugin should version bump dependencies together 1'] = `
+:robot: I have created a release *beep* *boop*
+---
+
+
+## [1.0.1](https://github.com/fake-owner/fake-repo/compare/pkgA-v1.0.0...pkgA-v1.0.1) (1983-10-10)
+
+
+### Dependencies
+
+* The following workspace dependencies were updated
+  * dependencies
+    * pkgC bumped to 1.1.0
+
+---
+This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
+`
+
+exports['Plugin compatibility separate-pull-requests and workspace plugin should version bump dependencies together 2'] = `
+:robot: I have created a release *beep* *boop*
+---
+
+
+## [1.0.1](https://github.com/fake-owner/fake-repo/compare/pkgB-v1.0.0...pkgB-v1.0.1) (1983-10-10)
+
+
+### Dependencies
+
+* The following workspace dependencies were updated
+  * dependencies
+    * pkgC bumped to 1.1.0
+
+---
+This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
+`
+
+exports['Plugin compatibility separate-pull-requests and workspace plugin should version bump dependencies together 3'] = `
+:robot: I have created a release *beep* *boop*
+---
+
+
+## [1.1.0](https://github.com/fake-owner/fake-repo/compare/pkgC-v1.0.0...pkgC-v1.1.0) (1983-10-10)
+
+
+### Features
+
+* some feature ([aaaaaa](https://github.com/fake-owner/fake-repo/commit/aaaaaa))
+
+---
+This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
+`
diff --git a/src/factories/plugin-factory.ts b/src/factories/plugin-factory.ts
@@ -38,6 +38,7 @@ export interface PluginFactoryOptions {
   targetBranch: string;
   repositoryConfig: RepositoryConfig;
   manifestPath: string;
+  separatePullRequests?: boolean;
 
   // node options
   alwaysLinkLocal?: boolean;
@@ -54,6 +55,8 @@ export type PluginBuilder = (options: PluginFactoryOptions) => ManifestPlugin;
 
 const pluginFactories: Record<string, PluginBuilder> = {
   'linked-versions': options =>
+    // NOTE: linked-versions had already have a different behavior about merging
+    // see test/plugins/compatibility/linked-versions-workspace.ts
     new LinkedVersions(
       options.github,
       options.targetBranch,
@@ -73,6 +76,9 @@ const pluginFactories: Record<string, PluginBuilder> = {
       {
         ...options,
         ...(options.type as WorkspacePluginOptions),
+        merge:
+          (options.type as WorkspacePluginOptions).merge ??
+          !options.separatePullRequests,
       }
     ),
   'node-workspace': options =>
@@ -83,6 +89,9 @@ const pluginFactories: Record<string, PluginBuilder> = {
       {
         ...options,
         ...(options.type as WorkspacePluginOptions),
+        merge:
+          (options.type as WorkspacePluginOptions).merge ??
+          !options.separatePullRequests,
       }
     ),
   'maven-workspace': options =>
@@ -93,6 +102,9 @@ const pluginFactories: Record<string, PluginBuilder> = {
       {
         ...options,
         ...(options.type as WorkspacePluginOptions),
+        merge:
+          (options.type as WorkspacePluginOptions).merge ??
+          !options.separatePullRequests,
       }
     ),
   'sentence-case': options =>
@@ -112,6 +124,9 @@ const pluginFactories: Record<string, PluginBuilder> = {
 };
 
 export function buildPlugin(options: PluginFactoryOptions): ManifestPlugin {
+  if (!options.separatePullRequests) {
+    options.separatePullRequests = false;
+  }
   if (typeof options.type === 'object') {
     const builder = pluginFactories[options.type.type];
     if (builder) {

diff --git a/src/manifest.ts b/src/manifest.ts
@@ -388,6 +388,7 @@ export class Manifest {
         targetBranch: this.targetBranch,
         repositoryConfig: this.repositoryConfig,
         manifestPath: this.manifestPath,
+        separatePullRequests: this.separatePullRequests,
       })
     );
     this.pullRequestOverflowHandler = new FilePullRequestOverflowHandler(

diff --git a/test/factories/plugin-factory.ts b/test/factories/plugin-factory.ts
@@ -54,6 +54,7 @@ describe('PluginFactory', () => {
           targetBranch: 'target-branch',
           repositoryConfig,
           manifestPath: '.manifest.json',
+          separatePullRequests: false,
         });
         expect(plugin).to.not.be.undefined;
       });
@@ -66,6 +67,7 @@ describe('PluginFactory', () => {
           targetBranch: 'target-branch',
           repositoryConfig,
           manifestPath: '.manifest.json',
+          separatePullRequests: false,
         })
       ).to.throw();
     });
@@ -80,6 +82,7 @@ describe('PluginFactory', () => {
         targetBranch: 'target-branch',
         repositoryConfig,
         manifestPath: '.manifest.json',
+        separatePullRequests: false,
       });
       expect(plugin).to.not.be.undefined;
       expect(plugin).instanceof(LinkedVersions);
@@ -94,6 +97,7 @@ describe('PluginFactory', () => {
         targetBranch: 'target-branch',
         repositoryConfig,
         manifestPath: '.manifest.json',
+        separatePullRequests: false,
       });
       expect(plugin).to.not.be.undefined;
       expect(plugin).instanceof(GroupPriority);
@@ -108,6 +112,7 @@ describe('PluginFactory', () => {
         targetBranch: 'target-branch',
         repositoryConfig,
         manifestPath: '.manifest.json',
+        separatePullRequests: false,
       });
       expect(plugin).to.not.be.undefined;
       expect(plugin).instanceof(NodeWorkspace);
@@ -151,6 +156,7 @@ describe('PluginFactory', () => {
         repositoryConfig: {},
         targetBranch: 'main',
         manifestPath: '.manifest.json',
+        separatePullRequests: false,
       };
       const strategy = await buildPlugin(pluginOptions);
       expect(strategy).to.be.instanceof(CustomTest);