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

Trim PR comments from the front. #1252

Merged
merged 7 commits into from
Aug 22, 2024
Merged
Show file tree
Hide file tree
Changes from 6 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
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.

- `trim-comments-from-front` - (optional) If `true`, then the action will trim
devonmoss marked this conversation as resolved.
Show resolved Hide resolved
long pr comments from the front instead of the back. This ensures that the
resources summary is always visible.

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'
trim-comments-from-front:
description: 'If comments must be trimmed, trim them from the front. This ensures the resources summary is included in the comment'
required: false
default: 'false'
outputs:
output:
description: Output from running command
Expand Down
2 changes: 2 additions & 0 deletions 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',
'trim-comments-from-front': 'false',
upsert: 'false',
remove: 'false',
refresh: 'false',
Expand Down Expand Up @@ -69,6 +70,7 @@ describe('config.ts', () => {
"remove": false,
"secretsProvider": "",
"stackName": "dev",
"trimCommentsFromFront": false,
"upsert": false,
"workDir": "./",
}
Expand Down
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'),
trimCommentsFromFront: getBooleanInput('trim-comments-from-front'),

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 trimFromFrontOptions = {
command: 'preview',
stackName: 'staging',
trimCommentsFromFront: true,
options: {},
} as Config;

await handlePullRequestMessage(
trimFromFrontOptions,
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>',
devonmoss marked this conversation as resolved.
Show resolved Hide resolved
});
});
});
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,
trimCommentsFromFront: 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
* trimCommentsFromFront: 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 (trimCommentsFromFront) {
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,
trimCommentsFromFront,
} = 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, trimCommentsFromFront);

const body = dedent`
${heading}

<details>
${summary}

${trimCommentsFromFront
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the reason why I added an extra new line. Previously, when the output was trimmed it would add a warning message to the end of the comment. My change makes it so in the event that we trim from the front it will add the warning message near the beginning of the comment, right before the trimmed output.

The blank line above was not there during the first pass I made and the tests passed without the addition of the extra \n to the tests. However when I used it in a project the markdown doesn't format properly. Instead of:

⚠️ Warn: The output was too long and trimmed from the front.

It did:

⚠️ **Warn**: The output was too long and trimmed from the front.

? ':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 && !trimCommentsFromFront
? ':warning: **Warn**: The output was too long and trimmed.'
: ''
}
</details>
`;
Expand Down