Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

ci: pass coverage env statically #32490

Closed
wants to merge 1 commit into from
Closed

ci: pass coverage env statically #32490

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 19 additions & 8 deletions ci/docker-run.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,12 +107,23 @@ ARGS+=(
--env CRATES_IO_TOKEN
)

# Also propagate environment variables needed for codecov
# https://docs.codecov.io/docs/testing-with-docker#section-codecov-inside-docker
# We normalize CI to `1`; but codecov expects it to be `true` to detect Buildkite...
# Unfortunately, codecov.io fails sometimes:
# curl: (7) Failed to connect to codecov.io port 443: Connection timed out
CODECOV_ENVS=$(CI=true bash <(while ! curl -sS --retry 5 --retry-delay 2 --retry-connrefused https://codecov.io/env; do sleep 10; done))
# https://docs.codecov.com/docs/testing-with-docker
# inline those environment variables for avoiding various curl errors
# coverage env
ARGS+=(
--env CODECOV_ENV
--env CODECOV_TOKEN
--env CODECOV_URL
--env CODECOV_SLUG
--env VCS_COMMIT_ID
--env VCS_BRANCH_NAME
--env VCS_PULL_REQUEST
--env VCS_SLUG
--env VCS_TAG
--env CI_BUILD_URL
--env CI_BUILD_ID
--env CI_JOB_ID
)
Comment on lines +113 to +126
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like avoid hard-coding as much as possible. i know i'm opiniated and seldom these envs should change. but i just don't like hard-coding....

seems curl --fail does the trick.

$ curl -sS --retry 5 --retry-delay 2 --retry-connrefused https://codecov.io/env
<html><head>
<meta http-equiv="content-type" content="text/html;charset=utf-8">
<title>502 Server Error</title>
</head>
<body text=#000000 bgcolor=#ffffff>
<h1>Error: Server Error</h1>
<h2>The server encountered a temporary error and could not complete your request.<p>Please try again in 30 seconds.</h2>
<h2></h2>
</body></html>
<html><head>
<meta http-equiv="content-type" content="text/html;charset=utf-8">
<title>502 Server Error</title>
</head>
<body text=#000000 bgcolor=#ffffff>
<h1>Error: Server Error</h1>
<h2>The server encountered a temporary error and could not complete your request.<p>Please try again in 30 seconds.</h2>
<h2></h2>
</body></html>
#!/usr/bin/env bash

# Apache License Version 2.0, January 2004
# https://github.com/codecov/codecov-bash/blob/master/LICENSE


$ curl --fail -sS --retry 5 --retry-delay 2 --retry-connrefused https://codecov.io/env
#!/usr/bin/env bash

# Apache License Version 2.0, January 2004
# https://github.com/codecov/codecov-bash/blob/master/LICENSE

set -e +o pipefail

VERSION="1.0.6"  

add()
{
  if [ -z "$1" ];
  then
    return
  fi

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the problem is that curl is printing 502 response body. blame flaky codecov infra.
seems their infra changed when i added this at #8263. so blame me as well. ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do this cuz

  1. we can see what we're passing easily also make it controllable.
  2. although we pass those env dynamically, we still need to set them manually to machines or we get an empty env in the CI environment

What's the benefit you would like to keep the dynamic environment variable set

Copy link
Contributor

@ryoqun ryoqun Jul 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(sorry i have to go to chore soon)

What's the benefit you would like to keep the dynamic environment variable set

i'd say a maintenance burden. for example, this pr starts to miss to export some of these envs, which is used for codecov report metadata... these env set can change anytime by codecov.

elif [ "$CI" = "true" ] && [ "$BUILDKITE" = "true" ];
then
  add "CI"
  add "BUILDKITE"
  add "BUILDKITE_BRANCH"
  add "BUILDKITE_BUILD_NUMBER"
  add "BUILDKITE_JOB_ID"
  add "BUILDKITE_BUILD_URL"
  add "BUILDKITE_PROJECT_SLUG"
  add "BUILDKITE_COMMIT"

also, codocov doc is recommending to call curl dynamically...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

true. it is easy to miss some needed environment variables. just opened another solution for handling curl response code. #32492


if $INTERACTIVE; then
if [[ -n $1 ]]; then
Expand All @@ -122,9 +133,9 @@ if $INTERACTIVE; then
fi
set -x
# shellcheck disable=SC2086
exec docker run --interactive --tty "${ARGS[@]}" $CODECOV_ENVS "$IMAGE" bash
exec docker run --interactive --tty "${ARGS[@]}" "$IMAGE" bash
fi

set -x
# shellcheck disable=SC2086
exec docker run "${ARGS[@]}" $CODECOV_ENVS -t "$IMAGE" "$@"
exec docker run "${ARGS[@]}" -t "$IMAGE" "$@"