Skip to content

Commit

Permalink
fix(ng-dev): allow deprecations in PRs during feature freeze (#256)
Browse files Browse the repository at this point in the history
Currently our tooling does not distinguish between when RC is occurring and when feature freeze is occurring.
Since they are treated as the same state, we are unable to allow deprecations messages on feature freeze without
allowing it during RC periods.  By creating this distinction we can properly handle this case.

PR Close #256
  • Loading branch information
josephperrott committed Oct 8, 2021
1 parent cc39ee3 commit cce6d07
Show file tree
Hide file tree
Showing 17 changed files with 357 additions and 239 deletions.
7 changes: 4 additions & 3 deletions ng-dev/caretaker/check/ci.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ describe('CiModule', () => {
status: 'failed',
},
]);
fetchActiveReleaseTrainsSpy.and.resolveTo([]);
const trains = buildMockActiveReleaseTrains(true);
fetchActiveReleaseTrainsSpy.and.resolveTo(trains);

const module = new CiModule({caretaker: {}, ...mockNgDevConfig});
Object.defineProperty(module, 'data', {value: fakeData});
Expand All @@ -113,9 +114,9 @@ function buildMockActiveReleaseTrains(withRc: boolean): versioning.ActiveRelease
isMajor: false,
version: new SemVer('0.0.0'),
};
return {
return new versioning.ActiveReleaseTrains({
releaseCandidate: withRc ? {branchName: 'rc-branch', ...baseResult} : null,
latest: {branchName: 'latest-branch', ...baseResult},
next: {branchName: 'next-branch', ...baseResult},
};
});
}
5 changes: 2 additions & 3 deletions ng-dev/caretaker/check/ci.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,8 @@ export class CiModule extends BaseModule<CiData> {
...this.git.remoteConfig,
nextBranchName,
};
const releaseTrains = await fetchActiveReleaseTrains(repo);

const ciResultPromises = Object.entries(releaseTrains).map(
const {latest, next, releaseCandidate} = await fetchActiveReleaseTrains(repo);
const ciResultPromises = Object.entries({releaseCandidate, latest, next}).map(
async ([trainName, train]: [string, ReleaseTrain | null]) => {
if (train === null) {
return {
Expand Down
4 changes: 2 additions & 2 deletions ng-dev/pr/common/targeting/labels.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import {assertValidReleaseConfig, ReleaseConfig} from '../../../release/config/index';
import {
fetchActiveReleaseTrains,
ActiveReleaseTrains,
getNextBranchName,
isVersionBranch,
ReleaseRepoWithApi,
Expand Down Expand Up @@ -38,6 +38,7 @@ import {debug} from '../../../utils/console';
* NPM version data when LTS version branches are validated.
*/
export async function getTargetLabelsForActiveReleaseTrains(
{latest, releaseCandidate, next}: ActiveReleaseTrains,
api: GithubClient,
config: Partial<{github: GithubConfig; release: ReleaseConfig}>,
): Promise<TargetLabel[]> {
Expand All @@ -50,7 +51,6 @@ export async function getTargetLabelsForActiveReleaseTrains(
nextBranchName,
api,
};
const {latest, releaseCandidate, next} = await fetchActiveReleaseTrains(repo);

const targetLabels: TargetLabel[] = [
{
Expand Down
12 changes: 10 additions & 2 deletions ng-dev/pr/common/targeting/target-label.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {Commit} from '../../../commit-message/parse';
import {assertChangesAllowForTargetLabel} from '../validation/validations';
import {PullRequestFailure} from '../validation/failures';
import {GithubClient} from '../../../utils/git/github';
import {fetchActiveReleaseTrains} from '../../../release/versioning';

/**
* Enum capturing available target label names in the Angular organization. A target
Expand Down Expand Up @@ -111,15 +112,22 @@ export async function getTargetBranchesForPullRequest(
// can lazily compute branches for a target label and throw. e.g. if an invalid target
// label is applied, we want to exit the script gracefully with an error message.
try {
const targetLabels = await getTargetLabelsForActiveReleaseTrains(api, config);
const {mainBranchName, name, owner} = config.github;
const releaseTrains = await fetchActiveReleaseTrains({
name,
nextBranchName: mainBranchName,
owner,
api,
});
const targetLabels = await getTargetLabelsForActiveReleaseTrains(releaseTrains, api, config);
const matchingLabel = await getMatchingTargetLabelForPullRequest(
config.pullRequest,
labelsOnPullRequest,
targetLabels,
);
const targetBranches = await getBranchesFromTargetLabel(matchingLabel, githubTargetBranch);

assertChangesAllowForTargetLabel(commits, matchingLabel, config.pullRequest);
assertChangesAllowForTargetLabel(commits, matchingLabel, config.pullRequest, releaseTrains);

return targetBranches;
} catch (error) {
Expand Down
4 changes: 3 additions & 1 deletion ng-dev/pr/common/validation/validations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
PullRequestFromGithub,
PullRequestStatus,
} from '../fetch-pull-request';
import {ActiveReleaseTrains} from '../../../release/versioning';

/**
* Assert the commits provided are allowed to merge to the provided target label,
Expand All @@ -26,6 +27,7 @@ export function assertChangesAllowForTargetLabel(
commits: Commit[],
label: TargetLabel,
config: PullRequestConfig,
releaseTrains: ActiveReleaseTrains,
) {
/**
* List of commit scopes which are exempted from target label content requirements. i.e. no `feat`
Expand Down Expand Up @@ -57,7 +59,7 @@ export function assertChangesAllowForTargetLabel(
// Deprecations should not be merged into RC, patch or LTS branches.
// https://semver.org/#spec-item-7. Deprecations should be part of
// minor releases, or major releases according to SemVer.
if (hasDeprecations) {
if (hasDeprecations && !releaseTrains.isFeatureFreeze()) {
throw PullRequestFailure.hasDeprecations(label);
}
break;
Expand Down
10 changes: 9 additions & 1 deletion ng-dev/pr/merge/integration.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
} from '../common/targeting/target-label';
import {fakeGithubPaginationResponse} from '../../utils/testing/github-interception';
import {getTargetLabelsForActiveReleaseTrains} from '../common/targeting/labels';
import {fetchActiveReleaseTrains} from '../../release/versioning';

const API_ENDPOINT = `https://api.github.com`;

Expand Down Expand Up @@ -103,7 +104,14 @@ describe('default target labels', () => {
name: string,
githubTargetBranch = 'master',
): Promise<string[] | null> {
const targetLabels = await getTargetLabelsForActiveReleaseTrains(api, {
const {mainBranchName, name: repoName, owner} = githubConfig;
const releaseTrains = await fetchActiveReleaseTrains({
name: repoName,
nextBranchName: mainBranchName,
owner,
api,
});
const targetLabels = await getTargetLabelsForActiveReleaseTrains(releaseTrains, api, {
github: githubConfig,
release: releaseConfig,
});
Expand Down
8 changes: 4 additions & 4 deletions ng-dev/release/publish/test/common.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,18 @@ import {getMockGitClient} from '../../../utils/testing';
import {SandboxGitRepo} from '../../../utils/testing';

describe('common release action logic', () => {
const baseReleaseTrains: ActiveReleaseTrains = {
const baseReleaseTrains = new ActiveReleaseTrains({
releaseCandidate: null,
next: new ReleaseTrain('master', parse('10.1.0-next.0')),
latest: new ReleaseTrain('10.0.x', parse('10.0.1')),
};
});

describe('version computation', () => {
const testReleaseTrain: ActiveReleaseTrains = {
const testReleaseTrain = new ActiveReleaseTrains({
releaseCandidate: new ReleaseTrain('10.1.x', parse('10.1.0-next.3')),
next: new ReleaseTrain('master', parse('10.2.0-next.0')),
latest: new ReleaseTrain('10.0.x', parse('10.0.1')),
};
});

it('should not modify release train versions and cause invalid other actions', async () => {
const {releaseConfig, githubConfig} = getTestConfigurationsForAction();
Expand Down
50 changes: 30 additions & 20 deletions ng-dev/release/publish/test/configure-next-as-major.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,47 +7,57 @@
*/

import {getBranchPushMatcher} from '../../../utils/testing';
import {ActiveReleaseTrains} from '../../versioning';
import {ReleaseTrain} from '../../versioning/release-trains';
import {ConfigureNextAsMajorAction} from '../actions/configure-next-as-major';
import {parse, setupReleaseActionForTesting} from './test-utils/test-utils';

describe('configure next as major action', () => {
it('should be active if the next branch is for a minor', async () => {
expect(
await ConfigureNextAsMajorAction.isActive({
releaseCandidate: null,
next: new ReleaseTrain('master', parse('10.1.0-next.3')),
latest: new ReleaseTrain('10.0.x', parse('10.0.3')),
}),
await ConfigureNextAsMajorAction.isActive(
new ActiveReleaseTrains({
releaseCandidate: null,
next: new ReleaseTrain('master', parse('10.1.0-next.3')),
latest: new ReleaseTrain('10.0.x', parse('10.0.3')),
}),
),
).toBe(true);
});

it('should be active regardless of a feature-freeze/release-candidate train', async () => {
expect(
await ConfigureNextAsMajorAction.isActive({
releaseCandidate: new ReleaseTrain('10.1.x', parse('10.1.0-rc.1')),
next: new ReleaseTrain('master', parse('10.2.0-next.3')),
latest: new ReleaseTrain('10.0.x', parse('10.0.3')),
}),
await ConfigureNextAsMajorAction.isActive(
new ActiveReleaseTrains({
releaseCandidate: new ReleaseTrain('10.1.x', parse('10.1.0-rc.1')),
next: new ReleaseTrain('master', parse('10.2.0-next.3')),
latest: new ReleaseTrain('10.0.x', parse('10.0.3')),
}),
),
).toBe(true);
});

it('should not be active if the next branch is for a major', async () => {
expect(
await ConfigureNextAsMajorAction.isActive({
releaseCandidate: null,
next: new ReleaseTrain('master', parse('11.0.0-next.0')),
latest: new ReleaseTrain('10.0.x', parse('10.0.3')),
}),
await ConfigureNextAsMajorAction.isActive(
new ActiveReleaseTrains({
releaseCandidate: null,
next: new ReleaseTrain('master', parse('11.0.0-next.0')),
latest: new ReleaseTrain('10.0.x', parse('10.0.3')),
}),
),
).toBe(false);
});

it('should compute proper version and create staging pull request', async () => {
const action = setupReleaseActionForTesting(ConfigureNextAsMajorAction, {
releaseCandidate: null,
next: new ReleaseTrain('master', parse('10.1.0-next.3')),
latest: new ReleaseTrain('10.0.x', parse('10.0.2')),
});
const action = setupReleaseActionForTesting(
ConfigureNextAsMajorAction,
new ActiveReleaseTrains({
releaseCandidate: null,
next: new ReleaseTrain('master', parse('10.1.0-next.3')),
latest: new ReleaseTrain('10.0.x', parse('10.0.2')),
}),
);

const {repo, fork, gitClient} = action;
const expectedVersion = `11.0.0-next.0`;
Expand Down
58 changes: 34 additions & 24 deletions ng-dev/release/publish/test/cut-lts-patch.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,44 +25,54 @@ import {readFileSync} from 'fs';
import {testTmpDir} from '../../../utils/testing';
import {SandboxGitRepo} from '../../../utils/testing';
import {getMockGitClient} from '../../../utils/testing';
import {ActiveReleaseTrains} from '../../versioning';

describe('cut an LTS patch action', () => {
it('should be active', async () => {
expect(
await CutLongTermSupportPatchAction.isActive({
releaseCandidate: null,
next: new ReleaseTrain('master', parse('10.1.0-next.3')),
latest: new ReleaseTrain('10.0.x', parse('10.0.3')),
}),
await CutLongTermSupportPatchAction.isActive(
new ActiveReleaseTrains({
releaseCandidate: null,
next: new ReleaseTrain('master', parse('10.1.0-next.3')),
latest: new ReleaseTrain('10.0.x', parse('10.0.3')),
}),
),
).toBe(true);
});

it('should be active if there is a feature-freeze train', async () => {
expect(
await CutLongTermSupportPatchAction.isActive({
releaseCandidate: new ReleaseTrain('10.1.x', parse('10.1.0-next.3')),
next: new ReleaseTrain('master', parse('10.2.0-next.3')),
latest: new ReleaseTrain('10.0.x', parse('10.0.3')),
}),
await CutLongTermSupportPatchAction.isActive(
new ActiveReleaseTrains({
releaseCandidate: new ReleaseTrain('10.1.x', parse('10.1.0-next.3')),
next: new ReleaseTrain('master', parse('10.2.0-next.3')),
latest: new ReleaseTrain('10.0.x', parse('10.0.3')),
}),
),
).toBe(true);
});

it('should be active if there is a release-candidate train', async () => {
expect(
await CutLongTermSupportPatchAction.isActive({
releaseCandidate: new ReleaseTrain('10.1.x', parse('10.1.0-rc.0')),
next: new ReleaseTrain('master', parse('10.2.0-next.3')),
latest: new ReleaseTrain('10.0.x', parse('10.0.3')),
}),
await CutLongTermSupportPatchAction.isActive(
new ActiveReleaseTrains({
releaseCandidate: new ReleaseTrain('10.1.x', parse('10.1.0-rc.0')),
next: new ReleaseTrain('master', parse('10.2.0-next.3')),
latest: new ReleaseTrain('10.0.x', parse('10.0.3')),
}),
),
).toBe(true);
});

it('should compute proper new version and select correct branch', async () => {
const action = setupReleaseActionForTesting(CutLongTermSupportPatchAction, {
releaseCandidate: null,
next: new ReleaseTrain('master', parse('10.1.0-next.3')),
latest: new ReleaseTrain('10.0.x', parse('10.0.2')),
});
const action = setupReleaseActionForTesting(
CutLongTermSupportPatchAction,
new ActiveReleaseTrains({
releaseCandidate: null,
next: new ReleaseTrain('master', parse('10.1.0-next.3')),
latest: new ReleaseTrain('10.0.x', parse('10.0.2')),
}),
);

spyOn<any>(action.instance, '_promptForTargetLtsBranch').and.resolveTo({
name: '9.2.x',
Expand All @@ -76,11 +86,11 @@ describe('cut an LTS patch action', () => {
it('should generate release notes capturing changes to previous latest LTS version', async () => {
const action = setupReleaseActionForTesting(
CutLongTermSupportPatchAction,
{
new ActiveReleaseTrains({
releaseCandidate: null,
next: new ReleaseTrain('master', parse('10.1.0-next.3')),
latest: new ReleaseTrain('10.0.x', parse('10.0.2')),
},
}),
true,
{useSandboxGitClient: true},
);
Expand Down Expand Up @@ -118,11 +128,11 @@ describe('cut an LTS patch action', () => {
it('should include number of active LTS branches in action description', async () => {
const {releaseConfig, githubConfig} = getTestConfigurationsForAction();
const gitClient = getMockGitClient(githubConfig, /* useSandboxGitClient */ false);
const activeReleaseTrains = {
const activeReleaseTrains = new ActiveReleaseTrains({
releaseCandidate: null,
next: new ReleaseTrain('master', parse('10.1.0-next.3')),
latest: new ReleaseTrain('10.0.x', parse('10.0.2')),
};
});

fakeNpmPackageQueryRequest(releaseConfig.npmPackages[0], {
'dist-tags': {'v9-lts': '9.1.2', 'v8-lts': '8.2.2'},
Expand Down
Loading

0 comments on commit cce6d07

Please sign in to comment.