Skip to content

Commit 242742d

Browse files
committed
refactor(github-actions): extract shared labeling functionality into base class
Extracts the common logic used by IssueLabeling and PullRequestLabeling into a shared Labeling base class. This simplifies code duplication and unifies how labels are added, removed, and authentication is handled with Octokit.
1 parent 7ab0c74 commit 242742d

12 files changed

Lines changed: 2027 additions & 5669 deletions

File tree

github-actions/labeling/issue/BUILD.bazel

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ ts_project(
1717
"//github-actions/labeling:node_modules/@octokit/openapi-types",
1818
"//github-actions/labeling:node_modules/@octokit/rest",
1919
"//github-actions/labeling:node_modules/@types/node",
20+
"//github-actions/labeling/shared",
2021
"//ng-dev/pr/common/labels",
2122
],
2223
)
@@ -28,7 +29,6 @@ ts_project(
2829
tsconfig = "//github-actions:tsconfig_test",
2930
deps = [
3031
":lib",
31-
3232
"//github-actions:utils",
3333
"//github-actions/labeling:node_modules/@actions/core",
3434
"//github-actions/labeling:node_modules/@actions/github",
Lines changed: 34 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -1,74 +1,48 @@
11
import {Octokit} from '@octokit/rest';
22
import * as core from '@actions/core';
3-
import {context} from '@actions/github';
4-
import {GoogleGenAI} from '@google/genai';
5-
import {IssueLabeling} from './issue-labeling.js';
3+
import {IssueLabeling as _IssueLabeling} from './issue-labeling.js';
64

7-
class TestableIssueLabeling extends IssueLabeling {
8-
public constructor(git: Octokit, coreService: typeof core) {
9-
super(git, coreService);
5+
class IssueLabeling extends _IssueLabeling {
6+
setGit(git: any) {
7+
this.git = git;
108
}
119
}
1210

1311
describe('IssueLabeling', () => {
14-
let mockGit: {
15-
paginate: jasmine.Spy;
16-
issues: {
17-
listLabelsForRepo: jasmine.Spy;
18-
addLabels: jasmine.Spy;
19-
get: jasmine.Spy;
20-
};
21-
};
12+
let mockGit: jasmine.SpyObj<Octokit>;
2213
let mockAI: any;
23-
let mockCore: jasmine.SpyObj<typeof core>;
2414
let issueLabeling: IssueLabeling;
15+
let getIssue: jasmine.Spy;
2516

2617
beforeEach(() => {
27-
mockGit = {
28-
paginate: jasmine.createSpy('paginate'),
29-
issues: {
30-
listLabelsForRepo: jasmine.createSpy('listLabelsForRepo'),
31-
addLabels: jasmine.createSpy('addLabels'),
32-
get: jasmine.createSpy('get'),
33-
},
34-
};
18+
mockGit = jasmine.createSpyObj('Octokit', ['paginate', 'issues', 'pulls']);
19+
mockGit.issues = jasmine.createSpyObj('issues', ['addLabels', 'get']);
20+
21+
// Mock paginate to return the result of the promise if it's a list, or just execute the callback
22+
(mockGit.paginate as jasmine.Spy).and.callFake((fn: any, args: any) => {
23+
if (fn === mockGit.issues.listLabelsOnIssue) {
24+
return Promise.resolve([{name: 'area: core'}, {name: 'area: router'}, {name: 'bug'}]);
25+
}
26+
return Promise.resolve([]);
27+
});
3528

36-
mockGit.issues.addLabels.and.returnValue(Promise.resolve({}));
37-
mockGit.issues.get.and.returnValue(
38-
Promise.resolve({
39-
data: {
40-
title: 'Tough Issue',
41-
body: 'Complex Body',
42-
labels: [],
43-
},
44-
}),
45-
);
46-
mockGit.paginate.and.resolveTo([{name: 'area: core'}, {name: 'area: router'}, {name: 'bug'}]);
29+
(mockGit.issues.addLabels as unknown as jasmine.Spy).and.returnValue(Promise.resolve({}));
30+
getIssue = mockGit.issues.get as unknown as jasmine.Spy;
31+
getIssue.and.resolveTo({
32+
data: {
33+
title: 'Tough Issue',
34+
body: 'Complex Body',
35+
labels: [],
36+
},
37+
});
4738

4839
mockAI = {
4940
models: jasmine.createSpyObj('models', ['generateContent']),
5041
};
51-
mockCore = jasmine.createSpyObj<typeof core>('core', [
52-
'getInput',
53-
'info',
54-
'error',
55-
'warning',
56-
'debug',
57-
'setFailed',
58-
]);
59-
mockCore.getInput.and.returnValue('mock-ai-key');
60-
mockCore.error.and.callFake(console.error);
61-
mockCore.info.and.callFake(console.info);
62-
mockCore.warning.and.callFake(console.warn);
63-
mockCore.debug.and.callFake(console.debug);
64-
mockCore.setFailed.and.callFake(console.error);
65-
66-
67-
// We must cast the mock to Octokit because the mock only implements the subset used by the class.
68-
// This is standard for mocking large interfaces like Octokit.
69-
issueLabeling = new TestableIssueLabeling(mockGit as unknown as Octokit, mockCore);
70-
71-
spyOn(issueLabeling, 'getGenerativeAI').and.returnValue(mockAI);
42+
43+
spyOn(IssueLabeling.prototype, 'getGenerativeAI').and.returnValue(mockAI);
44+
issueLabeling = new IssueLabeling();
45+
issueLabeling.setGit(mockGit as unknown as Octokit);
7246
});
7347

7448
it('should initialize labels correctly', async () => {
@@ -85,6 +59,7 @@ describe('IssueLabeling', () => {
8559
}),
8660
);
8761

62+
await issueLabeling.initialize();
8863
await issueLabeling.run();
8964

9065
expect(mockGit.issues.addLabels).toHaveBeenCalledWith(
@@ -101,6 +76,7 @@ describe('IssueLabeling', () => {
10176
}),
10277
);
10378

79+
await issueLabeling.initialize();
10480
await issueLabeling.run();
10581

10682
expect(mockGit.issues.addLabels).not.toHaveBeenCalled();
@@ -113,28 +89,24 @@ describe('IssueLabeling', () => {
11389
}),
11490
);
11591

92+
await issueLabeling.initialize();
11693
await issueLabeling.run();
11794

11895
expect(mockGit.issues.addLabels).not.toHaveBeenCalled();
11996
});
12097

121-
it('should initialize and run with manual instantiation check', () => {
122-
expect(issueLabeling).toBeDefined();
123-
expect(mockCore.getInput).not.toHaveBeenCalled(); // until run is called
124-
});
125-
12698
it('should skip labeling when issue already has an area label', async () => {
127-
mockGit.issues.get.and.resolveTo({
99+
getIssue.and.resolveTo({
128100
data: {
129101
title: 'Tough Issue',
130102
body: 'Complex Body',
131103
labels: [{name: 'area: core'}],
132104
},
133105
});
106+
await issueLabeling.initialize();
134107

135108
await issueLabeling.run();
136109

137110
expect(mockGit.issues.addLabels).not.toHaveBeenCalled();
138-
expect(mockCore.info).toHaveBeenCalledWith('Issue already has an area label. Skipping.');
139111
});
140112
});
Lines changed: 10 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -1,51 +1,27 @@
11
import * as core from '@actions/core';
22
import {context} from '@actions/github';
33
import {GoogleGenAI} from '@google/genai';
4-
import {Octokit} from '@octokit/rest';
5-
import {ANGULAR_ROBOT, getAuthTokenFor, revokeActiveInstallationToken} from '../../../utils.js';
64
import {components} from '@octokit/openapi-types';
75
import {miscLabels} from '../../../../ng-dev/pr/common/labels/index.js';
6+
import {Labeling} from '../../shared/labeling.js';
87

9-
export class IssueLabeling {
10-
static run = async () => {
11-
const token = await getAuthTokenFor(ANGULAR_ROBOT);
12-
const git = new Octokit({auth: token});
13-
try {
14-
const inst = new this(git, core);
15-
await inst.run();
16-
} finally {
17-
await revokeActiveInstallationToken(git);
18-
}
19-
};
20-
8+
export class IssueLabeling extends Labeling {
9+
readonly type = 'Issue';
2110
/** Set of area labels available in the current repository. */
2211
repoAreaLabels = new Set<string>();
2312
/** The issue data fetched from Github. */
2413
issueData?: components['schemas']['issue'];
2514

26-
protected constructor(
27-
private git: Octokit,
28-
private coreService: typeof core,
29-
) {}
30-
3115
async run() {
32-
const {issue} = context;
33-
if (!issue || !issue.number) {
34-
this.coreService.info('No issue context found. Skipping.');
35-
return;
36-
}
37-
this.coreService.info(`Issue #${issue.number}`);
38-
39-
// Initialize labels and issue data
40-
await this.initialize();
16+
core.info(`Updating labels for ${this.type} #${context.issue.number}`);
4117

4218
// Determine if the issue already has an area label, if it does we can exit early.
4319
if (
4420
this.issueData?.labels.some((label) =>
4521
(typeof label === 'string' ? label : label.name)?.startsWith('area: '),
4622
)
4723
) {
48-
this.coreService.info('Issue already has an area label. Skipping.');
24+
core.info('Issue already has an area label. Skipping.');
4925
return;
5026
}
5127

@@ -77,38 +53,27 @@ If no area label applies, respond with "none".
7753
});
7854
const text = (response.text || '').trim();
7955

80-
this.coreService.info(`Gemini suggested label: ${text}`);
56+
core.info(`Gemini suggested label: ${text}`);
8157

8258
if (this.repoAreaLabels.has(text)) {
8359
await this.addLabel(text);
8460
await this.addLabel(miscLabels.GEMINI_TRIAGED.name);
8561
} else {
86-
this.coreService.info(
62+
core.info(
8763
`Generated label "${text}" is not in the list of valid area labels or is "ambiguous"/"none".`,
8864
);
8965
}
9066
} catch (e) {
91-
this.coreService.error('Failed to generate content from Gemini.');
92-
this.coreService.setFailed(e as Error);
67+
core.error('Failed to generate content from Gemini.');
68+
core.setFailed(e as Error);
9369
}
9470
}
9571

9672
getGenerativeAI() {
97-
const apiKey = this.coreService.getInput('google-generative-ai-key', {required: true});
73+
const apiKey = core.getInput('google-generative-ai-key', {required: true});
9874
return new GoogleGenAI({apiKey});
9975
}
10076

101-
async addLabel(label: string) {
102-
const {number: issue_number, owner, repo} = context.issue;
103-
try {
104-
await this.git.issues.addLabels({repo, owner, issue_number, labels: [label]});
105-
this.coreService.info(`Added ${label} label to Issue #${issue_number}`);
106-
} catch (err) {
107-
this.coreService.error(`Failed to add ${label} label to Issue #${issue_number}`);
108-
this.coreService.debug(err as string);
109-
}
110-
}
111-
11277
async initialize() {
11378
const {owner, repo} = context.issue;
11479
await Promise.all([
@@ -123,15 +88,5 @@ If no area label applies, respond with "none".
12388
this.issueData = resp.data;
12489
}),
12590
]);
126-
127-
if (this.repoAreaLabels.size === 0) {
128-
this.coreService.warning('No area labels found in the repository.');
129-
return;
130-
}
131-
132-
if (!this.issueData) {
133-
this.coreService.error('Failed to fetch issue data.');
134-
return;
135-
}
13691
}
13792
}

0 commit comments

Comments
 (0)