Skip to content

Commit

Permalink
cleanup(snippet-bot): reduce memory footprint for large diffs (#1144)
Browse files Browse the repository at this point in the history
* cleanup(snippet-bot): reduce memory footprint for large diffs

fixes #1143

* refactor a bit more

Co-authored-by: Benjamin E. Coe <[email protected]>
  • Loading branch information
Takashi Matsuo and bcoe authored Nov 17, 2020
1 parent 5495e7b commit 03cf910
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 11 deletions.
10 changes: 6 additions & 4 deletions packages/snippet-bot/src/region-tag-parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
// limitations under the License.

import parseDiff from 'parse-diff';
import fetch from 'node-fetch';

/**
* The result for unmatched region tag checks.
Expand Down Expand Up @@ -57,15 +58,15 @@ const END_TAG_REGEX = /\[END ([^\]]*)\]/;
/**
* Detects region tag changes in a pull request and return the summary.
*/
export function parseRegionTagsInPullRequest(
diff: string,
export async function parseRegionTagsInPullRequest(
diffUrl: string,
owner: string,
repo: string,
sha: string,
headOwner: string,
headRepo: string,
headSha: string
): ChangesInPullRequest {
): Promise<ChangesInPullRequest> {
const changes: Change[] = [];
const files: string[] = [];
const ret = {
Expand All @@ -74,7 +75,8 @@ export function parseRegionTagsInPullRequest(
deleted: 0,
files: files,
};

const response = await fetch(diffUrl);
const diff = await response.text();
const diffResult = parseDiff(diff);
for (const file of diffResult) {
if (file.to !== undefined && file.to !== '/dev/null') {
Expand Down
6 changes: 2 additions & 4 deletions packages/snippet-bot/src/snippet-bot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -326,11 +326,9 @@ ${bodyDetail}`
// Check on pull requests.

// Parse the PR diff and recognize added/deleted region tags.
const response = await fetch(context.payload.pull_request.diff_url);
const diff = await response.text();

const result = parseRegionTagsInPullRequest(
diff,
const result = await parseRegionTagsInPullRequest(
context.payload.pull_request.diff_url,
context.payload.pull_request.base.repo.owner.login,
context.payload.pull_request.base.repo.name,
context.payload.pull_request.base.sha,
Expand Down
14 changes: 11 additions & 3 deletions packages/snippet-bot/test/region-tag-parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,21 @@ import {resolve} from 'path';
import * as fs from 'fs';
import assert from 'assert';
import {describe, it} from 'mocha';
import nock from 'nock';

const fixturesPath = resolve(__dirname, '../../test/fixtures');

nock.disableNetConnect();

describe('region-tag-parser', () => {
const diff = fs.readFileSync(resolve(fixturesPath, 'diff.txt'), 'utf8');
describe('parses a diff', () => {
it('returns a correct result', () => {
const result = parseRegionTagsInPullRequest(
diff,
it('returns a correct result', async () => {
const requests = nock('https://example.com')
.get('/diff.txt')
.reply(200, diff);
const result = await parseRegionTagsInPullRequest(
'https://example.com/diff.txt',
'owner',
'repo',
'sha',
Expand All @@ -46,6 +52,8 @@ describe('region-tag-parser', () => {
assert.strictEqual('headOwner', result.changes[1].owner);
assert.strictEqual('headRepo', result.changes[1].repo);
assert.strictEqual('headSha', result.changes[1].sha);

requests.done();
});
});
});

0 comments on commit 03cf910

Please sign in to comment.