Add GitHub Action to measure latency difference on PR label#13189
Draft
amk-stripe wants to merge 2 commits into
Draft
Add GitHub Action to measure latency difference on PR label#13189amk-stripe wants to merge 2 commits into
amk-stripe wants to merge 2 commits into
Conversation
When a PR is labeled with 'measure-latency-difference', this workflow: 1. Determines the commit (HEAD of the branch) and base commit (merge-base with master) 2. Triggers the new 'run-paymentsheet-latency-difference' Bitrise workflow with those two commits as parameters 3. Polls until the Bitrise build completes (up to 2 hours) 4. Fetches the build log, extracts the latency delta report and raw measurements, and posts them as a comment on the PR Also adds the 'run-paymentsheet-latency-difference' Bitrise workflow that runs the measure_latency_difference.rb script using the BASE_COMMIT and COMMIT environment variables. Committed-By-Agent: goose
Contributor
Latency Difference Measurement⏳ Measuring latency difference between commits...
This comment will be updated once the measurement is complete. |
Two bugs: 1. In the trigger step, `$(curl --silent --fail ...)` inside a variable assignment does not trigger bash's set -e when curl gets an error response, so the step silently continued with an empty build_slug. Fixed by: building the request body separately (so jq errors are visible), using --fail-with-body so error responses are logged, and explicitly checking that build_slug is non-empty before continuing. 2. In the poll step, `$(echo "$RESPONSE" | jq ...)` inside a variable assignment would crash with a jq parse error when RESPONSE was empty (e.g. due to a transient network error or an empty build_slug in the URL). Fixed by: removing --fail from the poll curl so transient errors retry instead of crashing, using jq's // empty filter so missing fields produce "" rather than "null", and adding || STATUS="" so a jq parse error on a bad response retries rather than crashing the whole step. Committed-By-Agent: goose
amk-stripe
added
measure-latency-difference
and removed
measure-latency-difference
labels
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
When a PR is labeled with 'measure-latency-difference', this workflow:
Also adds the 'run-paymentsheet-latency-difference' Bitrise workflow that runs the measure_latency_difference.rb script using the BASE_COMMIT and COMMIT environment variables.
Committed-By-Agent: goose
Summary
Motivation
Testing
Screenshots
Changelog