Skip to content
Merged
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
8 changes: 4 additions & 4 deletions .github/local-actions/branch-manager/main.js

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions ng-dev/pr/checkout/takeover.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ export async function checkoutAsPrTakeover(

// Validate that the takeover attempt is being made against a pull request created by an
// expected account.
if (!takeoverAccounts.includes(pullRequest.author.login)) {
if (!pullRequest.author || !takeoverAccounts.includes(pullRequest.author.login)) {
Log.warn(
` ⚠ ${bold(pullRequest.author.login)} is not an account fully supported for takeover.`,
` ⚠ ${bold(pullRequest.author?.login ?? 'unknown')} is not an account fully supported for takeover.`,
);
Log.warn(` Supported accounts: ${bold(takeoverAccounts.join(', '))}`);
if (
Expand Down
21 changes: 21 additions & 0 deletions ng-dev/pr/common/test/common.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,27 @@ describe('pull request validation', () => {
const results = await assertValidPullRequest(pr, config, ngDevConfig, null, prTarget, git);
expect(results.length).toBe(1);
});

it('should handle comments with null author without crashing', async () => {
const config = createIsolatedValidationConfig({assertEnforceTested: true});
let pr = createTestPullRequest();
const comments = [
{
authorAssociation: 'MEMBER' as CommentAuthorAssociation,
author: null as any,
bodyText: 'TESTED="blah"',
},
];
const commentHelper = PullRequestComments.create(git, pr.number);
spyOn(PullRequestComments, 'create').and.returnValue(commentHelper);
spyOn(commentHelper, 'loadPullRequestComments').and.returnValue(Promise.resolve(comments));

pr.labels.nodes.push({name: requiresLabels['REQUIRES_TGP'].name});
// Should not throw, should return validation error since no googler tested it
const results = await assertValidPullRequest(pr, config, ngDevConfig, null, prTarget, git);
expect(results.length).toBe(1);
expect(results[0].message).toContain('Pull Request requires a TGP');
});
});

describe('assert-isolate-primitives', () => {
Expand Down
7 changes: 5 additions & 2 deletions ng-dev/pr/common/validation/assert-allowed-target-label.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,8 +75,11 @@ class Validation extends PullRequestValidation {
}
break;
case targetLabels['TARGET_AUTOMATION']:
if (!automationBots.includes(pullRequest.author.login)) {
throw this._createUserUsingAutomationLabelError(targetLabel, pullRequest.author.login);
if (!pullRequest.author || !automationBots.includes(pullRequest.author.login)) {
throw this._createUserUsingAutomationLabelError(
targetLabel,
pullRequest.author?.login ?? 'unknown',
);
}
break;
default:
Expand Down
1 change: 1 addition & 0 deletions ng-dev/pr/common/validation/assert-enforce-tested.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@ export async function pullRequestHasValidTestedComment(
for (const {bodyText, author} of comments) {
if (
bodyText.startsWith(`TESTED=`) &&
author &&
(await githubMacros.isGooglerOrgMember(gitClient.github, author.login))
) {
return true;
Expand Down