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(ng-dev): allow deprecations in PRs during feature freeze #256

Closed
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
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