-
Notifications
You must be signed in to change notification settings - Fork 63
Upload JUnit files from Buildomat to Trunk #9049
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
Conversation
|
Before this can be deployed a secret needs to be configured. I'll work with Augustus to set things up, please hold off on merging for now :3 |
8623d0e to
b61fdb5
Compare
|
This is ready for review! |
sunshowers
left a comment
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.
nice!
| # Determine whether this comes from a PR or not. It's load bearing that we detect things correctly, | ||
| # since Trunk treats PRs vs main branch commits differently when analyzing flaky tests. |
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.
Interesting!
| commit="$(jq -r .head_sha api/check_run)" | ||
| github_app="$(jq -r .app.slug api/check_run)" | ||
| job_url="$(jq -r .details_url api/check_run)" | ||
| pr_url="$(jq -r .pull_requests[0].url api/check_run)" |
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.
Can there be multiple PRs attached to a check run?
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.
Yes.
PRs cannot be attached to check runs. Instead, GitHub tries to look for PRs that contain the branch being tested, and includes them in the API response. From their docs:
Pull requests that are open with a
head_shaorhead_branchthat matches the check. The returned pull requests do not necessarily indicate pull requests that triggered the check
In practice, if there are multiple PRs returned by the API this will show in the Trunk UI just the information of the first one. I don't think it's the end of the world though, I don't expect it to happen much.
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.
Oh weird. thanks, should be okay in that case -- might be worth putting in a small comment explaining this.
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.
Added a comment!
| # For PRs from outside forks the GitHub API doesn't return the PR linked to the commit, nor the | ||
| # branch name. In that case, assume it's an unknown PR and use placeholders. |
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.
lol...
hmm, I wonder how many folks are doing this -- what happens in this case? Does Trunk recognize this?
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.
| echo "unsupported job name, skipping upload: ${job_name}" | ||
| exit 0 | ||
| ;; |
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.
Should this check be moved to the top?
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.
It has to happen after the check_run API request (since it retrieves the job name from it). I can move it to happen just after that request, but stylistically I opted to do all API requests first. If you prefer I can move it up though.
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.
Yeah I guess I found it to be a little strange that we're doing all these extra requests for a check run we'll not process. I'd personally move it to right after the check_run request but feel free to make the call here.
| commit="$(jq -r .head_sha api/check_run)" | ||
| github_app="$(jq -r .app.slug api/check_run)" | ||
| job_url="$(jq -r .details_url api/check_run)" | ||
| pr_url="$(jq -r .pull_requests[0].url api/check_run)" |
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.
Oh weird. thanks, should be okay in that case -- might be worth putting in a small comment explaining this.
| echo "unsupported job name, skipping upload: ${job_name}" | ||
| exit 0 | ||
| ;; |
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.
Yeah I guess I found it to be a little strange that we're doing all these extra requests for a check run we'll not process. I'd personally move it to right after the check_run request but feel free to make the call here.
b61fdb5 to
ff6cc08
Compare

To help triage and handle flaky tests, it was suggested in the control plane huddle a few weeks ago to setup Trunk's flaky test detection. We run tests on Buildomat though, which doesn't support setting the secret to authenticate with Trunk.
To work around the issue, I created a new script to upload all the relevant metadata to Trunk given the GitHub Check Run ID of a Buildomat job:
The script is also hooked up to a GitHub Actions job that runs after every completed Buildomat job. Note that if jobs change names or are added the
caseblock in the script will need to be updated.