Fix shard summaries for Windows tasks (#1897)
Buildkite cannot download logs for Windows tasks since it mishandles
backslashes. This change fixes the problem by downloading log files
manually.
Fixes https://github.com/bazelbuild/continuous-integration/issues/1891
Drive-by changes: Improved error handling & formatted bazelci.py
diff --git a/buildkite/bazelci.py b/buildkite/bazelci.py
index 24fa625..351479b 100755
--- a/buildkite/bazelci.py
+++ b/buildkite/bazelci.py
@@ -703,12 +703,16 @@
def _get_buildkite_token(self):
return decrypt_token(
- encrypted_token=self._ENCRYPTED_BUILDKITE_API_TESTING_TOKEN
- if THIS_IS_TESTING
- else self._ENCRYPTED_BUILDKITE_API_TOKEN,
- kms_key="buildkite-testing-api-token"
- if THIS_IS_TESTING
- else "buildkite-untrusted-api-token",
+ encrypted_token=(
+ self._ENCRYPTED_BUILDKITE_API_TESTING_TOKEN
+ if THIS_IS_TESTING
+ else self._ENCRYPTED_BUILDKITE_API_TOKEN
+ ),
+ kms_key=(
+ "buildkite-testing-api-token"
+ if THIS_IS_TESTING
+ else "buildkite-untrusted-api-token"
+ ),
)
def _open_url(self, url, params=[]):
@@ -2378,7 +2382,11 @@
diffbase_archive_url = get_commit_archive_url(resolved_diffbase)
local_archive_path = download_file(diffbase_archive_url, tmpdir, "repo.tar.gz")
diffbase_repo_dir = os.path.join(tmpdir, resolved_diffbase)
- extract_archive(local_archive_path, diffbase_repo_dir, strip_top_level_dir = not is_googlesource_repo(diffbase_archive_url))
+ extract_archive(
+ local_archive_path,
+ diffbase_repo_dir,
+ strip_top_level_dir=not is_googlesource_repo(diffbase_archive_url),
+ )
eprint("Setting up comparison repository...")
os.chdir(diffbase_repo_dir)
@@ -2458,7 +2466,7 @@
def resolve_diffbase(diffbase):
if diffbase in AUTO_DIFFBASE_VALUES:
fetch_base_branch()
- return execute_command_and_get_output(["git", "merge-base", "HEAD", 'FETCH_HEAD']).strip()
+ return execute_command_and_get_output(["git", "merge-base", "HEAD", "FETCH_HEAD"]).strip()
elif COMMIT_RE.fullmatch(diffbase):
return diffbase
@@ -2716,12 +2724,12 @@
env=os.environ,
cwd=cwd,
errors="replace",
- stdout=subprocess.DEVNULL
- if suppress_stdout
- else None, # suppress_stdout=True when we don't want the output to be printed
- stderr=subprocess.PIPE
- if capture_stderr
- else None, # capture_stderr=True when we want exceptions to contain stderr
+ stdout=(
+ subprocess.DEVNULL if suppress_stdout else None
+ ), # suppress_stdout=True when we don't want the output to be printed
+ stderr=(
+ subprocess.PIPE if capture_stderr else None
+ ), # capture_stderr=True when we want exceptions to contain stderr
).returncode
@@ -3095,9 +3103,18 @@
def get_modified_files(git_commit):
fetch_base_branch()
- merge_base_commit = execute_command_and_get_output(["git", "merge-base", git_commit, 'FETCH_HEAD']).strip()
+ merge_base_commit = execute_command_and_get_output(
+ ["git", "merge-base", git_commit, "FETCH_HEAD"]
+ ).strip()
output = execute_command_and_get_output(
- ["git", "diff-tree", "--no-commit-id", "--name-only", "-r", "{}..{}".format(merge_base_commit, git_commit)]
+ [
+ "git",
+ "diff-tree",
+ "--no-commit-id",
+ "--name-only",
+ "-r",
+ "{}..{}".format(merge_base_commit, git_commit),
+ ]
)
return output.split("\n")
@@ -3783,7 +3800,10 @@
for test_artifact in current_test_artifacts:
local_bep_path = test_artifact.download_bep(tmpdir)
if not local_bep_path:
- # TODO: propagate errors
+ eprint(
+ f"Skipping step {test_artifact.job_id} since "
+ "{test_artifact.relative_bep_path} could not be downloaded..."
+ )
continue
for test_execution in parse_bep(local_bep_path):
@@ -3804,6 +3824,8 @@
f"{base_task}",
]
)
+ except Exception as ex:
+ eprint(f"Failed to print shard summary: {ex}")
finally:
shutil.rmtree(tmpdir)
@@ -3855,33 +3877,28 @@
class TestArtifacts:
+
def __init__(self, job_id, relative_bep_path, relative_log_paths) -> None:
self.job_id = job_id
self.relative_bep_path = relative_bep_path
self.relative_log_paths = relative_log_paths
def download_bep(self, dest_dir: str) -> str:
+ if not LOG_BUCKET:
+ return None
+
job_dir = os.path.join(dest_dir, self.job_id)
os.makedirs(job_dir)
+ # We cannot use `buildkite agent download *` since it cannot handle backslashes in Windows artifact paths.
+ # Consequently, we just escape all backslashes and download files directly from GCS.
+ url = "/".join([LOG_BUCKET, self.job_id, self.relative_bep_path.replace("\\", "%5C")])
try:
- execute_command(
- [
- "buildkite-agent",
- "artifact",
- "download",
- f"*/{_TEST_BEP_FILE}",
- job_dir,
- "--step",
- self.job_id,
- ]
- )
+ return download_file(url, job_dir, _TEST_BEP_FILE)
except:
# TODO: handle exception
return None
- return os.path.join(job_dir, self.relative_bep_path)
-
def get_test_file_paths(job_id):
bep_path = None
@@ -4027,9 +4044,7 @@
def format_shard(s):
overall, statistics = s.get_details()
- return (
- f"{format_test_status(overall)} {statistics}: [log]({get_log_url_for_shard(s)})"
- )
+ return f"{format_test_status(overall)} {statistics}: [log]({get_log_url_for_shard(s)})"
failing_shards = [s for s in self.shards if s.overall_status != "PASSED"]
if len(failing_shards) == 1:
