Skip to content

Commit

Permalink
Trim PR comments from the front. (#1252)
Browse files Browse the repository at this point in the history
* Trim PR comments from the front.

* add PR link to CHANGELOG

* optionally trim long pr comments from front instead of back

* consistently print trim warnings

* fix output issue and add to tests to account for it

* fix comment formatting and account for extra new line in tests

* update variables to reflect always-include-summary
  • Loading branch information
devonmoss authored Aug 22, 2024
1 parent 88001a3 commit 1fa1109
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 16 deletions.
5 changes: 3 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@

## HEAD (Unreleased)

_(none)_
- feat: Trim PR comments from the front
([#1252](https://github.com/pulumi/actions/pull/1252))

---

Expand Down Expand Up @@ -311,4 +312,4 @@ _(none)_

## 2.0.1 (2021-02-26)

- Initial reworking of the Action to be TypeScript based
- Initial reworking of the Action to be TypeScript based
5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -143,12 +143,17 @@ The action can be configured with the following arguments:

- `plan` - (optional) Used for
[update plans](https://www.pulumi.com/docs/concepts/update-plans/)

- On `preview`: Where to save the update plan. If you choose to use this in a
different run of your workflow (let's say you create an update plan via a
preview on Pull Request creation and want to `up` using the plan) you must
upload the plan as an artifact, and retrieve it wherever you run `up`
- On `up`: Where to read the update plan from.

- `always-include-summary` - (optional) If `true`, then the action will trim
long pr comments from the front instead of the back. This ensures that the
resources summary is always included in the comment.

By default, this action will try to authenticate Pulumi with
[Pulumi Cloud](https://app.pulumi.com/). If you have not specified a
`PULUMI_ACCESS_TOKEN` then you will need to specify an alternative backend via
Expand Down
4 changes: 4 additions & 0 deletions action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,10 @@ inputs:
description: 'Suppress display of periodic progress dots to limit logs length'
required: false
default: 'false'
always-include-summary:
description: 'If comments must be trimmed, trim them from the front. This ensures the resources summary is always included in the comment'
required: false
default: 'false'
outputs:
output:
description: Output from running command
Expand Down
3 changes: 2 additions & 1 deletion src/__tests__/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const defaultConfig: Record<string, string> = {
'pulumi-version': '^3',
'comment-on-pr': 'false',
'comment-on-summary': 'false',
'always-include-summary': 'false',
upsert: 'false',
remove: 'false',
refresh: 'false',
Expand Down Expand Up @@ -39,6 +40,7 @@ describe('config.ts', () => {
expect(c).toBeTruthy();
expect(c).toMatchInlineSnapshot(`
Object {
"alwaysIncludeSummary": false,
"cloudUrl": "file://~",
"command": "up",
"commentOnPr": false,
Expand Down Expand Up @@ -197,5 +199,4 @@ describe('config.ts', () => {
/Only one of 'pulumi-version' or 'pulumi-version-file' should be provided, got both/,
);
});

});
1 change: 1 addition & 0 deletions src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ export function makeConfig() {
// @see https://github.com/pulumi/actions/pull/912
configMap: getYAMLInput<ConfigMap>('config-map'),
editCommentOnPr: getBooleanInput('edit-pr-comment'),
alwaysIncludeSummary: getBooleanInput('always-include-summary'),

options: {
parallel: getNumberInput('parallel', {}),
Expand Down
43 changes: 37 additions & 6 deletions src/libs/__tests__/pr.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ describe('pr.ts', () => {

await handlePullRequestMessage(defaultOptions, projectName, 'test');
expect(createComment).toHaveBeenCalledWith({
body: '#### :tropical_drink: `preview` on myFirstProject/staging\n\n<details>\n<summary>Pulumi report</summary>\n\n<pre>\ntest\n</pre>\n\n</details>',
body: '#### :tropical_drink: `preview` on myFirstProject/staging\n\n<details>\n<summary>Pulumi report</summary>\n\n\n<pre>\ntest\n</pre>\n\n</details>',
issue_number: 123,
});
});
Expand All @@ -69,7 +69,7 @@ describe('pr.ts', () => {

await handlePullRequestMessage(defaultOptions, projectName, '\x1b[30mblack\x1b[37mwhite');
expect(createComment).toHaveBeenCalledWith({
body: '#### :tropical_drink: `preview` on myFirstProject/staging\n\n<details>\n<summary>Pulumi report</summary>\n\n<pre>\n<span style="color:#000">black<span style="color:#AAA">white</span></span>\n</pre>\n\n</details>',
body: '#### :tropical_drink: `preview` on myFirstProject/staging\n\n<details>\n<summary>Pulumi report</summary>\n\n\n<pre>\n<span style="color:#000">black<span style="color:#AAA">white</span></span>\n</pre>\n\n</details>',
issue_number: 123,
});
});
Expand All @@ -93,7 +93,7 @@ describe('pr.ts', () => {

await handlePullRequestMessage(options, projectName, 'test');
expect(createComment).toHaveBeenCalledWith({
body: '#### :tropical_drink: `preview` on myFirstProject/staging\n\n<details>\n<summary>Pulumi report</summary>\n\n<pre>\ntest\n</pre>\n\n</details>',
body: '#### :tropical_drink: `preview` on myFirstProject/staging\n\n<details>\n<summary>Pulumi report</summary>\n\n\n<pre>\ntest\n</pre>\n\n</details>',
issue_number: 87,
});
});
Expand All @@ -120,7 +120,7 @@ describe('pr.ts', () => {

await handlePullRequestMessage(options, projectName, 'test');
expect(createComment).toHaveBeenCalledWith({
body: '#### :tropical_drink: `preview` on myFirstProject/staging\n\n<details>\n<summary>Pulumi report</summary>\n\n<pre>\ntest\n</pre>\n\n</details>',
body: '#### :tropical_drink: `preview` on myFirstProject/staging\n\n<details>\n<summary>Pulumi report</summary>\n\n\n<pre>\ntest\n</pre>\n\n</details>',
issue_number: 87,
});
});
Expand All @@ -144,7 +144,38 @@ describe('pr.ts', () => {

const call = createComment.mock.calls[0][0];
expect(call.body.length).toBeLessThan(65_536);
expect(call.body).toContain('The output was too long and trimmed');
expect(call.body).toContain('The output was too long and trimmed.');
expect(call.body).not.toContain('The output was too long and trimmed from the front.');
});

it('should trim the output from front when the output is larger than 64k characters and config is set', async () => {
process.env.GITHUB_REPOSITORY = 'pulumi/actions';
// @ts-ignore
gh.context = {
payload: {
pull_request: {
number: 123,
},
},
};
const alwaysIncludeSummaryOptions = {
command: 'preview',
stackName: 'staging',
alwaysIncludeSummary: true,
options: {},
} as Config;

await handlePullRequestMessage(
alwaysIncludeSummaryOptions,
projectName,
'a'.repeat(65_000) + '\n' + 'this is at the end and should be in the output',
);

const call = createComment.mock.calls[0][0];
expect(call.body.length).toBeLessThan(65_536);
expect(call.body).toContain('this is at the end and should be in the output');
expect(call.body).toContain('The output was too long and trimmed from the front.');
expect(call.body).not.toContain('The output was too long and trimmed.');
});

it('should edit the comment if it finds a previous created one', async () => {
Expand All @@ -161,7 +192,7 @@ describe('pr.ts', () => {
await handlePullRequestMessage(options, projectName, 'test');
expect(updateComment).toHaveBeenCalledWith({
comment_id: 2,
body: '#### :tropical_drink: `preview` on myFirstProject/staging\n\n<details>\n<summary>Pulumi report</summary>\n\n<pre>\ntest\n</pre>\n\n</details>',
body: '#### :tropical_drink: `preview` on myFirstProject/staging\n\n<details>\n<summary>Pulumi report</summary>\n\n\n<pre>\ntest\n</pre>\n\n</details>',
});
});
});
26 changes: 19 additions & 7 deletions src/libs/pr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,13 @@ import { Config } from '../config';
function ansiToHtml(
message: string,
maxLength: number,
alwaysIncludeSummary: boolean,
): [string, boolean] {
/**
* Converts an ansi string to html by for example removing color escape characters.
* message: ansi string to convert
* maxLength: Maximum number of characters of final message incl. HTML tags
* alwaysIncludeSummary: if true, trim message from front (if trimming is needed), otherwise from end
*
* return message as html and information if message was trimmed because of length
*/
Expand All @@ -24,9 +26,15 @@ function ansiToHtml(
// Check if htmlBody exceeds max characters
if (htmlBody.length > maxLength) {

// trim input message by number of exceeded characters
// trim input message by number of exceeded characters from front or back as configured
const dif: number = htmlBody.length - maxLength;
message = message.substring(0, message.length - dif);

if (alwaysIncludeSummary) {
message = message.substring(dif, htmlBody.length);
} else {
message = message.substring(0, message.length - dif);
}

trimmed = true;

// convert trimmed message to html
Expand All @@ -46,6 +54,7 @@ export async function handlePullRequestMessage(
command,
stackName,
editCommentOnPr,
alwaysIncludeSummary,
} = config;

// GitHub limits comment characters to 65535, use lower max to keep buffer for variable values
Expand All @@ -55,21 +64,24 @@ export async function handlePullRequestMessage(

const summary = '<summary>Pulumi report</summary>';

const [htmlBody, trimmed]: [string, boolean] = ansiToHtml(output, MAX_CHARACTER_COMMENT);
const [htmlBody, trimmed]: [string, boolean] = ansiToHtml(output, MAX_CHARACTER_COMMENT, alwaysIncludeSummary);

const body = dedent`
${heading}
<details>
${summary}
${alwaysIncludeSummary
? ':warning: **Warn**: The output was too long and trimmed from the front.'
: ''
}
<pre>
${htmlBody}
</pre>
${
trimmed
? ':warning: **Warn**: The output was too long and trimmed.'
: ''
${trimmed && !alwaysIncludeSummary
? ':warning: **Warn**: The output was too long and trimmed.'
: ''
}
</details>
`;
Expand Down

0 comments on commit 1fa1109

Please sign in to comment.