Skip to content
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

[githooks] CI scripts pipe failures to stderr #1300

Merged
merged 1 commit into from
May 18, 2024
Merged

Conversation

joshlf
Copy link
Member

@joshlf joshlf commented May 18, 2024

Previously, some scripts had output which would only be generated on failure, but this output was passed to stdout, which githooks/pre-push pipes to /dev/null. In this commit, any such output is unconditionally redirected to stderr - if there's any output, we want to see it. This previously resulted in very opaque errors during git push, which should be addressed by this commit.

Also, in #728, we added ci/check_all_toolchains_tested.sh, but didn't add it to githooks/pre-push. In this commit, we fix that, and also add a test to githooks/pre-push to validate that all scripts in ci/* at least show up in githooks/pre-push by name. This isn't a foolproof check, but it should catch obvious errors.

Previously, some scripts had output which would only be generated on
failure, but this output was passed to stdout, which `githooks/pre-push`
pipes to `/dev/null`. In this commit, any such output is unconditionally
redirected to `stderr` - if there's any output, we want to see it. This
previously resulted in very opaque errors during `git push`, which
should be addressed by this commit.

Also, in #728, we added `ci/check_all_toolchains_tested.sh`, but didn't
add it to `githooks/pre-push`. In this commit, we fix that, and also add
a test to `githooks/pre-push` to validate that all scripts in `ci/*` at
least show up in `githooks/pre-push` by name. This isn't a foolproof
check, but it should catch obvious errors.
@joshlf joshlf requested a review from jswrenn May 18, 2024 19:01
@joshlf joshlf enabled auto-merge May 18, 2024 19:01
@codecov-commenter
Copy link

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.79%. Comparing base (1c77a9d) to head (a502d9b).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1300   +/-   ##
=======================================
  Coverage   87.79%   87.79%           
=======================================
  Files          15       15           
  Lines        5171     5171           
=======================================
  Hits         4540     4540           
  Misses        631      631           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@joshlf joshlf added this pull request to the merge queue May 18, 2024
Merged via the queue into main with commit cc5821c May 18, 2024
210 checks passed
@joshlf joshlf deleted the git-hook-failure-report branch May 18, 2024 21:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants