diff --git a/.buildkite/test_description.json b/.buildkite/test_description.json index f444440..d600c7e 100644 --- a/.buildkite/test_description.json +++ b/.buildkite/test_description.json @@ -3,12 +3,18 @@ { "test_name": "build-gnu", "command": "cargo build --release", - "platform": ["x86_64", "aarch64"] + "platform": [ + "x86_64", + "aarch64" + ] }, { "test_name": "build-musl", "command": "cargo build --release --target {target_platform}-unknown-linux-musl", - "platform": ["x86_64", "aarch64"] + "platform": [ + "x86_64", + "aarch64" + ] }, { "test_name": "style", @@ -17,35 +23,48 @@ { "test_name": "unittests-gnu", "command": "cargo test --all-features --workspace", - "platform": ["x86_64", "aarch64"] + "platform": [ + "x86_64", + "aarch64" + ] }, { "test_name": "unittests-musl", "command": "cargo test --all-features --workspace --target {target_platform}-unknown-linux-musl", - "platform": ["x86_64", "aarch64"] + "platform": [ + "x86_64", + "aarch64" + ] }, { "test_name": "clippy", "command": "cargo clippy --workspace --bins --examples --benches --all-features -- -D warnings", - "platform": ["x86_64", "aarch64"] + "platform": [ + "x86_64", + "aarch64" + ] }, { "test_name": "check-warnings", "command": "RUSTFLAGS=\"-D warnings\" cargo check --all-targets --all-features --workspace", - "platform": ["x86_64", "aarch64"] + "platform": [ + "x86_64", + "aarch64" + ] }, { "test_name": "coverage", - "command": "find . -type f -name \"test_coverage.py\" | xargs pytest", + "command": "pytest $(find . -type f -name \"test_coverage.py\")", "docker_plugin": { "privileged": true }, - "platform": ["x86_64"] + "platform": [ + "x86_64" + ] }, { "test_name": "commit-format", - "command": "find . -type f -name \"test_commit_format.py\" | xargs pytest", - "conditional": "build.env(\"BUILDKITE_REPO\") !~ /^git@/", + "command": "pytest $(find . -type f -name \"test_commit_format.py\")", "docker_plugin": { "propagate-environment": true } diff --git a/integration_tests/conftest.py b/integration_tests/conftest.py index 7011cb4..9fc5ef4 100644 --- a/integration_tests/conftest.py +++ b/integration_tests/conftest.py @@ -3,8 +3,8 @@ import pytest -PROFILE_CI="ci" -PROFILE_DEVEL="devel" +PROFILE_CI = "ci" +PROFILE_DEVEL = "devel" def pytest_addoption(parser): @@ -21,8 +21,9 @@ def pytest_addoption(parser): "--no-cleanup", action="store_true", default=False, - help="Keep the coverage report in `kcov_output` directory. If this flag is not provided, " - "both coverage related directories are removed." + help="Keep the coverage report in `kcov_output` directory. If this " + "flag is not provided, both coverage related directories are " + "removed." ) diff --git a/integration_tests/test_benchmark.py b/integration_tests/test_benchmark.py index 14d897d..12a4c82 100644 --- a/integration_tests/test_benchmark.py +++ b/integration_tests/test_benchmark.py @@ -1,29 +1,43 @@ # Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved. # SPDX-License-Identifier: Apache-2.0 -"""Compare benchmark results before and after a pull request.""" +""" +Compare benchmark results before and after a pull request. -import os, subprocess -import pytest +This test works properly on the local machine only when the environment +variables REMOTE and BASE_BRANCH are set. Otherwise the default values +are "origin" for the remote name of the upstream repository and "master" +for the name of the base branch, and this test may not work as expected. +""" + +import os +import subprocess from utils import get_repo_root_path -# Repository being tested (as configured with Buildkite). -UPSTREAM_REPO_URL = os.environ['BUILDKITE_REPO'] -# Base branch as configured with Buildkite. -BASE_BRANCH = os.environ['BUILDKITE_PULL_REQUEST_BASE_BRANCH'] -# File used for saving the results of cargo bench when running on the PR branch. +REMOTE = \ + os.environ.get('BUILDKITE_REPO') or \ + os.environ.get('REMOTE') or \ + "origin" +BASE_BRANCH = \ + os.environ.get('BUILDKITE_PULL_REQUEST_BASE_BRANCH') or \ + os.environ.get('BASE_BRANCH') or \ + "master" +# File used for saving the results of cargo bench +# when running on the PR branch. PR_BENCH_RESULTS_FILE = "pr_bench_results" -# File used for saving the results of cargo bench when running on the upstream branch. +# File used for saving the results of cargo bench +# when running on the upstream branch. UPSTREAM_BENCH_RESULTS_FILE = "upstream_bench_results" + def test_bench(): """Runs benchmarks before and after and compares the results.""" os.chdir(get_repo_root_path()) # Get numbers for current HEAD. return_code, stdout, stderr = _run_cargo_bench(PR_BENCH_RESULTS_FILE) - # Even if it is the first time this test is run, the benchmark tests should pass. - # For this purpose, we need to explicitly check the return code. + # Even if it is the first time this test is run, the benchmark tests should + # pass. For this purpose, we need to explicitly check the return code. assert return_code == 0, "stdout: {}\n stderr: {}".format(stdout, stderr) # Get numbers from upstream tip, without the changes from the current PR. @@ -31,37 +45,47 @@ def test_bench(): return_code, stdout, stderr = _run_cargo_bench(UPSTREAM_BENCH_RESULTS_FILE) # Before checking any results, let's just go back to the PR branch. - # This way we make sure that the cleanup always happens even if the test fails. + # This way we make sure that the cleanup always happens even if the test + # fails. _git_checkout_pr_branch() if return_code == 0: - # In case this benchmark also ran successfully, we can call critcmp and compare the results. + # In case this benchmark also ran successfully, we can call critcmp and + # compare the results. _run_critcmp() else: - # The benchmark did not run successfully, but it might be that it is because a benchmark does not exist. - # In this case, we do not want to fail the test. + # The benchmark did not run successfully, but it might be that it is + # because a benchmark does not exist. In this case, we do not want to + # fail the test. if "error: no bench target named `main`" in stderr: - # This is a bit of a &*%^ way of checking if the benchmark does not exist. - # Hopefully it will be possible to check it in another way...soon - print("There are no benchmarks in master. No comparison can happen.") + # This is a bit of a &*%^ way of checking if the benchmark does not + # exist. Hopefully it will be possible to check it in another way + # ...soon + print( + "There are no benchmarks in master. No comparison can happen." + ) else: - assert return_code == 0, "stdout: {}\n stderr: {}".format(stdout, stderr) + assert return_code == 0, "stdout: {}\n stderr: {}".format( + stdout, stderr) def _run_cargo_bench(baseline): """Runs `cargo bench` and tags the baseline.""" process = subprocess.run( - "cargo bench --bench main --all-features -- --noplot --save-baseline {}" - .format(baseline), + "cargo bench --bench main --all-features -- --noplot " + "--save-baseline {}".format(baseline), shell=True, stderr=subprocess.PIPE, stdout=subprocess.PIPE ) - return process.returncode, process.stdout.decode('utf-8'), process.stderr.decode('utf-8') + return process.returncode, process.stdout.decode('utf-8'),\ + process.stderr.decode('utf-8') def _run_critcmp(): p = subprocess.run( - "critcmp {} {}".format(UPSTREAM_BENCH_RESULTS_FILE, PR_BENCH_RESULTS_FILE), + "critcmp {} {}".format( + UPSTREAM_BENCH_RESULTS_FILE, PR_BENCH_RESULTS_FILE + ), shell=True, check=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE @@ -71,9 +95,10 @@ def _run_critcmp(): print('ERRORS') print(p.stderr.decode('utf-8')) + def _git_checkout_upstream_branch(): subprocess.run( - "git fetch {} {}".format(UPSTREAM_REPO_URL, BASE_BRANCH), + "git fetch {} {}".format(REMOTE, BASE_BRANCH), shell=True, check=True ) subprocess.run( @@ -81,6 +106,7 @@ def _git_checkout_upstream_branch(): shell=True, check=True ) + def _git_checkout_pr_branch(): subprocess.run( "git checkout -", diff --git a/integration_tests/test_commit_format.py b/integration_tests/test_commit_format.py index add727e..72a3b01 100644 --- a/integration_tests/test_commit_format.py +++ b/integration_tests/test_commit_format.py @@ -4,10 +4,9 @@ Test the commit message format. This test works properly on the local machine only when the environment -variables BASE_BRANCH and REMOTE are set. Otherwise the default values -are "master" for the name of the base branch and "origin" for the -remote name of the upstream repository, and this test may not work as -expected. +variables REMOTE and BASE_BRANCH are set. Otherwise the default values +are "origin" for the remote name of the upstream repository and "master" +for the name of the base branch, and this test may not work as expected. """ import os @@ -17,14 +16,14 @@ from utils import get_cmd_output COMMIT_TITLE_MAX_LEN = 50 COMMIT_BODY_LINE_MAX_LEN = 72 -BASE_BRANCH = \ - os.environ.get('BUILDKITE_PULL_REQUEST_BASE_BRANCH') or \ - os.environ.get('BASE_BRANCH') or \ - "master" REMOTE = \ os.environ.get('BUILDKITE_REPO') or \ os.environ.get('REMOTE') or \ "origin" +BASE_BRANCH = \ + os.environ.get('BUILDKITE_PULL_REQUEST_BASE_BRANCH') or \ + os.environ.get('BASE_BRANCH') or \ + "master" def test_commit_format(): diff --git a/integration_tests/test_coverage.py b/integration_tests/test_coverage.py index 65ccc23..1df66ec 100644 --- a/integration_tests/test_coverage.py +++ b/integration_tests/test_coverage.py @@ -2,7 +2,12 @@ # SPDX-License-Identifier: Apache-2.0 """Test the coverage and update the threshold when coverage is increased.""" -import json, os, re, shutil, subprocess, platform +import json +import os +import re +import shutil +import subprocess +import platform import pytest from utils import get_repo_root_path @@ -90,12 +95,12 @@ def _get_current_coverage(coverage_config, no_cleanup): "--exclude-region={} " \ "--exclude-pattern={} " \ "--verify".format( - kcov_build_dir, - additional_kcov_param, - kcov_output_dir, - exclude_region, - exclude_pattern - ) + kcov_build_dir, + additional_kcov_param, + kcov_output_dir, + exclude_region, + exclude_pattern + ) # Pytest closes stdin by default, but some tests might need it to be open. # In the future, should the need arise, we can feed custom data to stdin. @@ -110,7 +115,8 @@ def _get_current_coverage(coverage_config, no_cleanup): )[0]) # Remove coverage related directories. - # If user provided `--no-cleanup` flag, `kcov_output_dir` should not be removed. + # If user provided `--no-cleanup` flag, `kcov_output_dir` + # should not be removed. if not no_cleanup: shutil.rmtree(kcov_output_dir, ignore_errors=True) shutil.rmtree(kcov_build_dir, ignore_errors=True) @@ -125,12 +131,13 @@ def test_coverage(profile, no_cleanup): if previous_coverage < current_coverage: if profile == pytest.profile_ci: # In the CI Profile we expect the coverage to be manually updated. - assert False, "Coverage is increased from {} to {}. " \ - "Please update the coverage in " \ - "tests/coverage.".format( - previous_coverage, - current_coverage - ) + assert False,\ + "Coverage is increased from {} to {}. "\ + "Please update the coverage in coverage_config_{}.".format( + previous_coverage, + current_coverage, + platform.machine() + ) elif profile == pytest.profile_devel: coverage_config["coverage_score"] = current_coverage _write_coverage_config(coverage_config)