-
Notifications
You must be signed in to change notification settings - Fork 85
feat: Remove ANSI control chars from GitHub step Summary and PR comment #1312
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
Merged
Merged
Changes from 21 commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
815f5ce
strip ansi for PR and Summary
byian dffd651
update heading for summary
byian 186a0d4
update PR test
byian 6ac143b
update PR tests & add Summary tests
byian ae128ed
remove 'strip-ansi'
byian 79e3931
Merge pull request #2 from pulumi/main
byian d1d35d1
dist update
byian 11e7ebd
add dedent to summary
byian dbf320a
try fix dedent
byian 47ad8aa
build fix
byian e4c3b3e
addRaw
byian 9d0ba78
respect leading spaces
byian 3739e96
build
byian c46f4ba
remove dedent
byian 3c7c355
update CHANGELOG
byian 1738100
revert dist
byian c0d38d6
update CHANGELOG
byian d66b130
Merge branch 'main' into remove_ansi_from_summary
byian 684fff6
Update CHANGELOG.md
byian d9f2ec2
fix typo
byian 75617a0
fix broken logic
byian 6da2659
Merge branch 'main' into remove_ansi_from_summary
byian 0b87680
fix: trimOutput functions renamed
byian File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,112 @@ | ||
| import * as fs from 'fs' | ||
| import path from 'path' | ||
| import * as core from '@actions/core'; | ||
| import { SUMMARY_ENV_VAR } from '@actions/core/lib/summary' | ||
| import { Config } from '../../config'; | ||
| import { handleSummaryMessage } from '../summary'; | ||
|
|
||
| const testDirectoryPath = path.join(__dirname, 'test') | ||
| const testFilePath = path.join(testDirectoryPath, 'test-summary.md') | ||
|
|
||
| async function getSummary(): Promise<string> { | ||
| const file = await fs.promises.readFile(testFilePath, {encoding: 'utf8'}); | ||
| return file.replace(/\r\n/g, '\n'); | ||
| } | ||
|
|
||
| const projectName = 'myFirstProject'; | ||
| const defaultOptions = { | ||
| command: 'preview', | ||
| stackName: 'staging', | ||
| options: {}, | ||
| } as Config; | ||
|
|
||
| describe('summary.ts', () => { | ||
| beforeEach(async () => { | ||
| process.env[SUMMARY_ENV_VAR] = testFilePath; | ||
| await fs.promises.mkdir(testDirectoryPath, {recursive: true}); | ||
| await fs.promises.writeFile(testFilePath, '', {encoding: 'utf8'}); | ||
| core.summary.emptyBuffer(); | ||
| }) | ||
|
|
||
| afterAll(async () => { | ||
| await fs.promises.unlink(testFilePath); | ||
| }) | ||
|
|
||
| it('throws if summary env var is undefined', async () => { | ||
| process.env[SUMMARY_ENV_VAR] = undefined; | ||
| const write = core.summary.addRaw('123').write(); | ||
| await expect(write).rejects.toThrow(); | ||
| }) | ||
|
|
||
| it('throws if summary file does not exist', async () => { | ||
| await fs.promises.unlink(testFilePath); | ||
| const write = core.summary.addRaw('123').write(); | ||
| await expect(write).rejects.toThrow(); | ||
| }) | ||
|
|
||
| it('should add only heading with empty code block to summary', async () => { | ||
| const message = ''; | ||
| const expected = `<h1>Pulumi ${projectName}/${defaultOptions.stackName} results</h1>\n<pre lang="diff"><code></pre>\n`; | ||
|
|
||
| await handleSummaryMessage(defaultOptions, projectName, message); | ||
| const summary = await getSummary() | ||
|
|
||
| expect(summary).toBe(expected); | ||
| }) | ||
|
|
||
| it('should convert ansi control character to plain text and add to summary', async () => { | ||
| const message = '\x1b[30mblack\x1b[37mwhite'; | ||
| const expected = `<h1>Pulumi ${projectName}/${defaultOptions.stackName} results</h1>\n<pre lang="diff"><code>blackwhite</code></pre>\n`; | ||
|
|
||
| await handleSummaryMessage(defaultOptions, projectName, message); | ||
| const summary = await getSummary() | ||
|
|
||
| expect(summary).toBe(expected); | ||
| }) | ||
|
|
||
| it('should trim the output when the output is larger than 1 MiB', async () => { | ||
| const message = 'this is at the begining and should not be in the output' + 'a'.repeat(1_048_576) + 'this is at the end and should be in the output'; | ||
|
|
||
| await handleSummaryMessage(defaultOptions, projectName, message); | ||
| const summary = await getSummary() | ||
|
|
||
| expect(Buffer.byteLength(summary, 'utf8')).toBeLessThan(1_048_576); | ||
| expect(summary).toContain('this is at the begining and should not be in the output') | ||
| expect(summary).toContain('The output was too long and trimmed.'); | ||
| expect(summary).not.toContain('this is at the end and should be in the output'); | ||
| expect(summary).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 1 MiB and config is set', async () => { | ||
| const message = 'this is at the begining and should not be in the output' + 'a'.repeat(1_048_576) + 'this is at the end and should be in the output'; | ||
|
|
||
| const options: Config = { | ||
| ...defaultOptions, | ||
| alwaysIncludeSummary: true, | ||
| }; | ||
|
|
||
| await handleSummaryMessage(options, projectName, message); | ||
| const summary = await getSummary() | ||
|
|
||
| expect(Buffer.byteLength(summary, 'utf8')).toBeLessThan(1_048_576); | ||
| expect(summary).toContain('this is at the end and should be in the output'); | ||
| expect(summary).toContain('The output was too long and trimmed from the front.'); | ||
| expect(summary).not.toContain('this is at the begining and should not be in the output') | ||
| expect(summary).not.toContain('The output was too long and trimmed.'); | ||
| }) | ||
|
|
||
| it('should replace first leading space with non-breaking space character to preserve the formatting', async () => { | ||
| const message = | ||
| `begin | ||
| some leading spaces in this line | ||
| `; | ||
|
|
||
| const expected = `<h1>Pulumi ${projectName}/${defaultOptions.stackName} results</h1>\n<pre lang="diff"><code>begin\n some leading spaces in this line\n</code></pre>\n`; | ||
|
|
||
| await handleSummaryMessage(defaultOptions, projectName, message); | ||
| const summary = await getSummary() | ||
|
|
||
| expect(summary).toBe(expected); | ||
| }) | ||
|
|
||
| }) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,74 @@ | ||
| import * as core from '@actions/core'; | ||
| import { Config } from '../config'; | ||
|
|
||
| function trimOutput( | ||
| message: string, | ||
| maxSize: number, | ||
| alwaysIncludeSummary: boolean, | ||
| ): [string, boolean] { | ||
| /** | ||
| * Trim message to maxSize in bytes | ||
| * message: string to trim | ||
| * maxSize: Maximum number of bytes of final message | ||
| * alwaysIncludeSummary: if true, trim message from front (if trimming is needed), otherwise from end | ||
| * | ||
| * return message and information if message was trimmed | ||
| */ | ||
| let trimmed = false; | ||
|
|
||
| const messageSize = Buffer.byteLength(message, 'utf8'); | ||
|
|
||
| // Check if message exceeds max size | ||
| if (messageSize > maxSize) { | ||
|
|
||
| // Trim input message by number of exceeded bytes from front or back as configured | ||
| const dif: number = messageSize - maxSize; | ||
|
|
||
| if (alwaysIncludeSummary) { | ||
| message = Buffer.from(message).subarray(dif, messageSize).toString() | ||
| } else { | ||
| message = Buffer.from(message).subarray(0, messageSize - dif).toString() | ||
| } | ||
|
|
||
| trimmed = true; | ||
| } | ||
|
|
||
| return [message, trimmed]; | ||
| } | ||
|
|
||
| export async function handleSummaryMessage( | ||
| config: Config, | ||
| projectName: string, | ||
| output: string, | ||
| ): Promise<void> { | ||
| const { | ||
| stackName, | ||
| alwaysIncludeSummary, | ||
| } = config; | ||
|
|
||
| // Remove ANSI symbols from output because they are not supported in GitHub step Summary | ||
| const regex_ansi = RegExp(`\x1B(?:[@-Z\\-_]|[[0-?]*[ -/]*[@-~])`, 'g'); | ||
| output = output.replace(regex_ansi, ''); | ||
|
|
||
| // Replace the first leading space in each line with a non-breaking space character to preserve the formatting | ||
| const regex_space = RegExp(`^[ ]`, 'gm'); | ||
| output = output.replace(regex_space, ' '); | ||
|
|
||
| // GitHub limits step Summary to 1 MiB (1_048_576 bytes), use lower max to keep buffer for variable values | ||
| const MAX_SUMMARY_SIZE_BYTES = 1_000_000; | ||
|
|
||
| const [message, trimmed]: [string, boolean] = trimOutput(output, MAX_SUMMARY_SIZE_BYTES, alwaysIncludeSummary); | ||
|
|
||
| let heading = `Pulumi ${projectName}/${stackName} results`; | ||
|
|
||
| if (trimmed && alwaysIncludeSummary) { | ||
| heading += ' :warning: **Warn**: The output was too long and trimmed from the front.'; | ||
| } else if (trimmed && !alwaysIncludeSummary) { | ||
| heading += ' :warning: **Warn**: The output was too long and trimmed.'; | ||
| } | ||
|
|
||
| await core.summary | ||
| .addHeading(heading) | ||
| .addCodeBlock(message, "diff") | ||
| .write(); | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why trimOutput here and in pr.ts? Looks like one of these should be removed they're doing the same thing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically, they do the same thing, yes, they trim the input string to the desired format, but for pr and summary, the formats are different.
Summary has a limit on the maximum message size in bytes, and pr has a limit on the maximum length in characters.
Maybe I should use the different names to avoid confusion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done, ready for re-review