Copilot commented on code in PR #8746: URL: https://github.com/apache/hbase/pull/8746#discussion_r4203076432
########## dev-support/read-replica/python/scripts/render_console_report.py: ########## @@ -0,0 +1,307 @@ +#!/usr/bin/env python3 +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +""" +Generate a Yetus-style console report for the read-replica integration tests. + +Reads infrastructure timing from a KEY=VALUE env file and per-test results from +JUnit XML, then produces an HTML fragment matching the format used by the +upstream HBase Nightly Build (Apache Yetus). +""" + +from __future__ import annotations + +import argparse +import sys +import xml.etree.ElementTree as ET +from dataclasses import dataclass +from pathlib import Path + + +@dataclass +class TestResult: + name: str + time_sec: float + passed: bool + rerun_count: int = 0 + + +def parse_timing_env(path: Path) -> dict[str, int]: + timing: dict[str, int] = {} + if not path.exists(): + return timing + for line in path.read_text(encoding="utf-8").splitlines(): + line = line.strip() + if not line or line.startswith("#"): + continue + if "=" not in line: + continue + key, _, value = line.partition("=") + try: + timing[key.strip()] = int(value.strip()) + except ValueError: + pass + return timing + + +def parse_junit_xml(path: Path) -> list[TestResult]: + tree = ET.parse(path) + root = tree.getroot() + + testcases: list[tuple[str, float, bool]] = [] + for tc in root.iter("testcase"): + name = tc.get("name", "unknown") + try: + time_sec = float(tc.get("time", "0")) + except ValueError: + time_sec = 0.0 + has_failure = tc.find("failure") is not None or tc.find("error") is not None + testcases.append((name, time_sec, not has_failure)) + + seen_order: list[str] = [] + entries: dict[str, list[tuple[float, bool]]] = {} + for name, time_sec, passed in testcases: + if name not in entries: + seen_order.append(name) + entries[name] = [] + entries[name].append((time_sec, passed)) + + results: list[TestResult] = [] + for name in seen_order: + runs = entries[name] + total_time = sum(t for t, _ in runs) + final_passed = runs[-1][1] + rerun_count = len(runs) - 1 + results.append(TestResult(name=name, time_sec=total_time, + passed=final_passed, rerun_count=rerun_count)) + return results + + +def format_runtime(total_sec: int | float) -> str: + total_sec = int(round(total_sec)) + minutes = total_sec // 60 + seconds = total_sec % 60 + return f"{minutes:>3d}m {seconds:>2d}s" + + +def _log_link(test_name: str, color: str, logs_url: str, + output_dir: Path) -> str: + test_log_dir = output_dir / test_name + if not test_log_dir.is_dir(): + return "" + if logs_url: + href = f"{logs_url.rstrip('/')}/{test_name}/" + else: + href = f"{test_name}/" + return f'<font color="{color}"><a href="{href}">/{test_name}/</a></font>' + + +def _row(vote: str, color: str, subsystem: str, runtime: str, + log: str, comment: str) -> str: + return ( + "<tr>\n" + f'\t\t<td><font color="{color}">{vote}</font></td>\n' + f'\t\t<td><font color="{color}"> {subsystem} </font></td>\n' + f'\t\t<td><font color="{color}">{runtime}</font></td>\n' + f"<td>{log}</td>\n" + f'<td><font color="{color}"> {comment} </font></td>\n' + "</tr>\n" + ) + + +def _infra_row(subsystem: str, seconds: int, comment: str) -> str: + return _row("0", "blue", subsystem, format_runtime(seconds), "", comment) + + +def _detail_row(color: str, log: str, comment: str) -> str: + return ( + "<tr>\n" + "\t\t<td></td>\n" + "\t\t<td></td>\n" + "\t\t<td></td>\n" + f"<td>{log}</td>\n" + f'<td><font color="{color}"> {comment} </font></td>\n' + "</tr>\n" + ) + + +def _pytest_rows(test_results: list[TestResult], timing: dict[str, int], + logs_url: str, output_dir: Path) -> list[str]: + rows: list[str] = [] + + passed_count = sum(1 for t in test_results if t.passed) + failed_count = sum(1 for t in test_results if not t.passed) + total_reruns = sum(t.rerun_count for t in test_results) + + all_passed = failed_count == 0 Review Comment: A valid zero-test JUnit file (for example, when `-k` matches nothing and pytest exits 5) yields `failed_count == 0`, so this row incorrectly reports `+1`. Require at least one parsed test before treating pytest as passed. This issue also appears on line 206 of the same file. ########## dev-support/read-replica/python/scripts/render_console_report.py: ########## @@ -0,0 +1,307 @@ +#!/usr/bin/env python3 +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +""" +Generate a Yetus-style console report for the read-replica integration tests. + +Reads infrastructure timing from a KEY=VALUE env file and per-test results from +JUnit XML, then produces an HTML fragment matching the format used by the +upstream HBase Nightly Build (Apache Yetus). +""" + +from __future__ import annotations + +import argparse +import sys +import xml.etree.ElementTree as ET +from dataclasses import dataclass +from pathlib import Path + + +@dataclass +class TestResult: + name: str + time_sec: float + passed: bool + rerun_count: int = 0 + + +def parse_timing_env(path: Path) -> dict[str, int]: + timing: dict[str, int] = {} + if not path.exists(): + return timing + for line in path.read_text(encoding="utf-8").splitlines(): + line = line.strip() + if not line or line.startswith("#"): + continue + if "=" not in line: + continue + key, _, value = line.partition("=") + try: + timing[key.strip()] = int(value.strip()) + except ValueError: + pass + return timing + + +def parse_junit_xml(path: Path) -> list[TestResult]: + tree = ET.parse(path) + root = tree.getroot() + + testcases: list[tuple[str, float, bool]] = [] + for tc in root.iter("testcase"): + name = tc.get("name", "unknown") + try: + time_sec = float(tc.get("time", "0")) + except ValueError: + time_sec = 0.0 + has_failure = tc.find("failure") is not None or tc.find("error") is not None + testcases.append((name, time_sec, not has_failure)) + + seen_order: list[str] = [] + entries: dict[str, list[tuple[float, bool]]] = {} + for name, time_sec, passed in testcases: + if name not in entries: + seen_order.append(name) + entries[name] = [] + entries[name].append((time_sec, passed)) + + results: list[TestResult] = [] + for name in seen_order: + runs = entries[name] + total_time = sum(t for t, _ in runs) + final_passed = runs[-1][1] + rerun_count = len(runs) - 1 + results.append(TestResult(name=name, time_sec=total_time, + passed=final_passed, rerun_count=rerun_count)) + return results + + +def format_runtime(total_sec: int | float) -> str: + total_sec = int(round(total_sec)) + minutes = total_sec // 60 + seconds = total_sec % 60 + return f"{minutes:>3d}m {seconds:>2d}s" + + +def _log_link(test_name: str, color: str, logs_url: str, + output_dir: Path) -> str: + test_log_dir = output_dir / test_name + if not test_log_dir.is_dir(): + return "" + if logs_url: + href = f"{logs_url.rstrip('/')}/{test_name}/" + else: + href = f"{test_name}/" + return f'<font color="{color}"><a href="{href}">/{test_name}/</a></font>' + + +def _row(vote: str, color: str, subsystem: str, runtime: str, + log: str, comment: str) -> str: + return ( + "<tr>\n" + f'\t\t<td><font color="{color}">{vote}</font></td>\n' + f'\t\t<td><font color="{color}"> {subsystem} </font></td>\n' + f'\t\t<td><font color="{color}">{runtime}</font></td>\n' + f"<td>{log}</td>\n" + f'<td><font color="{color}"> {comment} </font></td>\n' + "</tr>\n" + ) + + +def _infra_row(subsystem: str, seconds: int, comment: str) -> str: + return _row("0", "blue", subsystem, format_runtime(seconds), "", comment) + + +def _detail_row(color: str, log: str, comment: str) -> str: + return ( + "<tr>\n" + "\t\t<td></td>\n" + "\t\t<td></td>\n" + "\t\t<td></td>\n" + f"<td>{log}</td>\n" + f'<td><font color="{color}"> {comment} </font></td>\n' + "</tr>\n" + ) + + +def _pytest_rows(test_results: list[TestResult], timing: dict[str, int], + logs_url: str, output_dir: Path) -> list[str]: + rows: list[str] = [] + + passed_count = sum(1 for t in test_results if t.passed) + failed_count = sum(1 for t in test_results if not t.passed) + total_reruns = sum(t.rerun_count for t in test_results) + + all_passed = failed_count == 0 + + if all_passed: + vote, color = "+1", "green" + else: + vote, color = "-1", "red" + + pytest_sec = timing.get("PYTEST_SEC", 0) + if pytest_sec == 0: + pytest_sec = int(round(sum(t.time_sec for t in test_results))) + + comment = f"{passed_count} passed, {failed_count} failed, {total_reruns} rerun(s)" + rows.append(_row(vote, color, "pytest", format_runtime(pytest_sec), "", comment)) + + for t in test_results: + if not t.passed: + log = _log_link(t.name, "red", logs_url, output_dir) + rows.append(_detail_row("red", log, f"failed {t.name}")) + + for t in test_results: + if t.passed and t.rerun_count > 0: + log = _log_link(t.name, "yellow", logs_url, output_dir) + rows.append(_detail_row("yellow", log, f"rerun {t.name}")) Review Comment: The stated behavior for a mixed run is to list only failing tests, but this loop also adds every eventually-passing rerun even when another test failed. Gate these yellow detail rows on `all_passed` so mixed failures show only the failing tests. ########## dev-support/read-replica/python/test/conftest.py: ########## @@ -0,0 +1,94 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# conftest.py is a special pytest file. Fixtures and hooks defined here are +# automatically available to all tests in this directory and its subdirectories. + +import logging +import os +import shutil + +import pytest + +from python.src.logger_config import LOG_FORMAT + +DEFAULT_OUTPUT_DIR = os.path.join(os.path.dirname(__file__), '..', '..', 'output') + + [email protected](tryfirst=True, hookwrapper=True) +def pytest_runtest_makereport(item, call): + outcome = yield + report = outcome.get_result() + setattr(item, f"rep_{report.when}", report) + + +def _clear_directory(path): + if not os.path.isdir(path): + return + for entry in os.listdir(path): + entry_path = os.path.join(path, entry) + if os.path.isdir(entry_path): + shutil.rmtree(entry_path) + else: + os.remove(entry_path) + + [email protected](autouse=True) +def per_test_log_file(request): + output_dir = os.environ.get('OUTPUT_DIR', DEFAULT_OUTPUT_DIR) + test_name = request.node.name + execution_count = getattr(request.node, 'execution_count', 1) + + run_dir = os.path.join(output_dir, test_name, f"run{execution_count}") + os.makedirs(run_dir, exist_ok=True) + + log_path = os.path.join(run_dir, f"{test_name}.run{execution_count}.log") + + active_logs_dir = os.environ.get('ACTIVE_CLUSTER_LOGS_DIR') + replica_logs_dir = os.environ.get('REPLICA_CLUSTER_LOGS_DIR') + + _clear_directory(active_logs_dir) + _clear_directory(replica_logs_dir) Review Comment: These directories are not created before Compose starts. On a clean checkout Docker creates each bind-mount source as root-owned, while the HBase image runs as UID 1000, so HBase cannot create its log files and startup can fail. Create both directories and make them writable before starting the containers. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
