Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Redesign review commenting as upload of JSON artifact #112

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
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
45 changes: 45 additions & 0 deletions __tests__/suggestions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,12 @@ const VIOLATION_2184_SUGGESTION: PatchSuggestion = {
\`\`\``
};

const PATCH_SUGGESTIONS = [
VIOLATION_1118_SUGGESTION,
VIOLATION_1854_SUGGESTION,
VIOLATION_2184_SUGGESTION
];

jest.setTimeout(20 * 1000);

/**
Expand Down Expand Up @@ -150,6 +156,45 @@ async function testGeneratePatchSuggestions(
expect(patchSuggestions).toEqual(expectedSuggestions);
}

test('toCommentsJSON produces JSON output containing all suggestions', async () => {
const jsonOutput = await suggestions.toCommentsJSON(PATCH_SUGGESTIONS);

for (const ps of PATCH_SUGGESTIONS) {
const encodedSuggestion = ps.suggestion.replace(/\n/g, '\\n');
expect(jsonOutput).toContain(encodedSuggestion);
}
});

test('toCommentsJSON produces JSON output containing all violation specifiers', async () => {
const jsonOutput = await suggestions.toCommentsJSON(PATCH_SUGGESTIONS);

for (const ps of PATCH_SUGGESTIONS) {
expect(jsonOutput).toContain(ps.violationSpec);
}
});

test('toCommentsJSON does not include start_line when linesToReplace range is single line', async () => {
const singleLinePatchSuggestion = {
...VIOLATION_1118_SUGGESTION
};
singleLinePatchSuggestion.linesToReplace = {start: 1, end: 2}; // note: exclusive at end

const jsonOutput = await suggestions.toCommentsJSON([singleLinePatchSuggestion]);

expect(jsonOutput).not.toContain('start_line');
});

test('toCommentsJSON includes start_line when linesToReplace range is multiple lines', async () => {
const multiLinePatchSuggestion = {
...VIOLATION_1118_SUGGESTION
};
multiLinePatchSuggestion.linesToReplace = {start: 1, end: 20};

const jsonOutput = await suggestions.toCommentsJSON([multiLinePatchSuggestion]);

expect(jsonOutput).toContain('start_line');
});

/**
* Setup a Git repository with the given resources committed.
*/
Expand Down
5 changes: 3 additions & 2 deletions action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@ inputs:
ratchet-from:
description: 'Commit-ish to ratchet from, typically origin/master or origin/main)'
required: false
suggestions-token:
description: 'Token to post pull request review comments with. If provided, repairs are posted as suggestions on the pull request.'
post-review-comments:
description: 'If true and the commenter bot is installed, repairs are posted as review comments on pull requests'
required: false
default: false
runs:
using: 'node12'
main: 'dist/index.js'
Loading