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

bugfix: Fix codecov broken backend coverage upload. #14972

Merged
merged 1 commit into from
May 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 13 additions & 4 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,20 @@ aliases:
run:
name: upload coverage report
command: |
# codecov requires `.coverage` file to be stored in pwd for
# uploading coverage results.
mv /home/circleci/zulip/var/.coverage /home/circleci/zulip/.coverage

. /srv/zulip-py3-venv/bin/activate
# codecov version is fixed here, since future versions of it
# use "find" for locating files which is buggy on some platforms.
# See https://github.com/codecov/codecov-python/issues/250
pip install codecov==2.0.15 && codecov \
# TODO: Check that the next release of codecov doesn't
# throw find error.
# codecov==2.0.16 introduced a bug which uses "find"
# for locating files which is buggy on some platforms.
# It was fixed via https://github.com/codecov/codecov-python/pull/217
# and should get automatically fixed here once it's released.
# We cannot pin the version here because we need the latest version for uploading files.
# see https://community.codecov.io/t/http-400-while-uploading-to-s3-with-python-codecov-from-travis/1428/7
pip install codecov && codecov \
|| echo "Error in uploading coverage reports to codecov.io."

- &build_production
Expand Down
2 changes: 1 addition & 1 deletion tools/ci/backend
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ set -x
# docker setup means the auto-detection logic sees the ~36 processes
# the Docker host has, not the ~2 processes of resources we're
# allocated.
./tools/test-backend --coverage --include-webhooks --parallel=6
./tools/test-backend --coverage --include-webhooks --no-cov-cleanup --parallel=6

# We run mypy after the backend tests so we get output from the
# backend tests, which tend to uncover more serious problems, first.
Expand Down
13 changes: 9 additions & 4 deletions tools/test-backend
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,10 @@ def main() -> None:
parser.add_argument('--verbose-coverage', dest='verbose_coverage',
action="store_true",
default=False, help='Enable verbose print of coverage report.')
parser.add_argument('--no-cov-cleanup', dest='no_cov_cleanup',
action='store_true',
default=False,
help="Do not clean generated coverage files.")

parser.add_argument('--parallel', dest='processes',
type=int,
Expand Down Expand Up @@ -354,10 +358,12 @@ def main() -> None:
assert_provisioning_status_ok(options.force)

if options.coverage:
import atexit
import coverage
cov = coverage.Coverage(config_file="tools/coveragerc", concurrency='multiprocessing')
atexit.register(lambda: cov.erase()) # Ensure the data file gets cleaned up at the end.
cov = coverage.Coverage(data_suffix="", config_file="tools/coveragerc", concurrency='multiprocessing')
# Do not clean .coverage file in CircleCi job so that coverage data can be uploaded.
if not options.no_cov_cleanup:
import atexit
atexit.register(lambda: cov.erase()) # Ensure the data file gets cleaned up at the end.
cov.start()
if options.profile:
import cProfile
Expand Down Expand Up @@ -407,7 +413,6 @@ def main() -> None:
cov.stop()
cov.save()
cov.combine()
cov.data_suffix = False # Disable suffix so that filename is .coverage
Copy link
Contributor

Choose a reason for hiding this comment

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

perhaps we should have the data_suffix change as a separate commit? I was confused for a minute about why this was necessary for the no-cov-cleanup option, before realizing that this is a conceptually separate change

Copy link
Member Author

Choose a reason for hiding this comment

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

If it's necessary for this fix, we should keep them in same commit. Let me see their documentation about data_suffix and see if we can do better here.

cov.save()
if options.verbose_coverage:
print("Printing coverage data")
Expand Down