Personal/l00800392/get 1119#977
Conversation
|
👋 Hi! Thank you for contributing to the TileLang project. Please remember to run We appreciate you taking this step! Our team will review your contribution, and we look forward to your awesome work! 🚀 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements a code coverage tracking system for both Python and C++ source code, adding configuration files, documentation, and orchestration scripts. The review feedback focuses on best practices such as excluding generated reports from version control, improving the reliability of the coverage parsing script, ensuring proper error handling in shell scripts, and using explicit Python module execution for the coverage tool.
| @@ -0,0 +1,268 @@ | |||
| # TileLang Coverage Report | |||
There was a problem hiding this comment.
This file appears to be a generated coverage report. Checking generated artifacts into the repository is generally discouraged as it leads to unnecessary repository bloat and merge conflicts. Consider adding this file to .gitignore and using a CI/CD pipeline or a dedicated coverage service to host these reports.
| match = re.match(r"^(\S+)\s+\|(\S+)\s+(\d+)\|(\S+)\s+(\d+)", line.strip()) | ||
| if match: | ||
| filename = match.group(1) | ||
| lines_rate = match.group(2) | ||
| funcs_rate = match.group(4) | ||
| print(f"| {filename} | {lines_rate} | {funcs_rate} |") |
There was a problem hiding this comment.
The current regex is fragile and may fail to parse standard lcov --list output, which typically includes parentheses for hit/total counts (e.g., 80.0% (4/5)). Using split('|') is a more robust approach to extract the filename and percentage rates.
| match = re.match(r"^(\S+)\s+\|(\S+)\s+(\d+)\|(\S+)\s+(\d+)", line.strip()) | |
| if match: | |
| filename = match.group(1) | |
| lines_rate = match.group(2) | |
| funcs_rate = match.group(4) | |
| print(f"| {filename} | {lines_rate} | {funcs_rate} |") | |
| parts = [p.strip() for p in line.split('|')] | |
| if len(parts) >= 3: | |
| filename = parts[0] | |
| lines_rate = parts[1].split()[0] if parts[1].split() else "-" | |
| funcs_rate = parts[2].split()[0] if parts[2].split() else "-" | |
| print(f"| {filename} | {lines_rate} | {funcs_rate} |") |
| # 捕获覆盖率数据 | ||
| if find build -name "*.gcda" | grep -q .; then | ||
| echo "Found .gcda files, generating coverage report..." | ||
| lcov --capture --directory build --output-file coverage_cpp.info --no-external --quiet 2>/dev/null || true |
There was a problem hiding this comment.
Removing || true ensures that the script fails immediately if coverage capture fails, preventing the generation of misleading or empty reports. Since set -e is active, it's better to let the script exit on critical failures.
| lcov --capture --directory build --output-file coverage_cpp.info --no-external --quiet 2>/dev/null || true | |
| lcov --capture --directory build --output-file coverage_cpp.info --no-external --quiet 2>/dev/null |
| if [ -f "$PROJECT_ROOT/examples/.coverage_combined" ]; then | ||
| cd "$PROJECT_ROOT" | ||
| cp examples/.coverage_combined . 2>/dev/null || true | ||
| coverage report --rcfile=.coveragerc --data-file=.coverage_combined --sort=Cover | tail -30 |
There was a problem hiding this comment.
Use python3 -m coverage instead of coverage to ensure the command is executed using the intended Python environment, as recommended in your own guild.md documentation. This also applies to line 159.
| coverage report --rcfile=.coveragerc --data-file=.coverage_combined --sort=Cover | tail -30 | |
| python3 -m coverage report --rcfile=.coveragerc --data-file=.coverage_combined --sort=Cover | tail -30 |
2 participants
No description provided.