From d926c840ec99de013a0b5cb198f9fb2700930cf7 Mon Sep 17 00:00:00 2001 From: Jeff Ching Date: Fri, 28 Oct 2022 10:17:26 -0700 Subject: [PATCH] fix(maven-workspace): SNAPSHOT versions should not be included in the manifest (#1727) * test: failing test for updating non-release versions in manifest * fix: SNAPSHOT versions should not be included in the manifest --- src/plugins/maven-workspace.ts | 19 ++++++- src/plugins/workspace.ts | 13 ++++- test/plugins/maven-workspace.ts | 95 +++++++++++++++++++++++++++++++++ 3 files changed, 124 insertions(+), 3 deletions(-) diff --git a/src/plugins/maven-workspace.ts b/src/plugins/maven-workspace.ts index 23cb6d758..76486a7d0 100644 --- a/src/plugins/maven-workspace.ts +++ b/src/plugins/maven-workspace.ts @@ -248,7 +248,10 @@ export class MavenWorkspace extends WorkspacePlugin { this.logger.warn(`${pomUpdate.path} does not have cached contents`); } } - if (candidate.pullRequest.version) { + if ( + candidate.pullRequest.version && + this.isReleaseVersion(candidate.pullRequest.version) + ) { updatedPathVersions.set(candidate.path, candidate.pullRequest.version); } } @@ -269,7 +272,9 @@ export class MavenWorkspace extends WorkspacePlugin { } else { this.logger.debug(`version: ${version} forced bump`); updatedVersions.set(packageName, version); - updatedPathVersions.set(this.pathFromPackage(pkg), version); + if (this.isReleaseVersion(version)) { + updatedPathVersions.set(this.pathFromPackage(pkg), version); + } } } } @@ -310,6 +315,16 @@ export class MavenWorkspace extends WorkspacePlugin { return graph; } + /** + * Given a release version, determine if we should bump the manifest + * version as well. For maven artifacts, SNAPSHOT versions are not + * considered releases. + * @param {Version} version The release version + */ + protected isReleaseVersion(version: Version): boolean { + return !version.preRelease?.includes('SNAPSHOT'); + } + protected bumpVersion(artifact: MavenArtifact): Version { const strategy = new JavaSnapshot(new AlwaysBumpPatch()); return strategy.bump(Version.parse(artifact.version), [FAKE_COMMIT]); diff --git a/src/plugins/workspace.ts b/src/plugins/workspace.ts index acbdc8a5e..8f5b2b9c5 100644 --- a/src/plugins/workspace.ts +++ b/src/plugins/workspace.ts @@ -266,7 +266,9 @@ export abstract class WorkspacePlugin extends ManifestPlugin { const version = this.bumpVersion(pkg); this.logger.debug(`version: ${version} forced bump`); updatedVersions.set(packageName, version); - updatedPathVersions.set(this.pathFromPackage(pkg), version); + if (this.isReleaseVersion(version)) { + updatedPathVersions.set(this.pathFromPackage(pkg), version); + } } } return { @@ -275,6 +277,15 @@ export abstract class WorkspacePlugin extends ManifestPlugin { }; } + /** + * Given a release version, determine if we should bump the manifest + * version as well. + * @param {Version} _version The release version + */ + protected isReleaseVersion(_version: Version): boolean { + return true; + } + /** * Given a package, return the new bumped version after updating * the dependency. diff --git a/test/plugins/maven-workspace.ts b/test/plugins/maven-workspace.ts index 33acf5e8e..9b3ddc735 100644 --- a/test/plugins/maven-workspace.ts +++ b/test/plugins/maven-workspace.ts @@ -31,6 +31,7 @@ import {Update} from '../../src/update'; import {Version} from '../../src/version'; import {PomXml} from '../../src/updaters/java/pom-xml'; import {RawContent} from '../../src/updaters/raw-content'; +import {ReleasePleaseManifest} from '../../src/updaters/release-please-manifest'; const sandbox = sinon.createSandbox(); const fixturesPath = './test/fixtures/plugins/maven-workspace'; @@ -403,6 +404,100 @@ describe('MavenWorkspace plugin', () => { ); safeSnapshot(await renderUpdate(github, bomUpdate)); }); + it('skips updating manifest with snapshot versions', async () => { + plugin = new MavenWorkspace(github, 'main', { + bom: { + releaseType: 'maven', + }, + maven1: { + releaseType: 'maven', + }, + maven2: { + releaseType: 'maven', + }, + maven3: { + releaseType: 'maven', + }, + maven4: { + releaseType: 'maven', + }, + }); + sandbox + .stub(github, 'findFilesByFilenameAndRef') + .withArgs('pom.xml', 'main') + .resolves([ + 'maven1/pom.xml', + 'maven2/pom.xml', + 'maven3/pom.xml', + 'maven4/pom.xml', + ]); + const candidates: CandidateReleasePullRequest[] = [ + buildMockCandidatePullRequest('maven1', 'maven', '1.1.2-SNAPSHOT', { + component: 'maven1', + updates: [ + buildMockPackageUpdate( + 'maven1/pom.xml', + 'maven1/pom.xml', + '1.1.2-SNAPSHOT' + ), + ], + }), + buildMockCandidatePullRequest('maven2', 'maven', '2.2.3-SNAPSHOT', { + component: 'maven2', + updates: [ + buildMockPackageUpdate( + 'maven2/pom.xml', + 'maven2/pom.xml', + '2.2.3-SNAPSHOT' + ), + ], + }), + buildMockCandidatePullRequest('maven3', 'maven', '3.3.4-SNAPSHOT', { + component: 'maven3', + updates: [ + buildMockPackageUpdate( + 'maven3/pom.xml', + 'maven3/pom.xml', + '3.3.4-SNAPSHOT' + ), + ], + }), + buildMockCandidatePullRequest('maven4', 'maven', '4.4.5-SNAPSHOT', { + component: 'maven4', + updates: [ + buildMockPackageUpdate( + 'maven4/pom.xml', + 'maven4/pom.xml', + '4.4.5-SNAPSHOT' + ), + ], + }), + ]; + stubFilesFromFixtures({ + sandbox, + github, + fixturePath: fixturesPath, + files: [ + 'maven1/pom.xml', + 'maven2/pom.xml', + 'maven3/pom.xml', + 'maven4/pom.xml', + ], + flatten: false, + targetBranch: 'main', + }); + const newCandidates = await plugin.run(candidates); + expect(newCandidates).length(1); + const update = assertHasUpdate( + newCandidates[0].pullRequest.updates, + '.release-please-manifest.json', + ReleasePleaseManifest + ); + const updater = update.updater as ReleasePleaseManifest; + for (const [_, version] of updater.versionsMap!) { + expect(version.toString()).to.not.include('SNAPSHOT'); + } + }); }); });