Add focused unit tests for core PR-Agent flows#2373
Open
PeterDaveHello wants to merge 1 commit intoThe-PR-Agent:mainfrom
Open
Add focused unit tests for core PR-Agent flows#2373PeterDaveHello wants to merge 1 commit intoThe-PR-Agent:mainfrom
PeterDaveHello wants to merge 1 commit intoThe-PR-Agent:mainfrom
Conversation
Contributor
Review Summary by QodoAdd focused unit tests for core PR-Agent flows
WalkthroughsDescription• Add comprehensive unit tests for core PR-Agent flows • Cover routing, PR processing, reviewer labels, code suggestions • Test GitHub Action behavior, webhook helpers, GitLab URL handling • Test LiteLLM request shaping and provider configuration errors Diagramflowchart LR
A["Test Coverage"] --> B["Routing & Processing"]
A --> C["Tools & Suggestions"]
A --> D["Webhooks & Providers"]
B --> E["PR Agent routing"]
B --> F["PR processing logic"]
C --> G["Code suggestions"]
C --> H["PR description"]
D --> I["GitHub Action runner"]
D --> J["GitLab provider"]
D --> K["Webhook logic"]
File Changes1. tests/unittest/test_git_provider_utils.py
|
Contributor
Code Review by Qodo
1. GitLab webhook import crashes
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Comment on lines
+9
to
+17
| @pytest.fixture | ||
| def gitlab_webhook_module(): | ||
| settings = get_settings() | ||
| original_git_provider = settings.config.get("git_provider", None) | ||
| module = importlib.import_module("pr_agent.servers.gitlab_webhook") | ||
| try: | ||
| yield module | ||
| finally: | ||
| settings.config.git_provider = original_git_provider |
Contributor
There was a problem hiding this comment.
1. Gitlab webhook import crashes 🐞 Bug ☼ Reliability
The gitlab_webhook_module fixture imports pr_agent.servers.gitlab_webhook without ensuring GITLAB.URL is set, but the module raises ValueError at import time when that setting is missing. This can make the new unit tests fail in environments that run with default settings (no GitLab URL configured).
Agent Prompt
## Issue description
`tests/unittest/test_webhook_logic_core.py` imports `pr_agent.servers.gitlab_webhook` inside a fixture, but `pr_agent.servers.gitlab_webhook` raises `ValueError` at import time if `GITLAB.URL` is not configured. This makes the test suite fail depending on environment defaults.
## Issue Context
`pr_agent/servers/gitlab_webhook.py` validates `GITLAB.URL` at module import (`raise ValueError("GITLAB.URL is not set")`). The test fixture currently only saves/restores `config.git_provider` and does not set/restore `GITLAB.URL`.
## Fix Focus Areas
- tests/unittest/test_webhook_logic_core.py[9-17]
## Suggested change
In the `gitlab_webhook_module` fixture:
- Save the original `GITLAB.URL` value.
- Set `GITLAB.URL` to a safe dummy value (e.g., `https://gitlab.com`) before importing.
- Optionally `importlib.reload()` the module if it may already be imported.
- Restore the original `GITLAB.URL` in `finally` (unset if it was absent).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
Contributor
Author
There was a problem hiding this comment.
Handled by setting GITLAB.URL before import and restoring GITLAB/git_provider afterward. avoiding reload() here because these tests only cover helper behavior, and forced module reinitialization would be more invasive than needed.
Cover routing, PR processing, reviewer labels, code suggestions, GitHub Action behavior, webhook helpers, GitLab URL handling, LiteLLM request shaping, provider configuration errors, and PR description helpers. These tests target behavior that is important and likely to change, while keeping production code untouched. This work was developed through human-led AI collaboration using codex-cli v0.128.0 with GPT-5.5, GitHub Copilot CLI 1.0.40 with GPT-5.5, and Claude Opus 4.7.
PeterDaveHello
force-pushed
the
add-core-unit-tests
branch
from
e253273 to
ca4ebd1
Compare
Contributor
|
Persistent review updated to latest commit ca4ebd1 |
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
2 participants
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.
Cover routing, PR processing, reviewer labels, code suggestions, GitHub Action behavior, webhook helpers, GitLab URL handling, LiteLLM request shaping, provider configuration errors, and PR description helpers.
These tests target behavior that is important and likely to change, while keeping production code untouched.
This work was developed through human-led AI collaboration using codex-cli v0.128.0 with GPT-5.5, GitHub Copilot CLI 1.0.40 with GPT-5.5, and Claude Opus 4.7.