.2059806565729300:95ad92ec965053fa925b1bca48fad429_69e3499ab13256d09ece8088.69e4d39ab13256d09ece855f.69e4d3997afba9383018b93d:Trae CN.T(2026/4/19 21:07:38)#1277
Conversation
重构集成测试脚本，增加健康检查轮询机制替代固定等待时间 添加详细的测试帮助文档和环境变量配置说明 改进日志记录和错误检测功能
添加全局和单测超时控制，改进错误分类和诊断信息 添加 pytest-timeout 依赖，支持进程树清理 优化日志输出和错误处理流程
添加 GitHub Actions 集成测试工作流，支持通过输入参数跳过构建步骤直接下载二进制文件 设置全局测试超时和分步超时控制 支持多平台(ubuntu/macos)测试环境
在构建流程中添加 doctor 检查步骤，确保环境依赖正确 新增严格模式验证脚本，检查打包内容和版本一致性 优化构建日志输出，增加详细步骤和错误处理
添加用于验证Windows安装包和zip压缩包内容一致性的脚本
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| import json | ||
| import signal | ||
| import atexit | ||
| import sys |
|
|
||
|
|
||
| GLOBAL_SERVER_PROCESS: Optional['ServerProcess'] = None | ||
| GLOBAL_TEST_TIMEOUT: int = 120 |
| for child in children: | ||
| try: | ||
| child.kill() | ||
| except psutil.NoSuchProcess: |
|
|
||
| try: | ||
| parent.kill() | ||
| except psutil.NoSuchProcess: |
| for p in still_alive: | ||
| try: | ||
| p.kill() | ||
| except psutil.NoSuchProcess: |
| p.kill() | ||
| except psutil.NoSuchProcess: | ||
| pass | ||
| except psutil.NoSuchProcess: |
| else: | ||
| try: | ||
| os.killpg(os.getpgid(pid), signal.SIGKILL) | ||
| except ProcessLookupError: |
| except OSError: | ||
| try: | ||
| os.kill(pid, signal.SIGKILL) | ||
| except ProcessLookupError: |
|
|
||
| start_time = time.time() | ||
| poll_count = 0 | ||
| last_exit_check = 0 |
| if path and os.path.exists(path): | ||
| try: | ||
| os.remove(path) | ||
| except Exception: |
Greptile SummaryThis PR adds Windows artifact verification (
Confidence Score: 3/5Three P1 defects mean the primary new verification paths are broken and should be fixed before merging. The PR's stated purpose is to add reliable Windows artifact verification. All three P1 bugs directly undermine that goal: the windows-artifact script aborts on errors (the exact case it should handle), the difference count overflows silently, and the doctor check is a no-op. The rest of the code (integration tests, verify-package.sh, CI wiring) is solid.
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[make package] --> B[package-pre-check\nvenv + submodule check]
B --> C[Build submodules\nfor each PACKAGEABLE]
C --> D[Copy to dist/activitywatch/]
D --> E[package-all.sh\nBuild zip + installer]
E --> F{PACKAGE_STRICT?}
F -- true --> G[verify-package.sh --strict\nExit non-zero on errors]
F -- false --> H[verify-package.sh\nReport only, exit 0]
G --> I{Windows?}
H --> I
I -- yes --> J[verify-windows-artifacts.sh\nCompare source dir vs zip]
J --> K{Differences?}
K -- none --> L[Exit 0 ✓]
K -- found --> M["local diff_count=\$?\n⚠ ABORTS with set -e"]
M --> N[Exit 1 ❌ unexpected]
style M fill:#f88,stroke:#c00
style N fill:#f88,stroke:#c00
Reviews (1): Last reviewed commit: "feat(scripts): 添加Windows安装包验证脚本" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| exit 0 | ||
| else | ||
| # Differences found | ||
| local diff_count=$? |
There was a problem hiding this comment.
local used outside a function — script aborts when differences are found
local is only valid inside a bash function. At the top level of a script, bash emits bash: local: can only be used in a function and returns exit code 1. Because the script runs with set -euo pipefail (line 26), that exit code causes an immediate abort the moment the else branch is taken — i.e., exactly when differences are found. The very error path that should report differences and exit 2 (strict mode) instead silently dies with exit code 1.
| local diff_count=$? | |
| diff_count=$? |
| rm -f "$missing_files" "$extra_files" "$different_sizes" | ||
|
|
||
| # Return count of differences | ||
| return $total_differences |
There was a problem hiding this comment.
return value capped at 255 — large difference counts silently become 0
Bash return only accepts values 0–255 (unsigned 8-bit). When total_differences is 256 or more, the return value wraps: 256 → 0, which makes the caller's if compare_manifests … branch succeed ("no differences"), completely hiding the real failures. Consider printing the count and returning a fixed sentinel instead:
| return $total_differences | |
| echo "$total_differences" | |
| return $(( total_differences > 0 ? 1 : 0 )) |
And in the caller update the capture accordingly:
if diff_count=$(compare_manifests "$SOURCE_MANIFEST" "$ZIP_MANIFEST" "Source Directory" "Zip Archive"); then| doctor: venv-check | ||
| @echo "===========================================================================" | ||
| @echo "ActivityWatch Build Environment Doctor" | ||
| @echo "===========================================================================" | ||
| @echo "" | ||
| @echo "--- Python Environment ---" | ||
| @echo "" | ||
| @echo -n " Python: " | ||
| @if command -v python3 >/dev/null 2>&1; then \ | ||
| PY_VER=$$(python3 --version 2>&1); \ | ||
| echo "✓ $$PY_VER"; \ | ||
| else \ | ||
| echo "✗ python3 not found in PATH"; \ | ||
| ERRORS=1; \ | ||
| fi | ||
|
|
||
| @echo -n " pip: " | ||
| @if python3 -m pip --version >/dev/null 2>&1; then \ | ||
| PIP_VER=$$(python3 -m pip --version 2>&1 | cut -d' ' -f2); \ | ||
| echo "✓ $$PIP_VER"; \ | ||
| else \ | ||
| echo "✗ pip not available"; \ | ||
| ERRORS=1; \ | ||
| fi | ||
|
|
||
| @echo -n " poetry: " | ||
| @if command -v poetry >/dev/null 2>&1; then \ | ||
| POETRY_VER=$$(poetry --version 2>&1 | sed 's/.*version \([0-9.]*\).*/\1/'); \ | ||
| echo "✓ $$POETRY_VER"; \ | ||
| else \ | ||
| echo "✗ poetry not found in PATH"; \ | ||
| echo " Install with: pip3 install poetry==1.4.2"; \ | ||
| ERRORS=1; \ | ||
| fi | ||
|
|
||
| @echo -n " setuptools: " | ||
| @SETUPTOOLS_VER=$(call setuptools_version); \ | ||
| if [ -n "$$SETUPTOOLS_VER" ] && [ "$$SETUPTOOLS_VER" != "0.0.0" ]; then \ | ||
| echo "✓ $$SETUPTOOLS_VER"; \ | ||
| NEEDS_UPDATE=$(call version_gt,49.1.1,$$SETUPTOOLS_VER); \ | ||
| if [ "$$NEEDS_UPDATE" = "1" ]; then \ | ||
| echo " ⚠ Version <= 49.1.1, may cause issues (see: pypa/setuptools#1963)"; \ | ||
| echo " Will be automatically updated during make build"; \ | ||
| fi; \ | ||
| else \ | ||
| echo "✗ Could not determine setuptools version"; \ | ||
| ERRORS=1; \ | ||
| fi | ||
|
|
||
| @echo "" | ||
| @echo "--- Node.js Environment (for web UI) ---" | ||
| @echo "" | ||
| @echo -n " node: " | ||
| @if command -v node >/dev/null 2>&1; then \ | ||
| NODE_VER=$$(node --version 2>&1); \ | ||
| echo "✓ $$NODE_VER"; \ | ||
| else \ | ||
| echo "⚠ node not found in PATH (only needed for web UI build)"; \ | ||
| echo " SKIP_WEBUI=true can be used to skip web UI build"; \ | ||
| fi | ||
|
|
||
| @echo -n " npm: " | ||
| @if command -v npm >/dev/null 2>&1; then \ | ||
| NPM_VER=$$(npm --version 2>&1); \ | ||
| echo "✓ $$NPM_VER"; \ | ||
| else \ | ||
| echo "⚠ npm not found (only needed for web UI build)"; \ | ||
| fi | ||
|
|
||
| @echo "" | ||
| @echo "--- Rust Environment (for aw-server-rust) ---" | ||
| @echo "" | ||
| @echo -n " rustc: " | ||
| @if command -v rustc >/dev/null 2>&1; then \ | ||
| RUST_VER=$$(rustc --version 2>&1); \ | ||
| echo "✓ $$RUST_VER"; \ | ||
| else \ | ||
| echo "⚠ rustc not found in PATH (only needed for aw-server-rust)"; \ | ||
| echo " SKIP_SERVER_RUST=true can be used to skip Rust build"; \ | ||
| fi | ||
|
|
||
| @echo -n " cargo: " | ||
| @if command -v cargo >/dev/null 2>&1; then \ | ||
| CARGO_VER=$$(cargo --version 2>&1 | cut -d' ' -f2); \ | ||
| echo "✓ $$CARGO_VER"; \ | ||
| else \ | ||
| echo "⚠ cargo not found (only needed for aw-server-rust)"; \ | ||
| fi | ||
|
|
||
| @echo "" | ||
| @echo "--- Git Submodules ---" | ||
| @echo "" | ||
| @for module in aw-core aw-client aw-server; do \ | ||
| echo -n " $$module: "; \ | ||
| if [ -d "$$module/.git" ]; then \ | ||
| echo "✓ initialized"; \ | ||
| else \ | ||
| echo "✗ not initialized"; \ | ||
| echo " Run: git submodule update --init --recursive"; \ | ||
| ERRORS=1; \ | ||
| fi; \ | ||
| done | ||
|
|
||
| @echo "" | ||
| @echo "===========================================================================" | ||
| @echo "Summary" | ||
| @echo "===========================================================================" | ||
| @echo "" | ||
| @echo "Virtual environment: ✓ Active" | ||
| @echo " Path: $$VIRTUAL_ENV" | ||
| @echo "" | ||
| @if [ -n "$$ERRORS" ]; then \ | ||
| echo "❌ Some issues found. Please fix them before building."; \ | ||
| exit 1; \ | ||
| else \ | ||
| echo "✅ All required dependencies look good!"; \ | ||
| echo ""; \ | ||
| echo " You can now run:"; \ | ||
| echo " make build"; \ | ||
| echo ""; \ | ||
| echo " Or with options:"; \ | ||
| echo " make build SKIP_WEBUI=true"; \ | ||
| echo " make build SKIP_SERVER_RUST=true"; \ | ||
| echo " make build RELEASE=true"; \ | ||
| echo ""; \ | ||
| fi |
There was a problem hiding this comment.
ERRORS variable never survives across recipe lines — doctor always exits 0
Each line in a Makefile recipe is executed in a separate shell. The ERRORS=1 assignments set in individual @if blocks are lost by the time the final @if [ -n "$$ERRORS" ] check runs, which always sees an empty variable and always exits 0. This means make doctor reports "✅ All required dependencies look good!" even when python3, poetry, or cargo are missing, making the "fail-fast" CI check ineffective.
Fix by collapsing the entire doctor body into a single recipe line joined with ; \ continuations, or by delegating to a shell script that maintains a single shell environment.
| runs-on: ${{ matrix.os }} | ||
| timeout-minutes: 15 | ||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| os: [ubuntu-latest, macos-latest] | ||
| python_version: ['3.12'] |
There was a problem hiding this comment.
The new integration job only targets ubuntu-latest and macos-latest, but the PR's primary motivation is verifying Windows artifacts (verify-windows-artifacts.sh, PACKAGE_STRICT CI changes). Windows integration coverage is absent from this job.
Consider adding windows-latest to the matrix so Windows-specific paths (7z compression, installer creation, aw-tauri.exe checks) are exercised in CI.
| log_ok "Inno Setup compilation complete" | ||
|
|
||
| log_action "Renaming installer: activitywatch-setup.exe → $installer_filename" | ||
| mv dist/activitywatch-setup.exe "dist/$installer_filename" | ||
| log_ok "Installer renamed" |
There was a problem hiding this comment.
The rename assumes the Inno Setup compiler always writes to dist/activitywatch-setup.exe. If the .iss file specifies a different OutputBaseFilename or OutputDir, the mv will fail with a "No such file or directory" error. It would be safer to check for the file's existence before renaming, or to read the actual output location from the ISS script.
3 participants
添加Windows安装包验证脚本
添加用于验证Windows安装包和zip压缩包内容一致性的脚本