fix(tests): 修复 test-prj-running.sh 脚本无法构建和生成覆盖率的问题 - #295
Merged
deepin-bot[bot] merged 1 commit intoJul 21, 2026
Conversation
Reviewer's GuideThe script for running project tests and collecting coverage is refactored to reliably configure, build, execute tests, and produce coverage and report artifacts using robust path handling and clean build/report directories. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The tests are executed twice (before and after
lcov --zerocounters), which looks redundant; consider dropping the first run unless it is required for a separate purpose not related to coverage. - The
cp -r asan*.log* "${report_path}/asan_deepin-reader.log"invocation is a bit odd (recursive copy into a single file path and a glob that may expand to multiple files); consider simplifying to a non-recursive copy and handling the case of multiple ASAN log files more explicitly.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The tests are executed twice (before and after `lcov --zerocounters`), which looks redundant; consider dropping the first run unless it is required for a separate purpose not related to coverage.
- The `cp -r asan*.log* "${report_path}/asan_deepin-reader.log"` invocation is a bit odd (recursive copy into a single file path and a glob that may expand to multiple files); consider simplifying to a non-recursive copy and handling the case of multiple ASAN log files more explicitly.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
pengfeixx
force-pushed
the
fix/test-running-script
branch
from
July 21, 2026 05:29
1859eeb to
a10c2c1
Compare
Contributor
Author
|
/retest |
3 similar comments
Contributor
Author
|
/retest |
Contributor
Author
|
/retest |
Contributor
Author
|
/retest |
The cmake line was commented out and the build dir was never created,
so make had no Makefile, the test binary was missing, and lcov found
no .gcda files, producing an empty coverage.info.
Create the build dir, run cmake with BUILD_TESTS, USE_PDFIUM_BUNDLE,
Debug and CMAKE_SAFETYTEST_ARG, derive project_root via script_dir.
Build test-deepin-reader with nproc, zerocounters before run, and
emit artifacts to build-ut/{html,report,asan.log}.
脚本中 cmake 配置命令被注释、build 目录未创建,导致 make 无
Makefile,lcov 找不到 .gcda 文件,coverage.info 为空。
自动创建 build 目录并执行 cmake 配置(启用 BUILD_TESTS、
USE_PDFIUM_BUNDLE、Debug、CMAKE_SAFETYTEST_ARG)。
用 nproc 并行编译 test-deepin-reader,运行前 zerocounters,
报告固定输出到 build-ut/{html,report,asan.log}。
Log: 修复测试运行脚本无法构建和生成覆盖率的问题
Influence: 流水线可通过 test-prj-running.sh 一键产出测试与覆盖率报告。
pengfeixx
force-pushed
the
fix/test-running-script
branch
from
July 21, 2026 07:22
a10c2c1 to
9f54e14
Compare
deepin pr auto review★ 总体评分:92分 - ■ 【总体评价】 > 脚本重构显著提升了路径处理健壮性与构建流程的可靠性,实现了干净的覆盖率收集环境。 > 逻辑严谨且修复了历史配置缺失问题,无安全漏洞,仅在异常处理和资源清理上存在轻微优化空间。 - ■ 【详细分析】 > > - 1.语法逻辑(完全正确)✓ > > 脚本启用了 `set -u` 防止未定义变量误用,路径解析逻辑通过 `cd && pwd` 正确获取绝对路径,构建、测试及覆盖率收集流程顺序合理。 > > 潜在问题:`cmake` 或 `make` 命令失败时,脚本不会立即退出(未启用 `set -e`),可能导致后续步骤在缺少构建产物的情况下继续执行并产生误导性错误。 > > 建议:考虑在关键构建命令后增加错误检查,或直接启用 `set -e` 以确保失败时快速终止。 - > - 2.代码质量(优秀)✓ > > 代码结构清晰,注释详尽且具有指导性,变量命名语义明确(如 `script_dir`、`project_root`),消除了旧脚本中相对路径的脆弱性,清理了历史遗留的注释代码。 > > 潜在问题:无 > > 建议:无 - > - 3.代码性能(高效)✓ > > 使用 `$(nproc)` 动态获取 CPU 核心数进行并行编译,充分利用多核资源,相比旧脚本的 `-j8` 更加智能和高效。清理旧目录保证了无冗余干扰。 > > 潜在问题:连续两次运行 `./tests/test-deepin-reader`(一次为生成初始报告,一次为刷新覆盖率数据),可能导致整体执行时间略微增加。 > > 建议:若测试执行时间较长,可评估是否合并运行步骤,或在第一次运行后直接重置计数器并复用结果。 - > - 4.代码安全(存在0个安全漏洞)✓ > > 漏洞对比统计:新增漏洞 0 个,减少漏洞 0 个,持平 0 个 > > 脚本主要用于本地或 CI 环境的自动化测试,所有变量均通过内部路径解析和固定参数控制,无外部输入注入风险。路径处理使用了双引号包裹,防止了路径包含空格导致的异常。 > > > > - 建议:无需特殊安全修复,保持现有安全编码规范。 - ■ 【改进建议代码示例】set -eu
builddir=build
reportdir=build-ut
# Resolve project root (parent of the tests/ directory that holds this script)
script_dir="$(cd "$(dirname "$0")" && pwd)"
project_root="$(cd "${script_dir}/.." && pwd)"
build_path="${project_root}/${builddir}"
report_path="${project_root}/${reportdir}"
# Fresh build directory to ensure a clean coverage run
rm -rf "${build_path}"
mkdir -p "${build_path}"
# Fresh report directory
rm -rf "${report_path}"
mkdir -p "${report_path}"
cd "${build_path}"
# Configure project with unit tests and coverage instrumentation enabled
cmake -DCMAKE_SAFETYTEST_ARG="CMAKE_SAFETYTEST_ARG_ON" \
-DDOTEST=ON \
-DBUILD_TESTS=ON \
-DUSE_PDFIUM_BUNDLE=ON \
-DCMAKE_BUILD_TYPE=Debug \
"${project_root}"
# Compile tests target
make -j"$(nproc)" test-deepin-reader
# Ensure report directory used by gtest exists inside the build tree
mkdir -p "${build_path}/report"
# Run tests and produce XML report
./tests/test-deepin-reader --gtest_output=xml:"${build_path}/report/report_deepin-reader.xml"
# Directory that holds coverage artifacts (build tree of the project)
workdir="${build_path}"
# Reset any stale coverage counters before recapture
lcov --directory "${workdir}" --zerocounters || true
# Re-run tests so .gcda files reflect a clean run
./tests/test-deepin-reader --gtest_output=xml:"${build_path}/report/report_deepin-reader.xml"
# Collect coverage data
lcov -d "${workdir}" -c -o ./coverage.info
# Keep only reader sources, drop tests themselves
lcov --extract ./coverage.info '*/reader/*' -o ./coverage.info
lcov --remove ./coverage.info '*/tests/*' -o ./coverage.info
# Generate HTML report
genhtml -o ./html ./coverage.info
# Rename main index for downstream tooling
mv ./html/index.html ./html/cov_deepin-reader.html
# Publish artifacts into project report dir
cp -r html "${report_path}/"
cp -r "${build_path}/report" "${report_path}/"
cp -r asan*.log* "${report_path}/asan_deepin-reader.log" 2>/dev/null || true
exit 0 |
lzwind
approved these changes
Jul 21, 2026
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: lzwind, pengfeixx The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
Contributor
Author
|
/merge |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
问题:脚本中 cmake 配置命令被注释、build 目录未创建,导致
make 无 Makefile、测试可执行文件不存在、lcov 找不到 .gcda 文件,
最终生成空的 coverage.info。
修复:
Summary by Sourcery
Ensure the test-prj-running.sh script reliably builds the project, runs unit tests, and generates valid coverage and report artifacts.
Enhancements: