Log warning when MAX_PAGE is reached

This always logs a warning if we fetched page MAX_PAGE
regardless whether there would be more pages to fetch
or not. No effort is done to determine
whether the list of modified files is complete.

Also the modified files are now collected in
a Set to avoid duplicate entries.

Author: adangel

dist/index.js

@@ -7,6 +7,11 @@ const os = require('os');
 const fs = require('fs').promises;
 const path = require('path');
 
+// Load at most MAX_PAGE pages when determining modified files.
+// This is used both for pull/{pull_number}/files as well as for
+// repos/compareCommits API calls.
+const MAX_PAGE = 10;
+
 const downloadPmd = async function(version, token) {
     let pmdVersion = version;
     let cachedPmdPath = tc.find('pmd', version);
@@ -104,10 +109,11 @@ const determineModifiedFiles = async function(token, sourcePath) {
     if (context.eventName === 'pull_request') {
         core.debug(`Pull request ${eventData.number}: ${eventData.pull_request.html_url}`);
 
-        let modifiedFilenames = [];
+        let modifiedFilenames = new Set();
 
         // maximum of 300 files are loaded (page size is 30, max 10 pages)
-        for(let page = 1; page < 10; page++) {
+        let page;
+        for(page = 1; page <= MAX_PAGE; page++) {
             const listFilesResponse = await octokit.rest.pulls.listFiles({
                 ...context.repo,
                 pull_number: eventData.number,
@@ -119,17 +125,21 @@ const determineModifiedFiles = async function(token, sourcePath) {
                 break;
             }
             const filenames = extractFilenames(allFiles, page, sourcePath);
-            modifiedFilenames.push(...filenames);
+            filenames.forEach(f => modifiedFilenames.add(f));
+        }
+        if (page >= MAX_PAGE) {
+            core.warning(`The pull request ${eventData.number} is too big - not all changed files will be analyzed!`);
         }
 
-        return modifiedFilenames;
+        return [...modifiedFilenames];
     } else if (context.eventName === 'push') {
         core.debug(`Push on ${eventData.ref}: ${eventData.before}...${eventData.after}`);
 
-        let modifiedFilenames = [];
+        let modifiedFilenames = new Set();
 
         // maximum of 300 files are loaded (page size is 30, max 10 pages)
-        for(let page = 1; page < 10; page++) {
+        let page;
+        for(page = 1; page <= MAX_PAGE; page++) {
             const compareResponse = await octokit.rest.repos.compareCommitsWithBasehead({
                 ...context.repo,
                 basehead: `${eventData.before}...${eventData.after}`,
@@ -141,10 +151,13 @@ const determineModifiedFiles = async function(token, sourcePath) {
                 break;
             }
             const filenames = extractFilenames(allFiles, page, sourcePath);
-            modifiedFilenames.push(...filenames);
+            filenames.forEach(f => modifiedFilenames.add(f));
+        }
+        if (page >= MAX_PAGE) {
+            core.warning(`The push on ${eventData.ref} is too big - not all changed files will be analyzed!`);
         }
 
-        return modifiedFilenames;
+        return [...modifiedFilenames];
     } else {
         core.warning(`Unsupported github action event '${context.eventName}' - cannot determine modified files. All files will be analyzed.`);
         return undefined;

lib/util.js

@@ -218,6 +218,24 @@ describe('pmd-github-action-util', function () {
       .map(f => path.normalize(f)));
   })
 
+  test('can determine modified files from pull request with max page', async () => {
+    // disable ACTIONS_STEP_DEBUG
+    delete process.env['RUNNER_DEBUG'];
+    process.env['GITHUB_REPOSITORY'] = 'pmd/pmd-github-action-tests'
+    process.env['GITHUB_EVENT_NAME'] = 'pull_request';
+    process.env['GITHUB_EVENT_PATH'] = __dirname + '/data/pull-request-event-data.json';
+    for (let page = 1; page <= 10; page++) {
+      nock('https://api.github.com')
+        .get(`/repos/pmd/pmd-github-action-tests/pulls/1/files?per_page=30&page=${page}`)
+        .replyWithFile(200, __dirname + '/data/pull-request-files.json', {
+          'Content-Type': 'application/json',
+        });
+    }
+    let fileList = await util.determineModifiedFiles('my_test_token', path.normalize('src/main/java'));
+    expect(fileList).toStrictEqual(['src/main/java/AvoidCatchingThrowableSample.java', 'src/main/java/NewFile.java', 'src/main/java/ChangedFile.java']
+      .map(f => path.normalize(f)));
+  })
+
   test('return undefined for unsupported event', async () => {
     process.env['GITHUB_REPOSITORY'] = 'pmd/pmd-github-action-tests'
     process.env['GITHUB_EVENT_NAME'] = 'workflow_dispatch';
@@ -289,6 +307,24 @@ describe('pmd-github-action-util', function () {
     expect(fileList).toStrictEqual(['src/main/java/AvoidCatchingThrowableSample.java', 'src/main/java/NewFile.java', 'src/main/java/ChangedFile.java']
       .map(f => path.normalize(f)));
   })
+
+  test('can determine modified files from push event with max page', async () => {
+    // disable ACTIONS_STEP_DEBUG
+    delete process.env['RUNNER_DEBUG'];
+    process.env['GITHUB_REPOSITORY'] = 'pmd/pmd-github-action-tests'
+    process.env['GITHUB_EVENT_NAME'] = 'push';
+    process.env['GITHUB_EVENT_PATH'] = __dirname + '/data/push-event-data.json';
+    for (let page = 1; page <= 10; page++) {
+      nock('https://api.github.com')
+        .get(`/repos/pmd/pmd-github-action-tests/compare/44c1557134c7dbaf46ecdf796fb871c8df8989e4...8a7a25638d8ca5207cc824dea9571325b243c6a1?per_page=30&page=${page}`)
+        .replyWithFile(200, __dirname + '/data/compare-files-page1.json', {
+          'Content-Type': 'application/json',
+        });
+    }
+    let fileList = await util.determineModifiedFiles('my_test_token', path.normalize('src/main/java'));
+    expect(fileList).toStrictEqual(['src/main/java/AvoidCatchingThrowableSample.java', 'src/main/java/NewFile.java', 'src/main/java/ChangedFile.java']
+      .map(f => path.normalize(f)));
+  })
 });
 
 function setGlobal(key, value) {