Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 0 additions & 59 deletions __snapshots__/php-manifest.js

This file was deleted.

51 changes: 51 additions & 0 deletions __snapshots__/separate-pull-requests-workspace.js
Original file line number Diff line number Diff line change
@@ -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).
`
27 changes: 27 additions & 0 deletions src/factories/plugin-factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
RepositoryConfig,
SentenceCasePluginConfig,
GroupPriorityPluginConfig,
WorkspacePluginConfig,
} from '../manifest';
import {GitHub} from '../github';
import {ManifestPlugin} from '../plugin';
Expand All @@ -38,6 +39,7 @@ export interface PluginFactoryOptions {
targetBranch: string;
repositoryConfig: RepositoryConfig;
manifestPath: string;
separatePullRequests: boolean;

// node options
alwaysLinkLocal?: boolean;
Expand All @@ -63,6 +65,7 @@ const pluginFactories: Record<string, PluginBuilder> = {
{
...options,
...(options.type as WorkspacePluginOptions),
merge: determineMerge(options),
}
),
'cargo-workspace': options =>
Expand All @@ -73,6 +76,7 @@ const pluginFactories: Record<string, PluginBuilder> = {
{
...options,
...(options.type as WorkspacePluginOptions),
merge: determineMerge(options),
}
),
'node-workspace': options =>
Expand All @@ -83,6 +87,7 @@ const pluginFactories: Record<string, PluginBuilder> = {
{
...options,
...(options.type as WorkspacePluginOptions),
merge: determineMerge(options),
}
),
'maven-workspace': options =>
Expand All @@ -93,6 +98,7 @@ const pluginFactories: Record<string, PluginBuilder> = {
{
...options,
...(options.type as WorkspacePluginOptions),
merge: determineMerge(options),
}
),
'sentence-case': options =>
Expand Down Expand Up @@ -149,3 +155,24 @@ export function unregisterPlugin(name: string) {
export function getPluginTypes(): readonly VersioningStrategyType[] {
return Object.keys(pluginFactories).sort();
}

export function determineMerge(
options: PluginFactoryOptions
): boolean | undefined {
// NOTE: linked-versions had already have a different behavior when this code wrote
// see test/plugins/compatibility/linked-versions-workspace.ts
if (typeof options.type === 'string' && options.type !== 'linked-versions') {
return !options.separatePullRequests;
}
if (typeof options.type !== 'string') {
const type = options.type as
| LinkedVersionPluginConfig
| WorkspacePluginConfig;
if (typeof type.merge === 'undefined' && type.type !== 'linked-versions') {
return !options.separatePullRequests;
}
return type.merge;
}
// return undefined due to relying on the default behavior of the plugin constructor
return undefined;
}
1 change: 1 addition & 0 deletions src/manifest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,7 @@ export class Manifest {
targetBranch: this.targetBranch,
repositoryConfig: this.repositoryConfig,
manifestPath: this.manifestPath,
separatePullRequests: this.separatePullRequests,
})
);
this.pullRequestOverflowHandler = new FilePullRequestOverflowHandler(
Expand Down
75 changes: 75 additions & 0 deletions test/factories/plugin-factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
getPluginTypes,
registerPlugin,
unregisterPlugin,
determineMerge,
} from '../../src/factories/plugin-factory';
import {expect} from 'chai';
import {LinkedVersions} from '../../src/plugins/linked-versions';
Expand Down Expand Up @@ -54,6 +55,7 @@ describe('PluginFactory', () => {
targetBranch: 'target-branch',
repositoryConfig,
manifestPath: '.manifest.json',
separatePullRequests: false,
});
expect(plugin).to.not.be.undefined;
});
Expand All @@ -66,6 +68,7 @@ describe('PluginFactory', () => {
targetBranch: 'target-branch',
repositoryConfig,
manifestPath: '.manifest.json',
separatePullRequests: false,
})
).to.throw();
});
Expand All @@ -80,6 +83,7 @@ describe('PluginFactory', () => {
targetBranch: 'target-branch',
repositoryConfig,
manifestPath: '.manifest.json',
separatePullRequests: false,
});
expect(plugin).to.not.be.undefined;
expect(plugin).instanceof(LinkedVersions);
Expand All @@ -94,6 +98,7 @@ describe('PluginFactory', () => {
targetBranch: 'target-branch',
repositoryConfig,
manifestPath: '.manifest.json',
separatePullRequests: false,
});
expect(plugin).to.not.be.undefined;
expect(plugin).instanceof(GroupPriority);
Expand All @@ -108,6 +113,7 @@ describe('PluginFactory', () => {
targetBranch: 'target-branch',
repositoryConfig,
manifestPath: '.manifest.json',
separatePullRequests: false,
});
expect(plugin).to.not.be.undefined;
expect(plugin).instanceof(NodeWorkspace);
Expand Down Expand Up @@ -151,6 +157,7 @@ describe('PluginFactory', () => {
repositoryConfig: {},
targetBranch: 'main',
manifestPath: '.manifest.json',
separatePullRequests: false,
};
const strategy = await buildPlugin(pluginOptions);
expect(strategy).to.be.instanceof(CustomTest);
Expand All @@ -170,4 +177,72 @@ describe('PluginFactory', () => {
expect(allTypes).to.contain(pluginType);
});
});
describe('determineMerge', () => {
const pluginOptions = {
github,
repositoryConfig: {},
targetBranch: 'main',
manifestPath: '.manifest.json',
};
it('should return false', () => {
const separatePullRequests = true;

expect(
determineMerge({
...pluginOptions,
separatePullRequests,
type: 'node-workspace',
})
).to.be.false;

expect(
determineMerge({
...pluginOptions,
separatePullRequests,
type: {
type: 'maven-workspace',
},
})
).to.be.false;
});
it('should return true', () => {
expect(
determineMerge({
...pluginOptions,
separatePullRequests: false,
type: 'cargo-workspace',
})
).to.be.true;

expect(
determineMerge({
...pluginOptions,
separatePullRequests: true,
type: {
type: 'node-workspace',
merge: true,
},
})
).to.be.true;
});
it('should return undefined', () => {
expect(
determineMerge({
...pluginOptions,
separatePullRequests: true,
type: 'linked-versions',
})
).to.be.undefined;

expect(
determineMerge({
...pluginOptions,
separatePullRequests: true,
type: {
type: 'linked-versions',
},
})
).to.be.undefined;
});
});
});
Loading