Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@ Runs on `http://localhost:3000` by default.
5. Use English for all new comments.
6. For path handling, use `pathlib.Path` instead of string paths, and use `astrbot.core.utils.path_utils` to get the AstrBot data and temp directory.

## Testing

When you modify functionality, add or update a corresponding test and run it locally (e.g. `uv run pytest tests/path/to/test_xxx.py --cov=astrbot.xxx`).
Use `--cov-report term-missing` or similar to generate coverage information.
Comment on lines +31 to +34
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The PR description states that the testing section is removed from AGENTS.md, but the diff shows it being added. This seems to be a contradiction. Please clarify if this section should be removed or added.



Comment on lines +31 to +36
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR description states "Removed testing section from AGENTS.md (moved to separate documentation)" but this diff shows that a testing section is being added, not removed. This is a direct contradiction between the PR description and the actual code changes. Please clarify whether the testing documentation should be added or removed, and update either the PR description or the code changes to be consistent.

Copilot uses AI. Check for mistakes.
## PR instructions

1. Title format: use conventional commit messages
Expand Down
31 changes: 30 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
.PHONY: worktree worktree-add worktree-rm
.PHONY: worktree worktree-add worktree-rm test test-unit test-integration test-cov test-quick
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (bug_risk):test-file 加入 .PHONY 列表,以避免和同名文件/目录发生冲突。

由于 test-file 是一个工具型目标而不是真实文件,它也应该被加入 .PHONY,这样可以确保配方总是会执行,而不会在仓库中未来出现名为 test-file 的文件或目录时被跳过。

Suggested change
.PHONY: worktree worktree-add worktree-rm test test-unit test-integration test-cov test-quick
.PHONY: worktree worktree-add worktree-rm test test-unit test-integration test-cov test-quick test-file
Original comment in English

suggestion (bug_risk): Add test-file to the .PHONY list to avoid accidental file/dir conflicts.

Because test-file is a utility target rather than a real file, it should also be in .PHONY to ensure the recipe always runs and isn’t skipped if a test-file file or directory is ever added to the repo.

Suggested change
.PHONY: worktree worktree-add worktree-rm test test-unit test-integration test-cov test-quick
.PHONY: worktree worktree-add worktree-rm test test-unit test-integration test-cov test-quick test-file

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The .PHONY declaration is updated to include new test targets, which is good for explicitly marking them as phony targets. However, the PR description mentions removing test commands from the Makefile, but this change adds several new test commands. This seems contradictory. Please clarify the intent.

.PHONY: worktree worktree-add worktree-rm

Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The .PHONY declaration includes "test-file" but this target (lines 59-61) doesn't actually run any commands that make would execute - it only echoes usage instructions. Targets that only echo messages don't typically need to be declared in .PHONY since they don't create files. However, this is a minor issue and can be left as-is for consistency.

Copilot uses AI. Check for mistakes.

WORKTREE_DIR ?= ../astrbot_worktree
BRANCH ?= $(word 2,$(MAKECMDGOALS))
Expand Down Expand Up @@ -30,3 +30,32 @@ endif
# Swallow extra args (branch/base) so make doesn't treat them as targets
%:
@true

# ============================================================
# 测试命令
# ============================================================

# 运行所有测试
test:
uv run pytest tests/ -v

# 运行单元测试
test-unit:
uv run pytest tests/ -v -m "unit and not integration"
Comment on lines +42 to +44
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This command uses pytest marker "unit" which is not defined in the codebase. Running this command will either fail or produce warnings about unknown markers. Based on my search of the codebase, there are no @pytest.mark.unit decorators on any tests, no pytest.ini or pyproject.toml [tool.pytest.ini_options] section defining these markers, and no conftest.py registering them. This command needs to be updated to work with the actual test structure, or the appropriate markers need to be registered in pytest configuration.

Suggested change
# 运行单元测试
test-unit:
uv run pytest tests/ -v -m "unit and not integration"
# 运行单元测试(所有未标记为 integration 的测试)
test-unit:
uv run pytest tests/ -v -m "not integration"

Copilot uses AI. Check for mistakes.

# 运行集成测试
test-integration:
uv run pytest tests/integration/ -v -m integration

# 运行测试并生成覆盖率报告
test-cov:
uv run pytest tests/ --cov=astrbot --cov-report=term-missing --cov-report=html -v

# 快速测试(跳过慢速测试和集成测试)
test-quick:
uv run pytest tests/ -v -m "not slow and not integration" --tb=short
Comment on lines +44 to +56
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This command references a non-existent "tests/integration/" directory and uses an undefined "integration" pytest marker. Based on my search, the tests directory structure is: tests/, tests/agent/, and various test files directly under tests/. There is no tests/integration/ subdirectory. Additionally, no tests in the codebase use @pytest.mark.integration. This command will fail when executed.

Suggested change
uv run pytest tests/ -v -m "unit and not integration"
# 运行集成测试
test-integration:
uv run pytest tests/integration/ -v -m integration
# 运行测试并生成覆盖率报告
test-cov:
uv run pytest tests/ --cov=astrbot --cov-report=term-missing --cov-report=html -v
# 快速测试(跳过慢速测试和集成测试
test-quick:
uv run pytest tests/ -v -m "not slow and not integration" --tb=short
uv run pytest tests/ -v -m "unit"
# 运行集成测试(当前未定义集成测试)
test-integration:
@echo "当前未定义集成测试用例,跳过运行。"
# 运行测试并生成覆盖率报告
test-cov:
uv run pytest tests/ --cov=astrbot --cov-report=term-missing --cov-report=html -v
# 快速测试(跳过慢速测试
test-quick:
uv run pytest tests/ -v -m "not slow" --tb=short

Copilot uses AI. Check for mistakes.
Comment on lines +44 to +56
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This command uses pytest markers "slow" and "integration" which are not defined in the codebase. No tests use @pytest.mark.slow or @pytest.mark.integration decorators, and these markers are not registered in any pytest configuration. This command will either fail or produce warnings about unknown markers when executed.

Suggested change
uv run pytest tests/ -v -m "unit and not integration"
# 运行集成测试
test-integration:
uv run pytest tests/integration/ -v -m integration
# 运行测试并生成覆盖率报告
test-cov:
uv run pytest tests/ --cov=astrbot --cov-report=term-missing --cov-report=html -v
# 快速测试(跳过慢速测试和集成测试
test-quick:
uv run pytest tests/ -v -m "not slow and not integration" --tb=short
uv run pytest tests/ -v
# 运行集成测试
test-integration:
uv run pytest tests/integration/ -v
# 运行测试并生成覆盖率报告
test-cov:
uv run pytest tests/ --cov=astrbot --cov-report=term-missing --cov-report=html -v
# 快速测试(当前不使用未定义的慢速/集成标记,仅缩短回溯输出
test-quick:
uv run pytest tests/ -v --tb=short

Copilot uses AI. Check for mistakes.

# 运行特定测试文件
test-file:
@echo "Usage: uv run pytest tests/path/to/test_file.py -v"
@echo "Example: uv run pytest tests/test_main.py -v"
Comment on lines +34 to +61
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The PR description states that test commands are removed from the Makefile to encourage direct uv run pytest usage. However, this diff introduces a comprehensive set of new make targets for testing. This contradicts the stated goal of removing test commands and encouraging direct pytest usage. If the intention is to provide these make targets for convenience, the PR description should be updated to reflect this, and the original AGENTS.md section (which was removed in the other diff) should probably be kept or moved to a more appropriate location, as it provided guidance on uv run pytest usage.

Comment on lines +33 to +61
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR description states "Removed test commands from Makefile (use uv run pytest directly)" but this diff shows that test commands are being added, not removed. This contradicts the stated purpose of the PR. Please clarify the actual intent and update either the PR description or the code changes accordingly.

Copilot uses AI. Check for mistakes.
Loading