From 63cdd09a010d002e92917d8b80fc6f778ecfc81d Mon Sep 17 00:00:00 2001 From: Jamil Date: Tue, 20 Feb 2024 18:17:48 -0800 Subject: [PATCH] refactor(ci): Merge perf results into one comment (#3707) One comment vs eight, need I say more? --- .github/workflows/ci.yml | 91 ++++++++++------------ scripts/tests/perf/results.js | 141 ++++++++++++++++++---------------- 2 files changed, 117 insertions(+), 115 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 15e2a155e..79458b466 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -281,56 +281,9 @@ jobs: - name: 'Save performance test results: ${{ matrix.test_name }}' uses: actions/upload-artifact@v4 with: - name: '${{ matrix.test_name }}-iperf3results' + overwrite: true + name: ${{ matrix.test_name }}-${{ github.ref_name == 'main' && 'main' || github.sha }}-iperf3results path: ./${{ matrix.test_name }}.json - - name: 'Download main branch performance test results: ${{ matrix.test_name }}' - id: download-artifact - if: ${{ github.event_name == 'pull_request' }} - env: - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - run: | - set -xe - - REPO="${{ github.repository }}" - WORKFLOW="cd.yml" - ARTIFACT_NAME="${{ matrix.test_name }}-iperf3results" - - ARTIFACTS_URL=$( - gh api \ - -H "Accept: application/vnd.github+json" \ - -H "X-GitHub-Api-Version: 2022-11-28" \ - "/repos/${REPO}/actions/workflows/${WORKFLOW}/runs?event=push&branch=main&status=success&per_page=1" \ - --jq ".workflow_runs[0].artifacts_url" - ) - - DOWNLOAD_URL=$( - gh api \ - -H "Accept: application/vnd.github+json" \ - -H "X-GitHub-Api-Version: 2022-11-28" \ - "${ARTIFACTS_URL}" \ - --jq '.artifacts[] | select(.name == "'${ARTIFACT_NAME}'") | .archive_download_url' - ) - - if [ -z "$DOWNLOAD_URL" ]; then - echo "No artifact found for ${ARTIFACT_NAME} in main branch" - else - echo "continue=true" >> $GITHUB_OUTPUT - set +x - curl -H "Accept: application/vnd.github+json" -H "Authorization: token ${{ secrets.GITHUB_TOKEN }}" -L -o "${ARTIFACT_NAME}.zip" "$DOWNLOAD_URL" - - set -x - unzip "${ARTIFACT_NAME}.zip" -d "./main" - rm "${ARTIFACT_NAME}.zip" - fi - - name: Update PR with results - uses: actions/github-script@v7 - id: perf-comment - if: ${{ github.event_name == 'pull_request' && steps.download-artifact.outputs.continue == 'true' }} - with: - github-token: ${{ secrets.GITHUB_TOKEN }} - script: | - const { script } = require('./scripts/tests/perf/results.js'); - script(github, context, '${{ matrix.test_name }}'); - name: Show Client logs if: "!cancelled()" run: docker compose logs client @@ -346,3 +299,43 @@ jobs: - name: Show iperf3 logs if: "!cancelled()" run: docker compose logs iperf3 + + compare-results: + if: ${{ github.event_name == 'pull_request' }} + needs: perf-tests + runs-on: ubuntu-22.04 + permissions: + contents: read + id-token: write + pull-requests: write + steps: + - uses: actions/checkout@v4 + - name: Download PR performance test results + uses: actions/download-artifact@v4 + with: + pattern: '*-${{ github.sha }}-iperf3results' + merge-multiple: true + path: ./ + - name: Download main branch performance test results + uses: actions/download-artifact@v4 + with: + pattern: '*-main-iperf3results' + merge-multiple: true + github-token: ${{ secrets.GITHUB_TOKEN }} + path: ./main + - name: Update PR with results + uses: actions/github-script@v7 + with: + github-token: ${{ secrets.GITHUB_TOKEN }} + script: | + const { script } = require('./scripts/tests/perf/results.js'); + script(github, context, [ + 'direct-tcp-client2server', + 'direct-tcp-server2client', + 'direct-udp-client2server', + 'direct-udp-server2client', + 'relayed-tcp-client2server', + 'relayed-tcp-server2client', + 'relayed-udp-client2server', + 'relayed-udp-server2client' + ]); diff --git a/scripts/tests/perf/results.js b/scripts/tests/perf/results.js index c020a8b54..1ff712949 100644 --- a/scripts/tests/perf/results.js +++ b/scripts/tests/perf/results.js @@ -35,80 +35,89 @@ function humanFileSize(bytes, dp = 1) { return bytes.toFixed(dp) + " " + units[u]; } -exports.script = async function (github, context, test_name) { - // 1. Read the current results - const results = JSON.parse(fs.readFileSync(test_name + ".json")).end; +exports.script = async function (github, context, test_names) { + let output = `### Performance Test Results - // 2. Read the main results - const results_main = JSON.parse( - fs.readFileSync(path.join("main", test_name + ".json")) - ).end; + `; + let tcp_output = `#### TCP - let output = ""; - - if (test_name.includes("tcp")) { - const tcp_sum_received_bits_per_second = - humanFileSize(results.sum_received.bits_per_second) + - " (" + - getDiffPercents( - results_main.sum_received.bits_per_second, - results.sum_received.bits_per_second - ) + - ")"; - const tcp_sum_sent_bits_per_second = - humanFileSize(results.sum_sent.bits_per_second) + - " (" + - getDiffPercents( - results_main.sum_sent.bits_per_second, - results.sum_sent.bits_per_second - ) + - ")"; - const tcp_sum_sent_retransmits = - results.sum_sent.retransmits + - " (" + - getDiffPercents( - results_main.sum_sent.retransmits, - results.sum_sent.retransmits - ) + - ")"; - - output = `## Performance Test Results: ${test_name} - -| Received/s | Sent/s | Retransmits | -|---|---|---| -| ${tcp_sum_received_bits_per_second} | ${tcp_sum_sent_bits_per_second} | ${tcp_sum_sent_retransmits} | +| Test Name | Received/s | Sent/s | Retransmits | +|---|---|---|---| `; - } else if (test_name.includes("udp")) { - const udp_sum_bits_per_second = - humanFileSize(results.sum.bits_per_second) + - " (" + - getDiffPercents( - results_main.sum.bits_per_second, - results.sum.bits_per_second - ) + - ")"; - const udp_sum_jitter_ms = - results.sum.jitter_ms.toFixed(2) + - "ms (" + - getDiffPercents(results_main.sum.jitter_ms, results.sum.jitter_ms) + - ")"; - const udp_sum_lost_percent = - results.sum.lost_percent.toFixed(2) + - "% (" + - getDiffPercents(results_main.sum.lost_percent, results.sum.lost_percent) + - ")"; - output = `## Performance Test Results: ${test_name} - -| Total/s | Jitter | Lost | -|---|---|---| -| ${udp_sum_bits_per_second} | ${udp_sum_jitter_ms} | ${udp_sum_lost_percent} | + let udp_output = `#### UDP +| Test Name | Total/s | Jitter | Lost | +|---|---|---|---| `; - } else { - throw new Error("Unknown test type"); + + for (const test_name of test_names) { + // 1. Read the current results + const results = JSON.parse(fs.readFileSync(test_name + ".json")).end; + + // 2. Read the main results + const results_main = JSON.parse( + fs.readFileSync(path.join("main", test_name + ".json")) + ).end; + + if (test_name.includes("tcp")) { + const tcp_sum_received_bits_per_second = + humanFileSize(results.sum_received.bits_per_second) + + " (" + + getDiffPercents( + results_main.sum_received.bits_per_second, + results.sum_received.bits_per_second + ) + + ")"; + const tcp_sum_sent_bits_per_second = + humanFileSize(results.sum_sent.bits_per_second) + + " (" + + getDiffPercents( + results_main.sum_sent.bits_per_second, + results.sum_sent.bits_per_second + ) + + ")"; + const tcp_sum_sent_retransmits = + results.sum_sent.retransmits + + " (" + + getDiffPercents( + results_main.sum_sent.retransmits, + results.sum_sent.retransmits + ) + + ")"; + + tcp_output += `| ${test_name} | ${tcp_sum_received_bits_per_second} | ${tcp_sum_sent_bits_per_second} | ${tcp_sum_sent_retransmits} |\n`; + } else if (test_name.includes("udp")) { + const udp_sum_bits_per_second = + humanFileSize(results.sum.bits_per_second) + + " (" + + getDiffPercents( + results_main.sum.bits_per_second, + results.sum.bits_per_second + ) + + ")"; + const udp_sum_jitter_ms = + results.sum.jitter_ms.toFixed(2) + + "ms (" + + getDiffPercents(results_main.sum.jitter_ms, results.sum.jitter_ms) + + ")"; + const udp_sum_lost_percent = + results.sum.lost_percent.toFixed(2) + + "% (" + + getDiffPercents( + results_main.sum.lost_percent, + results.sum.lost_percent + ) + + ")"; + + udp_output += `| ${test_name} | ${udp_sum_bits_per_second} | ${udp_sum_jitter_ms} | ${udp_sum_lost_percent} |\n`; + } else { + throw new Error("Unknown test type"); + } } + output += tcp_output + "\n" + udp_output; + // Retrieve existing bot comments for the PR const { data: comments } = await github.rest.issues.listComments({ owner: context.repo.owner,