From 1d23b5d1de0b1b373f507d2e9407e104a171bd48 Mon Sep 17 00:00:00 2001 From: Alexander Bayandin Date: Tue, 4 Apr 2023 12:22:47 +0100 Subject: [PATCH] Comment PR with test results (#3907) This PR adds posting a comment with test results. Each workflow run updates the comment with new results. The layout and the information that we post can be changed to our needs, right now, it contains failed tests and test which changes status after rerun (i.e. flaky tests) --- .github/actions/allure-report/action.yml | 44 ++++++-- .github/workflows/build_and_test.yml | 82 +++++++++++---- scripts/pr-comment-test-report.js | 125 +++++++++++++++++++++++ 3 files changed, 222 insertions(+), 29 deletions(-) create mode 100644 scripts/pr-comment-test-report.js diff --git a/.github/actions/allure-report/action.yml b/.github/actions/allure-report/action.yml index e685006245..e35cbb20fd 100644 --- a/.github/actions/allure-report/action.yml +++ b/.github/actions/allure-report/action.yml @@ -15,10 +15,32 @@ outputs: report-url: description: 'Allure report URL' value: ${{ steps.generate-report.outputs.report-url }} + report-json-url: + description: 'Allure report JSON URL' + value: ${{ steps.generate-report.outputs.report-json-url }} runs: using: "composite" + steps: + # We're using some of env variables quite offen, so let's set them once. + # + # It would be nice to have them set in common runs.env[0] section, but it doesn't work[1] + # + # - [0] https://docs.github.com/en/actions/creating-actions/metadata-syntax-for-github-actions#runsenv + # - [1] https://github.com/neondatabase/neon/pull/3907#discussion_r1154703456 + # + - name: Set common environment variables + shell: bash -euxo pipefail {0} + run: | + echo "BUILD_TYPE=${BUILD_TYPE}" >> $GITHUB_ENV + echo "BUCKET=${BUCKET}" >> $GITHUB_ENV + echo "TEST_OUTPUT=${TEST_OUTPUT}" >> $GITHUB_ENV + env: + BUILD_TYPE: ${{ inputs.build_type }} + BUCKET: neon-github-public-dev + TEST_OUTPUT: /tmp/test_output + - name: Validate input parameters shell: bash -euxo pipefail {0} run: | @@ -84,8 +106,6 @@ runs: env: REPORT_PREFIX: reports/${{ steps.calculate-vars.outputs.KEY }}/${{ inputs.build_type }} RAW_PREFIX: reports-raw/${{ steps.calculate-vars.outputs.KEY }}/${{ inputs.build_type }} - TEST_OUTPUT: /tmp/test_output - BUCKET: neon-github-public-dev TEST_SELECTION: ${{ steps.calculate-vars.outputs.TEST_SELECTION }} shell: bash -euxo pipefail {0} run: | @@ -104,7 +124,7 @@ runs: EOF cat < $TEST_OUTPUT/allure/results/environment.properties TEST_SELECTION=${{ inputs.test_selection }} - BUILD_TYPE=${{ inputs.build_type }} + BUILD_TYPE=${BUILD_TYPE} EOF ARCHIVE="${GITHUB_RUN_ID}-${TEST_SELECTION}-${GITHUB_RUN_ATTEMPT}-$(date +%s).tar.zst" @@ -113,13 +133,12 @@ runs: tar -C ${TEST_OUTPUT}/allure/results -cf ${ARCHIVE} --zstd . aws s3 mv --only-show-errors ${ARCHIVE} "s3://${BUCKET}/${RAW_PREFIX}/${ARCHIVE}" - # Potentially we could have several running build for the same key (for example for the main branch), so we use improvised lock for this + # Potentially we could have several running build for the same key (for example for the main branch), so we use improvised lock for this - name: Acquire Allure lock if: ${{ inputs.action == 'generate' }} shell: bash -euxo pipefail {0} env: LOCK_FILE: reports/${{ steps.calculate-vars.outputs.KEY }}/lock.txt - BUCKET: neon-github-public-dev TEST_SELECTION: ${{ steps.calculate-vars.outputs.TEST_SELECTION }} run: | LOCK_TIMEOUT=300 # seconds @@ -149,8 +168,6 @@ runs: env: REPORT_PREFIX: reports/${{ steps.calculate-vars.outputs.KEY }}/${{ inputs.build_type }} RAW_PREFIX: reports-raw/${{ steps.calculate-vars.outputs.KEY }}/${{ inputs.build_type }} - TEST_OUTPUT: /tmp/test_output - BUCKET: neon-github-public-dev shell: bash -euxo pipefail {0} run: | # Get previously uploaded data for this run @@ -186,24 +203,24 @@ runs: REPORT_URL=https://${BUCKET}.s3.amazonaws.com/${REPORT_PREFIX}/${GITHUB_RUN_ID}/index.html # Generate redirect - cat < ./index.html + cat < ${TEST_OUTPUT}/allure/index.html Redirecting to ${REPORT_URL} EOF - aws s3 cp --only-show-errors ./index.html "s3://${BUCKET}/${REPORT_PREFIX}/latest/index.html" + aws s3 cp --only-show-errors ${TEST_OUTPUT}/allure/index.html "s3://${BUCKET}/${REPORT_PREFIX}/latest/index.html" echo "[Allure Report](${REPORT_URL})" >> ${GITHUB_STEP_SUMMARY} echo "report-url=${REPORT_URL}" >> $GITHUB_OUTPUT + echo "report-json-url=${REPORT_URL%/index.html}/data/suites.json" >> $GITHUB_OUTPUT - name: Release Allure lock if: ${{ inputs.action == 'generate' && always() }} shell: bash -euxo pipefail {0} env: LOCK_FILE: reports/${{ steps.calculate-vars.outputs.KEY }}/lock.txt - BUCKET: neon-github-public-dev TEST_SELECTION: ${{ steps.calculate-vars.outputs.TEST_SELECTION }} run: | aws s3 cp --only-show-errors "s3://${BUCKET}/${LOCK_FILE}" ./lock.txt || exit 0 @@ -212,11 +229,16 @@ runs: aws s3 rm "s3://${BUCKET}/${LOCK_FILE}" fi + - name: Cleanup + if: always() + shell: bash -euxo pipefail {0} + run: | + rm -rf ${TEST_OUTPUT}/allure + - uses: actions/github-script@v6 if: ${{ inputs.action == 'generate' && always() }} env: REPORT_URL: ${{ steps.generate-report.outputs.report-url }} - BUILD_TYPE: ${{ inputs.build_type }} SHA: ${{ github.event.pull_request.head.sha || github.sha }} with: script: | diff --git a/.github/workflows/build_and_test.yml b/.github/workflows/build_and_test.yml index 8c108e7f50..68102bce84 100644 --- a/.github/workflows/build_and_test.yml +++ b/.github/workflows/build_and_test.yml @@ -374,42 +374,88 @@ jobs: # XXX: no coverage data handling here, since benchmarks are run on release builds, # while coverage is currently collected for the debug ones - merge-allure-report: + create-test-report: runs-on: [ self-hosted, gen3, small ] container: image: 369495373322.dkr.ecr.eu-central-1.amazonaws.com/rust:pinned options: --init needs: [ regress-tests, benchmarks ] if: ${{ !cancelled() }} - strategy: - fail-fast: false - matrix: - build_type: [ debug, release ] - steps: - - name: Checkout - uses: actions/checkout@v3 - with: - submodules: false - - name: Create Allure report - id: create-allure-report + steps: + - uses: actions/checkout@v3 + + - name: Create Allure report (debug) + if: ${{ !cancelled() }} + id: create-allure-report-debug uses: ./.github/actions/allure-report with: action: generate - build_type: ${{ matrix.build_type }} + build_type: debug + + - name: Create Allure report (release) + if: ${{ !cancelled() }} + id: create-allure-report-release + uses: ./.github/actions/allure-report + with: + action: generate + build_type: release + + - uses: actions/github-script@v6 + if: > + !cancelled() && + github.event_name == 'pull_request' && ( + steps.create-allure-report-debug.outputs.report-url || + steps.create-allure-report-release.outputs.report-url + ) + with: + script: | + const reports = [{ + buildType: "debug", + reportUrl: "${{ steps.create-allure-report-debug.outputs.report-url }}", + jsonUrl: "${{ steps.create-allure-report-debug.outputs.report-json-url }}", + }, { + buildType: "release", + reportUrl: "${{ steps.create-allure-report-release.outputs.report-url }}", + jsonUrl: "${{ steps.create-allure-report-release.outputs.report-json-url }}", + }] + + const script = require("./scripts/pr-comment-test-report.js") + await script({ + github, + context, + fetch, + reports, + }) - name: Store Allure test stat in the DB - if: ${{ steps.create-allure-report.outputs.report-url }} + if: > + !cancelled() && ( + steps.create-allure-report-debug.outputs.report-url || + steps.create-allure-report-release.outputs.report-url + ) env: - BUILD_TYPE: ${{ matrix.build_type }} SHA: ${{ github.event.pull_request.head.sha || github.sha }} - REPORT_URL: ${{ steps.create-allure-report.outputs.report-url }} + REPORT_JSON_URL_DEBUG: ${{ steps.create-allure-report-debug.outputs.report-json-url }} + REPORT_JSON_URL_RELEASE: ${{ steps.create-allure-report-release.outputs.report-json-url }} TEST_RESULT_CONNSTR: ${{ secrets.REGRESS_TEST_RESULT_CONNSTR }} run: | - curl --fail --output suites.json ${REPORT_URL%/index.html}/data/suites.json ./scripts/pysync - DATABASE_URL="$TEST_RESULT_CONNSTR" poetry run python3 scripts/ingest_regress_test_result.py --revision ${SHA} --reference ${GITHUB_REF} --build-type ${BUILD_TYPE} --ingest suites.json + for report_url in $REPORT_JSON_URL_DEBUG $REPORT_JSON_URL_RELEASE; do + if [ -z "$report_url" ]; then + continue + fi + + if [[ "$report_url" == "$REPORT_JSON_URL_DEBUG" ]]; then + BUILD_TYPE=debug + else + BUILD_TYPE=release + fi + + curl --fail --output suites.json "${report_url}" + DATABASE_URL="$TEST_RESULT_CONNSTR" poetry run python3 scripts/ingest_regress_test_result.py --revision ${SHA} --reference ${GITHUB_REF} --build-type ${BUILD_TYPE} --ingest suites.json + done coverage-report: runs-on: [ self-hosted, gen3, small ] diff --git a/scripts/pr-comment-test-report.js b/scripts/pr-comment-test-report.js new file mode 100644 index 0000000000..d2c5aebc4f --- /dev/null +++ b/scripts/pr-comment-test-report.js @@ -0,0 +1,125 @@ +// +// The script parses Allure reports and posts a comment with a summary of the test results to the PR. +// It accepts an array of items and creates a comment with a summary for each one (for "release" and "debug", together or separately if any of them failed to be generated). +// +// The comment is updated on each run with the latest results. +// +// It is designed to be used with actions/github-script from GitHub Workflows: +// - uses: actions/github-script@v6 +// with: +// script: | +// const script = require("./scripts/pr-comment-test-report.js") +// await script({ +// github, +// context, +// fetch, +// reports: [{...}, ...], // each report is expected to have "buildType", "reportUrl", and "jsonUrl" properties +// }) +// + +module.exports = async ({ github, context, fetch, reports }) => { + // Marker to find the comment in the subsequent runs + const startMarker = `` + // GitHub bot id taken from (https://api.github.com/users/github-actions[bot]) + const githubActionsBotId = 41898282 + // The latest commit in the PR URL + const commitUrl = `${context.serverUrl}/${context.repo.owner}/${context.repo.repo}/pull/${context.payload.number}/commits/${context.sha}` + // Commend body itself + let commentBody = `${startMarker}\n### Test results for ${commitUrl}:\n___\n` + + // Common parameters for GitHub API requests + const ownerRepoParams = { + owner: context.repo.owner, + repo: context.repo.repo, + } + + for (const report of reports) { + const {buildType, reportUrl, jsonUrl} = report + + if (!reportUrl || !jsonUrl) { + console.warn(`"reportUrl" or "jsonUrl" aren't set for ${buildType} build`) + continue + } + + const suites = await (await fetch(jsonUrl)).json() + + // Allure distinguishes "failed" (with an assertion error) and "broken" (with any other error) tests. + // For this report it's ok to treat them in the same way (as failed). + failedTests = [] + passedTests = [] + skippedTests = [] + + retriedTests = [] + retriedStatusChangedTests = [] + + for (const parentSuite of suites.children) { + for (const suite of parentSuite.children) { + for (const test of suite.children) { + pytestName = `${parentSuite.name.replace(".", "/")}/${suite.name}.py::${test.name}` + test.pytestName = pytestName + + if (test.status === "passed") { + passedTests.push(test); + } else if (test.status === "failed" || test.status === "broken") { + failedTests.push(test); + } else if (test.status === "skipped") { + skippedTests.push(test); + } + + if (test.retriesCount > 0) { + retriedTests.push(test); + + if (test.retriedStatusChangedTests) { + retriedStatusChangedTests.push(test); + } + } + } + } + } + + const totalTestsCount = failedTests.length + passedTests.length + skippedTests.length + commentBody += `#### ${buildType} build: ${totalTestsCount} tests run: ${passedTests.length} passed, ${failedTests.length} failed, ${skippedTests.length} ([full report](${reportUrl}))\n` + if (failedTests.length > 0) { + commentBody += `Failed tests:\n` + for (const test of failedTests) { + const allureLink = `${reportUrl}#suites/${test.parentUid}/${test.uid}` + + commentBody += `- [\`${test.pytestName}\`](${allureLink})` + if (test.retriesCount > 0) { + commentBody += ` (ran [${test.retriesCount + 1} times](${allureLink}/retries))` + } + commentBody += "\n" + } + commentBody += "\n" + } + if (retriedStatusChangedTests > 0) { + commentBody += `Flaky tests:\n` + for (const test of retriedStatusChangedTests) { + const status = test.status === "passed" ? ":white_check_mark:" : ":x:" + commentBody += `- ${status} [\`${test.pytestName}\`](${reportUrl}#suites/${test.parentUid}/${test.uid}/retries)\n` + } + commentBody += "\n" + } + commentBody += "___\n" + } + + const { data: comments } = await github.rest.issues.listComments({ + issue_number: context.payload.number, + ...ownerRepoParams, + }) + + const comment = comments.find(comment => comment.user.id === githubActionsBotId && comment.body.startsWith(startMarker)) + if (comment) { + await github.rest.issues.updateComment({ + comment_id: comment.id, + body: commentBody, + ...ownerRepoParams, + }) + } else { + await github.rest.issues.createComment({ + issue_number: context.payload.number, + body: commentBody, + ...ownerRepoParams, + }) + } +}