From 45c991e2ac19a7c2f1c15df59d4bb73c4c1c478c Mon Sep 17 00:00:00 2001 From: Danielle Adams Date: Mon, 26 Feb 2024 16:49:09 -0700 Subject: [PATCH] ARCH-2011 - Re-work the expected PR comment assertions & add expected file for truncated scenarios --- .github/workflows/build-and-review-pr.yml | 91 ++++--- test/assert-pr-comment-exists.js | 9 +- .../assert-pr-comment-matches-expectations.js | 59 +++- test/assert-status-check-exists.js | 6 +- ...ssert-status-check-matches-expectations.js | 18 +- test/delete-pr-comment.js | 6 +- test/delete-pre-existing-comments.js | 66 +++++ test/files/expected-pr-comment-truncated.md | 252 ++++++++++++++++++ test/files/expected-pr-comment.md | 49 ++++ test/files/truncate.json | 114 +------- test/update-failing-status-check.js | 10 +- 11 files changed, 502 insertions(+), 178 deletions(-) create mode 100644 test/delete-pre-existing-comments.js create mode 100644 test/files/expected-pr-comment-truncated.md create mode 100644 test/files/expected-pr-comment.md diff --git a/.github/workflows/build-and-review-pr.yml b/.github/workflows/build-and-review-pr.yml index 69197e6..cde2a4c 100644 --- a/.github/workflows/build-and-review-pr.yml +++ b/.github/workflows/build-and-review-pr.yml @@ -419,7 +419,7 @@ jobs: const assertStatusCheckMatchesExpectations = require('./test/assert-status-check-matches-expectations.js'); const checkId = '${{ steps.no-failures.outputs.status-check-id }}'; - const actualCheck = await assertStatusCheckExists(github, core, checkId); + const actualCheck = await assertStatusCheckExists(github, context, core, checkId); const expectedBody = fs.readFileSync('${{ env.NO_FAILURES_MD_FILE }}', 'utf8'); const expectedValues = { @@ -470,7 +470,7 @@ jobs: const assertStatusCheckMatchesExpectations = require('./test/assert-status-check-matches-expectations.js'); const checkId = '${{ steps.ignore-failures.outputs.status-check-id }}'; - const actualCheck = await assertStatusCheckExists(github, core, checkId); + const actualCheck = await assertStatusCheckExists(github, context, core, checkId); const expectedBody = fs.readFileSync('${{ env.IGNORE_FAILURES_MD_FILE }}', 'utf8'); const expectedValues = { @@ -521,7 +521,7 @@ jobs: const assertStatusCheckMatchesExpectations = require('./test/assert-status-check-matches-expectations.js'); const checkId = '${{ steps.allow-failures.outputs.status-check-id }}'; - const actualCheck = await assertStatusCheckExists(github, core, checkId); + const actualCheck = await assertStatusCheckExists(github, context, core, checkId); const expectedBody = fs.readFileSync('${{ env.ALLOW_FAILURES_MD_FILE }}', 'utf8'); const expectedValues = { @@ -546,7 +546,7 @@ jobs: const fs = require('fs'); const updateFailingStatusCheck = require('./test/update-failing-status-check.js'); - await updateFailingStatusCheck(github, core, '${{ steps.allow-failures.outputs.status-check-id }}'); + await updateFailingStatusCheck(github, context, core, '${{ steps.allow-failures.outputs.status-check-id }}'); - name: '-------------------------------------------------------------------------------------------------------------' run: echo "" @@ -560,12 +560,11 @@ jobs: TRUNCATE_TESTS_JSON: './test/files/truncate.json' PASSING_TESTS_JSON: './test/files/passing.json' - NO_FAILURES_MD_FILE: './test/files/expected-check-results-no-failures.md' - IGNORE_FAILURES_MD_FILE: './test/files/expected-check-results-ignore-failures.md' - ALLOW_FAILURES_MD_FILE: './test/files/expected-check-results-allow-failures.md' + NO_FAILURES_MD_FILE: './test/files/expected-pr-comment.md' + TRUNCATE_MD_FILE: './test/files/expected-pr-comment-truncated.md' - NO_FAILURES_REPORT_NAME: 'No Failures Scenario' - TRUNCATE_FAILURES_REPORT_NAME: 'Truncate Failures Scenario' + NO_FAILURES_REPORT_NAME: 'No Failures for PR Comment Scenario' + TRUNCATE_FAILURES_REPORT_NAME: 'Truncate Failures for PR Comment Scenario' @@ -585,7 +584,15 @@ jobs: - name: Setup - Checkout the action uses: actions/checkout@v4 - - name: Setup - Create a comment that can be updated + - name: Setup - Delete pre-existing process-jest-test-results PR Comments + if: always() + uses: actions/github-script@v7 + with: + script: | + const deletePrComments = require('./test/delete-pre-existing-comments.js'); + await deletePrComments(github, context, core); + + - name: Setup - Create a process-jest-test-results comment that can be updated if: always() uses: actions/github-script@v7 with: @@ -598,6 +605,7 @@ jobs: body: `\nThis comment will be replaced soon.` }) .then(response => { + core.info(`The 'existing' process-jest-test-results comment has id: ${response.data.id}`); core.exportVariable('EXISTING_COMMENT_ID', response.data.id); }) .catch(error => { @@ -645,13 +653,17 @@ jobs: const assertCommentMatchesExpectations = require('./test/assert-pr-comment-matches-expectations.js'); const commentId = '${{ steps.update-with-matching-prefix.outputs.pr-comment-id }}'; - const actualComment = await assertCommentExists(github, core, commentId); + const actualComment = await assertCommentExists(github, context, core, commentId); - const expectedBody = fs.readFileSync('${{ steps.update-with-matching-prefix.outputs.test-results-file-path }}', 'utf8'); - const expectedPrefix = ''; + const expectedBody = fs.readFileSync('${{ env.NO_FAILURES_MD_FILE }}', 'utf8'); + const testResultsBody = fs.readFileSync('${{ steps.update-with-matching-prefix.outputs.test-results-file-path }}', 'utf8'); + const expectedComment = { - prefixAndBody: `${expectedPrefix}\n${expectedBody}`, - action: 'updated' + expectedPrefix: '', + expectedBody: expectedBody, + actualTestResults: testResultsBody, + action: 'updated', + truncated: false }; assertCommentMatchesExpectations(core, actualComment, expectedComment); @@ -695,13 +707,17 @@ jobs: const assertCommentMatchesExpectations = require('./test/assert-pr-comment-matches-expectations.js'); const commentId = '${{ steps.update-without-matching-prefix.outputs.pr-comment-id }}'; - const actualComment = await assertCommentExists(github, core, commentId); + const actualComment = await assertCommentExists(github, context, core, commentId); - const expectedBody = fs.readFileSync('${{ steps.update-without-matching-prefix.outputs.test-results-file-path }}', 'utf8'); - const expectedPrefix = ''; + const expectedBody = fs.readFileSync('${{ env.NO_FAILURES_MD_FILE }}', 'utf8'); + const testResultsBody = fs.readFileSync('${{ steps.update-without-matching-prefix.outputs.test-results-file-path }}', 'utf8'); + const expectedComment = { - prefixAndBody: `${expectedPrefix}\n${expectedBody}`, - action: 'created' + expectedPrefix: '', + expectedBody: expectedBody, + actualTestResults: testResultsBody, + action: 'created', + truncated: false }; assertCommentMatchesExpectations(core, actualComment, expectedComment); @@ -744,13 +760,17 @@ jobs: const assertCommentMatchesExpectations = require('./test/assert-pr-comment-matches-expectations.js'); const commentId = '${{ steps.matching-prefix-no-update.outputs.pr-comment-id }}'; - const actualComment = await assertCommentExists(github, core, commentId); + const actualComment = await assertCommentExists(github, context, core, commentId); - const expectedBody = fs.readFileSync('${{ steps.matching-prefix-no-update.outputs.test-results-file-path }}', 'utf8'); - const expectedPrefix = ''; + const expectedBody = fs.readFileSync('${{ env.NO_FAILURES_MD_FILE }}', 'utf8'); + const testResultsBody = fs.readFileSync('${{ steps.matching-prefix-no-update.outputs.test-results-file-path }}', 'utf8'); + const expectedComment = { - prefixAndBody: `${expectedPrefix}\n${expectedBody}`, - action: 'created' + expectedPrefix: '', + expectedBody: expectedBody, + actualTestResults: testResultsBody, + action: 'created', + truncated: false }; assertCommentMatchesExpectations(core, actualComment, expectedComment); @@ -794,15 +814,18 @@ jobs: const assertCommentMatchesExpectations = require('./test/assert-pr-comment-matches-expectations.js'); const commentId = '${{ steps.truncate.outputs.pr-comment-id }}'; - const actualComment = await assertCommentExists(github, core, commentId); + const actualComment = await assertCommentExists(github, context, core, commentId); - const expectedBody = fs.readFileSync('${{ steps.truncate.outputs.test-results-file-path }}', 'utf8'); - const expectedPrefix = ''; + const expectedBody = fs.readFileSync('${{ env.TRUNCATE_MD_FILE }}', 'utf8'); + const testResultsBody = fs.readFileSync('${{ steps.truncate.outputs.test-results-file-path }}', 'utf8'); + const expectedComment = { - prefixAndBody: `${expectedPrefix}\nTest results truncated due to character limit. See full report in output. \n${expectedBody}`, - action: 'updated' + expectedPrefix: '\nTest results truncated due to character limit. See full report in output.\n', + expectedBody: expectedBody, + actualTestResults: testResultsBody, + action: 'updated', + truncated: true }; - assertCommentMatchesExpectations(core, actualComment, expectedComment); - name: '-------------------------------------------------------------------------------------------------------------' @@ -818,9 +841,9 @@ jobs: const fs = require('fs'); const deletePrComment = require('./test/delete-pr-comment.js'); - await deletePrComment(github, core, '${{ env.EXISTING_COMMENT_ID }}'); - await deletePrComment(github, core, '${{ steps.matching-prefix-no-update.outputs.pr-comment-id }}'); - await deletePrComment(github, core, '${{ steps.update-without-matching-prefix.outputs.pr-comment-id }}'); + await deletePrComment(github, context, core, '${{ env.EXISTING_COMMENT_ID }}'); + await deletePrComment(github, context, core, '${{ steps.matching-prefix-no-update.outputs.pr-comment-id }}'); + await deletePrComment(github, context, core, '${{ steps.update-without-matching-prefix.outputs.pr-comment-id }}'); - name: '-------------------------------------------------------------------------------------------------------------' run: echo "" \ No newline at end of file diff --git a/test/assert-pr-comment-exists.js b/test/assert-pr-comment-exists.js index d15614b..f01d1ca 100644 --- a/test/assert-pr-comment-exists.js +++ b/test/assert-pr-comment-exists.js @@ -1,4 +1,4 @@ -module.exports = async (github, core, commentId) => { +module.exports = async (github, context, core, commentId) => { core.info(`\nAsserting that PR Comment with the following id exists: '${commentId}'`); let actualComment; @@ -8,8 +8,8 @@ module.exports = async (github, core, commentId) => { } const commentResponse = await github.rest.issues.getComment({ - owner: 'im-open', - repo: 'process-jest-test-results', + owner: context.repo.owner, + repo: context.repo.repo, comment_id: commentId.trim() }); @@ -26,8 +26,9 @@ module.exports = async (github, core, commentId) => { updatedAt: rawComment.updated_at, issueUrl: rawComment.issue_url }; - core.info(`Comment ${actualComment.id} details:`); + core.startGroup(`Comment ${actualComment.id} details:`); console.log(actualComment); + core.endGroup(); } return actualComment; diff --git a/test/assert-pr-comment-matches-expectations.js b/test/assert-pr-comment-matches-expectations.js index 886c603..f587b50 100644 --- a/test/assert-pr-comment-matches-expectations.js +++ b/test/assert-pr-comment-matches-expectations.js @@ -1,4 +1,25 @@ module.exports = async (core, comment, expectedValues) => { + function assertLengthsAreTheSame(prCommentLength, testResultsMdLength) { + core.info(`\n\tPR Comment length:\t\t'${prCommentLength}'`); + core.info(`\ttest-results.md length: '${testResultsMdLength}'`); + + if (prCommentLength != testResultsMdLength) { + core.setFailed(`\tThe lengths do not match, which is not expected.`); + } else { + core.info(`\tThe lengths match, which is expected.`); + } + } + function assertLengthsAreNotTheSame(prCommentLength, testResultsMdLength) { + core.info(`\n\tPR Comment length:\t\t'${prCommentLength}'`); + core.info(`\ttest-results.md length: '${testResultsMdLength}'`); + + if (prCommentLength != testResultsMdLength) { + core.info(`\tThe lengths do not match, which is expected.`); + } else { + core.setFailed(`\tThe lengths match, which is not expected.`); + } + } + function assertCreatedAndUpdatedMatch(created, updated) { core.info(`\n\tCreated: '${created}'`); core.info(`\tUpdated: '${updated}'`); @@ -21,24 +42,45 @@ module.exports = async (core, comment, expectedValues) => { } } - function assertValueContainsSubstring(variableName, value, substring) { - core.startGroup(`\n\tChecking ${variableName} contains the substring.`); + function assertValueContainsSubstring(valueName, value, substringName, substring) { if (value.includes(substring)) { - core.info(`\tThe ${variableName} string contains the substring.`); + core.info(`\tChecking ${valueName} contains the ${substringName} substring.`); + core.info(`\tThe ${valueName} string contains the substring.`); } else { - core.setFailed(`\tThe ${variableName} string does not contain the substring.`); - core.info(`\n\tExpected ${variableName}: '${value}'`); - core.info(`\tActual ${variableName}: '${substring}'`); + core.info(`\tChecking ${valueName} contains the ${substringName} substring.`); + core.setFailed(`\tThe ${valueName} string does not contain the ${substringName} substring.`); + core.startGroup('\tString and substring Details'); + core.info(`\n\t${valueName}: '${value}'`); + core.info(`\t${substringName}: '${substring}'`); + core.endGroup(); } - core.endGroup(); } function validateProps() { core.info(`\nAsserting that PR Comment properties match the expected values.`); core.info(`Comment ID: ${comment.id}`); - assertValueContainsSubstring('Body', expectedValues['prefixAndBody'], comment.body); + const expectedPrefix = expectedValues.expectedPrefix; + const expectedBody = expectedValues.expectedBody; + const actualTestResultsMd = expectedValues.actualTestResults; + const actualTestResultsMdWithPrefix = `${expectedPrefix}\n${actualTestResultsMd}`; + const actualComment = comment.body; + + // The actual comment body should contain the expected prefix and the expected body + assertValueContainsSubstring('PR Comment', actualComment, 'Expected Prefix', expectedPrefix); + assertValueContainsSubstring('PR Comment', actualComment, 'Expected Body', expectedBody); + + // The test-results.md file is the whole markdown before truncation so + // it should contain the substring of the actual comment + assertValueContainsSubstring('test-results.md', actualTestResultsMdWithPrefix, 'Actual Comment Body', actualComment); + + if (expectedValues.truncated) { + assertLengthsAreNotTheSame(actualComment.length, actualTestResultsMdWithPrefix.length); + } else { + assertLengthsAreTheSame(actualComment.length, actualTestResultsMdWithPrefix.length); + } + // Doublecheck the timestamps are generally what we expected based on created/updated status switch (expectedValues.action) { case 'updated': assertUpdatedIsAfterCreated(comment.createdAt, comment.updatedAt); @@ -53,5 +95,4 @@ module.exports = async (core, comment, expectedValues) => { } validateProps(); - await new Promise(r => setTimeout(r, 5 * 1000)); }; diff --git a/test/assert-status-check-exists.js b/test/assert-status-check-exists.js index b3172ae..3fb16cc 100644 --- a/test/assert-status-check-exists.js +++ b/test/assert-status-check-exists.js @@ -1,4 +1,4 @@ -module.exports = async (github, core, statusCheckId) => { +module.exports = async (github, context, core, statusCheckId) => { core.info(`\nAsserting that status check '${statusCheckId} exists`); if (!statusCheckId || statusCheckId.trim() === '') { @@ -9,8 +9,8 @@ module.exports = async (github, core, statusCheckId) => { let statusCheckToReturn; await github.rest.checks .get({ - owner: 'im-open', - repo: 'process-jest-test-results', + owner: context.repo.owner, + repo: context.repo.repo, check_run_id: statusCheckId.trim() }) .then(checkResponse => { diff --git a/test/assert-status-check-matches-expectations.js b/test/assert-status-check-matches-expectations.js index d5a7fc5..15dfb0f 100644 --- a/test/assert-status-check-matches-expectations.js +++ b/test/assert-status-check-matches-expectations.js @@ -10,16 +10,18 @@ module.exports = async (core, statusCheck, expectedValues) => { } } - function assertValueContainsSubstring(variableName, value, substring) { - core.startGroup(`\tChecking ${variableName} contains the substring.`); + function assertValueContainsSubstring(valueName, value, substringName, substring) { if (value.includes(substring)) { - core.info(`\tThe ${variableName} string contains the substring.`); + core.info(`\tChecking ${valueName} contains the ${substringName} substring.`); + core.info(`\tThe ${valueName} string contains the substring.`); } else { - core.setFailed(`\tThe ${variableName} string does not contain the substring.`); - core.info(`\n\tExpected ${variableName}: '${value}'`); - core.info(`\tActual ${variableName}: '${substring}'`); + core.info(`\tChecking ${valueName} contains the ${substringName} substring.`); + core.setFailed(`\tThe ${valueName} string does not contain the ${substringName} substring.`); + core.startGroup('\tString and substring Details'); + core.info(`\n\t${valueName}: '${value}'`); + core.info(`\t${substringName}: '${substring}'`); + core.endGroup(); } - core.endGroup(); } function validateProps() { @@ -34,7 +36,7 @@ module.exports = async (core, statusCheck, expectedValues) => { // The summary should be something like: 'This test run completed at `Wed, 21 Feb 2024 20:21:48 GMT`' // so just check that it contains the static portion. - assertValueContainsSubstring('Summary', statusCheck.summary, 'This test run completed at `'); + assertValueContainsSubstring('Summary', statusCheck.summary, 'Partial Test Run Text', 'This test run completed at `'); } validateProps(); diff --git a/test/delete-pr-comment.js b/test/delete-pr-comment.js index 4cbe121..59f6915 100644 --- a/test/delete-pr-comment.js +++ b/test/delete-pr-comment.js @@ -1,4 +1,4 @@ -module.exports = async (github, core, commentId) => { +module.exports = async (github, context, core, commentId) => { core.info(`\nDeleting comment '${commentId}'`); if (!commentId) { @@ -7,8 +7,8 @@ module.exports = async (github, core, commentId) => { await github .request(`DELETE /repos/{owner}/{repo}/issues/comments/{comment_id}`, { - owner: 'im-open', - repo: 'process-jest-test-results', + owner: context.repo.owner, + repo: context.repo.repo, comment_id: commentId, headers: { 'X-GitHub-Api-Version': '2022-11-28' diff --git a/test/delete-pre-existing-comments.js b/test/delete-pre-existing-comments.js new file mode 100644 index 0000000..6e94b48 --- /dev/null +++ b/test/delete-pre-existing-comments.js @@ -0,0 +1,66 @@ +module.exports = async (github, context, core) => { + async function lookForExistingComments(github, context, core, prNum) { + const markupPrefix = `