Skip to content

Commit ffa00d2

Browse files
adangeladangel
committed
Determine modified files for pushes
The modified files are determined via compare API call. Similar to pull requests, we only collect at most 10 pages. However, the number of total changed files can't be calculated straightforward, since the pagination happens on the basis of commits rather than files. In any case, for big pushes this means, that not all files might be analyzed.
1 parent 4211caa commit ffa00d2

File tree

6 files changed

+117
-17
lines changed

6 files changed

+117
-17
lines changed

‎dist/index.js‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎lib/util.js‎

Lines changed: 44 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -108,30 +108,39 @@ const determineModifiedFiles = async function(token, sourcePath) {
108108

109109
// maximum of 300 files are loaded (page size is 30, max 10 pages)
110110
for(let page = 1; page < 10; page++) {
111-
const allFiles = await octokit.rest.pulls.listFiles({
111+
const listFilesResponse = await octokit.rest.pulls.listFiles({
112112
...context.repo,
113113
pull_number: eventData.number,
114114
per_page: 30,
115115
page: page
116116
});
117-
if (allFiles.data.length == 0) {
117+
const allFiles = listFilesResponse.data;
118+
if (allFiles.length == 0) {
118119
break;
119120
}
120-
core.debug(` got ${allFiles.data.length} entries from page ${page} to check...`);
121-
if (core.isDebug()) {
122-
// output can be enabled by adding repository secret "ACTIONS_STEP_DEBUG" with value "true".
123-
for (let i = 0; i < allFiles.data.length; i++) {
124-
core.debug(` ${i}: ${allFiles.data[i].status} ${allFiles.data[i].filename}`);
125-
}
126-
}
127-
const filenames = allFiles.data
128-
.filter(f => f.status === 'added' || f.status === 'changed' || f.status === 'modified')
129-
.map(f => path.normalize(f.filename))
130-
.filter(f => sourcePath === '.' || f.startsWith(sourcePath));
131-
if (core.isDebug()) {
132-
core.debug(` after filtering by status and with '${sourcePath}' ${filenames.length} files remain:`);
133-
core.debug(` ${filenames.join(', ')}`);
121+
const filenames = extractFilenames(allFiles, page, sourcePath);
122+
modifiedFilenames.push(...filenames);
123+
}
124+
125+
return modifiedFilenames;
126+
} else if (context.eventName === 'push') {
127+
core.debug(`Push on ${eventData.ref}: ${eventData.before}...${eventData.after}`);
128+
129+
let modifiedFilenames = [];
130+
131+
// maximum of 300 files are loaded (page size is 30, max 10 pages)
132+
for(let page = 1; page < 10; page++) {
133+
const compareResponse = await octokit.rest.repos.compareCommitsWithBasehead({
134+
...context.repo,
135+
basehead: `${eventData.before}...${eventData.after}`,
136+
per_page: 30,
137+
page: page
138+
});
139+
const allFiles = compareResponse.data.files;
140+
if (allFiles === undefined || allFiles.length == 0) {
141+
break;
134142
}
143+
const filenames = extractFilenames(allFiles, page, sourcePath);
135144
modifiedFilenames.push(...filenames);
136145
}
137146

@@ -141,6 +150,25 @@ const determineModifiedFiles = async function(token, sourcePath) {
141150
}
142151
}
143152

153+
function extractFilenames(allFiles, page, sourcePath) {
154+
core.debug(` got ${allFiles.length} entries from page ${page} to check...`);
155+
if (core.isDebug()) {
156+
// output can be enabled by adding repository secret "ACTIONS_STEP_DEBUG" with value "true".
157+
for (let i = 0; i < allFiles.length; i++) {
158+
core.debug(` ${i}: ${allFiles[i].status} ${allFiles[i].filename}`);
159+
}
160+
}
161+
const filenames = allFiles
162+
.filter(f => f.status === 'added' || f.status === 'changed' || f.status === 'modified')
163+
.map(f => path.normalize(f.filename))
164+
.filter(f => sourcePath === '.' || f.startsWith(sourcePath));
165+
if (core.isDebug()) {
166+
core.debug(` after filtering by status and with '${sourcePath}' ${filenames.length} files remain:`);
167+
core.debug(` ${filenames.join(', ')}`);
168+
}
169+
return filenames;
170+
}
171+
144172
module.exports.downloadPmd = downloadPmd;
145173
module.exports.executePmd = executePmd;
146174
module.exports.determineModifiedFiles = determineModifiedFiles;

‎tests/data/compare-files-page1.json‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
{
2+
"url": "https://api.github.com/repos/pmd/pmd-github-action-tests/compare/44c1557134c7dbaf46ecdf796fb871c8df8989e4...8a7a25638d8ca5207cc824dea9571325b243c6a1",
3+
"status": "ahead",
4+
"ahead_by": 2,
5+
"behind_by": 0,
6+
"total_commits": 2,
7+
"commits": [],
8+
"files": [
9+
{
10+
"sha": "879c25e370bb62b991c95adf0e4becf220d394fb",
11+
"filename": "src/main/java/AvoidCatchingThrowableSample.java",
12+
"status": "modified",
13+
"additions": 2,
14+
"deletions": 1,
15+
"changes": 3,
16+
"blob_url": "https://github.com/pmd/pmd-github-action-tests/blob/cfe417bd477804d0dd4cd9a6fd56e0bee5235b3f/src/main/java/AvoidCatchingThrowableSample.java",
17+
"raw_url": "https://github.com/pmd/pmd-github-action-tests/raw/cfe417bd477804d0dd4cd9a6fd56e0bee5235b3f/src/main/java/AvoidCatchingThrowableSample.java",
18+
"contents_url": "https://api.github.com/repos/pmd/pmd-github-action-tests/contents/src/main/java/AvoidCatchingThrowableSample.java?ref=cfe417bd477804d0dd4cd9a6fd56e0bee5235b3f",
19+
"patch": "@@ -1,9 +1,10 @@\n public class AvoidCatchingThrowableSample {\n+ \n public void bar() {\n try {\n // do something\n } catch (Throwable th) { // should not catch Throwable\n th.printStackTrace();\n }\n }\n-}\n\\ No newline at end of file\n+}"
20+
},
21+
{
22+
"filename": "src/main/java/DeletedFile.java",
23+
"status": "removed"
24+
},
25+
{
26+
"filename": "src/main/java/NewFile.java",
27+
"status": "added"
28+
},
29+
{
30+
"filename": "src/main/java/ChangedFile.java",
31+
"status": "changed"
32+
},
33+
{
34+
"filename": "README.md",
35+
"status": "modified"
36+
}
37+
]
38+
}

‎tests/data/compare-files-page2.json‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
{
2+
"url": "https://api.github.com/repos/pmd/pmd-github-action-tests/compare/44c1557134c7dbaf46ecdf796fb871c8df8989e4...8a7a25638d8ca5207cc824dea9571325b243c6a1",
3+
"status": "ahead",
4+
"ahead_by": 2,
5+
"behind_by": 0,
6+
"total_commits": 2,
7+
"commits": []
8+
}

‎tests/data/push-event-data.json‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"ref": "refs/heads/java",
3+
"before": "44c1557134c7dbaf46ecdf796fb871c8df8989e4",
4+
"after": "8a7a25638d8ca5207cc824dea9571325b243c6a1"
5+
}

‎tests/util.test.js‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,27 @@ describe('pmd-github-action-util', function () {
267267
await io.rmRF(pmdFilelist);
268268
await io.rmRF(path.join('.', 'pmd-report.sarif'));
269269
})
270+
271+
test('can determine modified files from push event (with debug)', async () => {
272+
// enable ACTIONS_STEP_DEBUG
273+
process.env['RUNNER_DEBUG'] = '1';
274+
process.env['GITHUB_REPOSITORY'] = 'pmd/pmd-github-action-tests'
275+
process.env['GITHUB_EVENT_NAME'] = 'push';
276+
process.env['GITHUB_EVENT_PATH'] = __dirname + '/data/push-event-data.json';
277+
nock('https://api.github.com')
278+
.get('/repos/pmd/pmd-github-action-tests/compare/44c1557134c7dbaf46ecdf796fb871c8df8989e4...8a7a25638d8ca5207cc824dea9571325b243c6a1?per_page=30&page=1')
279+
.replyWithFile(200, __dirname + '/data/compare-files-page1.json', {
280+
'Content-Type': 'application/json',
281+
});
282+
nock('https://api.github.com')
283+
.get('/repos/pmd/pmd-github-action-tests/compare/44c1557134c7dbaf46ecdf796fb871c8df8989e4...8a7a25638d8ca5207cc824dea9571325b243c6a1?per_page=30&page=2')
284+
.replyWithFile(200, __dirname + '/data/compare-files-page2.json', {
285+
'Content-Type': 'application/json',
286+
});
287+
let fileList = await util.determineModifiedFiles('my_test_token', path.normalize('src/main/java'));
288+
expect(fileList).toStrictEqual(['src/main/java/AvoidCatchingThrowableSample.java', 'src/main/java/NewFile.java', 'src/main/java/ChangedFile.java']
289+
.map(f => path.normalize(f)));
290+
})
270291
});
271292

272293
function setGlobal(key, value) {

0 commit comments

Comments
 (0)