test: strengthen focused unit coverage for core behavior#2397
Open
PeterDaveHello wants to merge 1 commit into
Open
test: strengthen focused unit coverage for core behavior#2397PeterDaveHello wants to merge 1 commit into
PeterDaveHello wants to merge 1 commit into
Conversation
Contributor
Review Summary by QodoComprehensive unit test coverage for core PR-Agent behavior and security hardening
WalkthroughsDescription• Adds comprehensive unit test coverage for core PR-Agent functionality across 15 new test modules • **Markdown and Output Rendering**: Tests for convert_to_markdown_v2, ticket extraction, and PR description generation with structural marker validation • **Diff Processing Pipeline**: Tests for hunk processing, file deletion handling, token budgeting, and pagination logic • **GitHub App Integration**: Tests for timeout dict behavior, GitHub app events, push trigger deduplication, and webhook signature verification • **Code Suggestions**: Tests for filtering, quality guards, rendering, and output formatting with truncation and score thresholds • **Ticket Extraction**: Async tests for GitHub and Azure DevOps provider integration with deduplication and caching • **Provider Helpers**: Tests for GitHub provider comment publishing, URL parsing, diff handling, and inline code suggestions • **Configuration Security**: Tests for repository settings application, CLI argument validation, and custom merge loader with forbidden directive detection • **Retry Logic**: Tests for async retry with fallback model chains and deployment ID updates • **Test Infrastructure**: Adds _settings_helpers.py module providing snapshot_settings and restore_settings utilities for reliable test isolation with Dynaconf settings • All tests use structural marker validation and fake providers to avoid network calls and LLM dependencies • Tests maintain order-independence through proper settings state management and cleanup Diagramflowchart LR
A["Test Infrastructure<br/>_settings_helpers.py"] --> B["Configuration Security<br/>Settings/CLI/Loader"]
A --> C["Output Rendering<br/>Markdown/PR Description"]
A --> D["Code Suggestions<br/>Filtering/Rendering"]
B --> E["Integration Tests<br/>Settings Application"]
C --> F["Diff Pipeline<br/>Hunk Processing"]
D --> G["Provider Helpers<br/>Comments/Suggestions"]
F --> H["Ticket Extraction<br/>Async Providers"]
G --> I["GitHub App<br/>Events/Timeouts"]
H --> J["Retry Logic<br/>Fallback Models"]
I --> K["Webhook Verification<br/>Signature Checks"]
File Changes1. tests/unittest/test_markdown_ticket_output_core.py
|
Contributor
Code Review by Qodo
1. ask_diff_hunk unset can raise
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
PeterDaveHello
force-pushed
the
harden-tests
branch
from
1ece758 to
370b0b8
Compare
Contributor
|
Persistent review updated to latest commit 370b0b8 |
![qodo-free-for-open-source-projects[bot]](https://avatars.githubusercontent.com/in/1068000?s=60&v=4){kind=small}
Comment on lines
+42
to
+45
| if original is sentinel: | ||
| settings.unset("ask_diff_hunk", force=True) | ||
| else: | ||
| settings.set("ask_diff_hunk", original) |
Contributor
There was a problem hiding this comment.
1. ask_diff_hunk unset can raise 📘 Rule violation ☼ Reliability
_restore_ask_diff_hunk calls settings.unset("ask_diff_hunk", force=True) without handling
KeyError for keys that were never set. This can break teardown and leak process-global Dynaconf
state, creating order-dependent/flaky tests.
Agent Prompt
## Issue description
A test cleanup helper unsets `ask_diff_hunk` without guarding against `KeyError` when the key was never set, which can cause teardown failures and leave process-global settings in a dirty state.
## Issue Context
Other new test helpers document that Dynaconf `settings.unset(..., force=True)` may raise `KeyError` for keys that were never set; cleanup code should treat that case as benign.
## Fix Focus Areas
- tests/unittest/test_github_app_timeout_core.py[42-45]
ⓘ 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.
Verified locally: missing ask_diff_hunk does not raise on unset, so no change needed. Please help confirm.
Cover PR processing, provider helpers, review output rendering, configuration behavior, retry handling, ticket extraction, code suggestions, and GitHub app workflows. These tests target behavior that is important and likely to change, while keeping production code untouched. They also isolate shared test state so the new coverage remains order-independent. This work was developed through multiple rounds of human-led AI collaboration using codex-cli v0.130.0 with GPT-5.5, GitHub Copilot CLI v1.0.48 with GPT-5.5, and Claude Opus 4.7.
PeterDaveHello
force-pushed
the
harden-tests
branch
from
370b0b8 to
8a7c33e
Compare
Contributor
|
Persistent review updated to latest commit 8a7c33e |
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 PR processing, provider helpers, review output rendering, configuration behavior, retry handling, ticket extraction, code suggestions, and GitHub app workflows.
These tests target behavior that is important and likely to change, while keeping production code untouched. They also isolate shared test state so the new coverage remains order-independent.
This work was developed through multiple rounds of human-led AI collaboration using codex-cli v0.130.0 with GPT-5.5, GitHub Copilot CLI v1.0.48 with GPT-5.5, and Claude Opus 4.7.
cc #2373
GitHub Copilot Pull Request Summary