Skip to content

Commit

Permalink
fix(maven-workspace): SNAPSHOT versions should not be included in the…
Browse files Browse the repository at this point in the history
… manifest (#1727)

* test: failing test for updating non-release versions in manifest

* fix: SNAPSHOT versions should not be included in the manifest
  • Loading branch information
chingor13 authored Oct 28, 2022
1 parent 8920c7f commit d926c84
Show file tree
Hide file tree
Showing 3 changed files with 124 additions and 3 deletions.
19 changes: 17 additions & 2 deletions src/plugins/maven-workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,10 @@ export class MavenWorkspace extends WorkspacePlugin<MavenArtifact> {
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);
}
}
Expand All @@ -269,7 +272,9 @@ export class MavenWorkspace extends WorkspacePlugin<MavenArtifact> {
} 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);
}
}
}
}
Expand Down Expand Up @@ -310,6 +315,16 @@ export class MavenWorkspace extends WorkspacePlugin<MavenArtifact> {
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]);
Expand Down
13 changes: 12 additions & 1 deletion src/plugins/workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,9 @@ export abstract class WorkspacePlugin<T> 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 {
Expand All @@ -275,6 +277,15 @@ export abstract class WorkspacePlugin<T> 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.
Expand Down
95 changes: 95 additions & 0 deletions test/plugins/maven-workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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');
}
});
});
});

Expand Down

0 comments on commit d926c84

Please sign in to comment.