Skip to content

fix: remove server URL from test oracle GitHub comment#1005

Merged
myakove merged 2 commits into
mainfrom
fix/test-oracle-comment-url
Mar 1, 2026
Merged

fix: remove server URL from test oracle GitHub comment#1005
myakove merged 2 commits into
mainfrom
fix/test-oracle-comment-url

Conversation

@rnetser

@rnetser rnetser commented Mar 1, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Keep detailed server URL and status code in logs for operator debugging
  • Use a generic message in PR comments to avoid exposing internal infrastructure details
  • Updated test to match new generic comment format

Test plan

  • All 17 test_test_oracle tests pass
  • 100% coverage on test_oracle.py module

Summary by CodeRabbit

  • Bug Fixes
    • Standardized PR comment for Test Oracle health-check failures to omit the server URL and include status information (e.g., "(status 503)"), yielding a consistent, privacy-preserving message.
  • Tests
    • Updated test expectations to assert the exact, standardized health-check failure message shown in pull requests.

Keep detailed server URL and status code in logs for debugging,
but use a generic message in the PR comment to avoid exposing
internal infrastructure details.
@qodo-code-review

Copy link
Copy Markdown

Review Summary by Qodo

Remove server URL from Test Oracle GitHub comments

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Remove server URL from GitHub PR comments for security
• Keep detailed URL and status code in logs for debugging
• Update test assertions to match generic comment format
Diagram
flowchart LR
  A["Test Oracle Error"] --> B["Log with URL details"]
  A --> C["Post generic comment"]
  B --> D["Operator debugging"]
  C --> E["No infrastructure exposure"]
Loading

Grey Divider

File Changes

1. webhook_server/libs/test_oracle.py 🐞 Bug fix +4/-1

Generic error message in GitHub comments

• Changed GitHub PR comment to use generic message without server URL
• Kept detailed server URL and status code in warning logs
• Refactored comment creation call to separate lines for readability

webhook_server/libs/test_oracle.py


2. webhook_server/tests/test_test_oracle.py 🧪 Tests +1/-2

Update test assertions for generic comment

• Updated test assertion to match exact generic comment text
• Removed assertions checking for server URL and status code in comment
• Simplified test validation logic

webhook_server/tests/test_test_oracle.py


Grey Divider

Qodo Logo

@qodo-code-review

qodo-code-review Bot commented Mar 1, 2026

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0)

Grey Divider


Remediation recommended

1. Inaccurate health-failure comment🐞 Bug ✓ Correctness
Description
The PR comment always says the Test Oracle is “not responding”, even when /health returns a non-200
response (i.e., the server did respond but is unhealthy). This can mislead developers triaging
failures from the PR comment and increase debugging time.
Code

webhook_server/libs/test_oracle.py[R61-64]

+                    await asyncio.to_thread(
+                        pull_request.create_issue_comment,
+                        "Test Oracle server is not responding, skipping test analysis",
+                    )
Evidence
The code explicitly detects HTTP status failures (meaning the server responded) and even captures
the status code for logs, but the posted PR comment remains the same “not responding” message in all
cases.

webhook_server/libs/test_oracle.py[53-67]
webhook_server/tests/test_test_oracle.py[72-92]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The PR comment text says “not responding” for all health-check failures, including cases where the server responded with a non-200 status. This can mislead developers reading the PR comment.
### Issue Context
We still want to avoid exposing internal infra details (URL/status code) in PR comments, but we can make the generic wording accurate for both connectivity and unhealthy responses.
### Fix Focus Areas
- webhook_server/libs/test_oracle.py[53-67]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@myakove-bot

Copy link
Copy Markdown
Collaborator

Report bugs in Issues

Welcome! 🎉

This pull request will be automatically processed with the following features:

🔄 Automatic Actions

  • Reviewer Assignment: Reviewers are automatically assigned based on the OWNERS file in the repository root
  • Size Labeling: PR size labels (XS, S, M, L, XL, XXL) are automatically applied based on changes
  • Issue Creation: Disabled for this repository
  • Pre-commit Checks: pre-commit runs automatically if .pre-commit-config.yaml exists
  • Branch Labeling: Branch-specific labels are applied to track the target branch
  • Auto-verification: Auto-verified users have their PRs automatically marked as verified
  • Labels: All label categories are enabled (default configuration)

📋 Available Commands

PR Status Management

  • /wip - Mark PR as work in progress (adds WIP: prefix to title)
  • /wip cancel - Remove work in progress status
  • /hold - Block PR merging (approvers only)
  • /hold cancel - Unblock PR merging
  • /verified - Mark PR as verified
  • /verified cancel - Remove verification status
  • /reprocess - Trigger complete PR workflow reprocessing (useful if webhook failed or configuration changed)
  • /regenerate-welcome - Regenerate this welcome message

Review & Approval

  • /lgtm - Approve changes (looks good to me)
  • /approve - Approve PR (approvers only)
  • /automerge - Enable automatic merging when all requirements are met (maintainers and approvers only)
  • /assign-reviewers - Assign reviewers based on OWNERS file
  • /assign-reviewer @username - Assign specific reviewer
  • /check-can-merge - Check if PR meets merge requirements

Testing & Validation

  • /retest tox - Run Python test suite with tox
  • /retest build-container - Rebuild and test container image
  • /retest python-module-install - Test Python package installation
  • /retest pre-commit - Run pre-commit hooks and checks
  • /retest conventional-title - Validate commit message format
  • /retest all - Run all available tests

Container Operations

  • /build-and-push-container - Build and push container image (tagged with PR number)
    • Supports additional build arguments: /build-and-push-container --build-arg KEY=value

Cherry-pick Operations

  • /cherry-pick <branch> - Schedule cherry-pick to target branch when PR is merged
    • Multiple branches: /cherry-pick branch1 branch2 branch3

Label Management

  • /<label-name> - Add a label to the PR
  • /<label-name> cancel - Remove a label from the PR

✅ Merge Requirements

This PR will be automatically approved when the following conditions are met:

  1. Approval: /approve from at least one approver
  2. LGTM Count: Minimum 1 /lgtm from reviewers
  3. Status Checks: All required status checks must pass
  4. No Blockers: No WIP, hold, conflict labels
  5. Verified: PR must be marked as verified (if verification is enabled)

📊 Review Process

Approvers and Reviewers

Approvers:

  • myakove
  • rnetser

Reviewers:

  • myakove
  • rnetser
Available Labels
  • hold
  • verified
  • wip
  • lgtm
  • approve
  • automerge

💡 Tips

  • WIP Status: Use /wip when your PR is not ready for review
  • Verification: The verified label is automatically removed on each new commit
  • Cherry-picking: Cherry-pick labels are processed when the PR is merged
  • Container Builds: Container images are automatically tagged with the PR number
  • Permission Levels: Some commands require approver permissions
  • Auto-verified Users: Certain users have automatic verification and merge privileges

For more information, please refer to the project documentation or contact the maintainers.

@coderabbitai

coderabbitai Bot commented Mar 1, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d64abd7 and 4fedbee.

📒 Files selected for processing (2)
  • webhook_server/libs/test_oracle.py
  • webhook_server/tests/test_test_oracle.py

Walkthrough

The PR changes the health-check failure comment posted by the Test Oracle path to a new formatted string that includes status_info but omits the server URL. The test asserting that comment was updated to expect the exact new message.

Changes

Cohort / File(s) Summary
Health-check failure message
webhook_server/libs/test_oracle.py
Replaced previous dynamic comment body with f"Test Oracle server is not responding{status_info}, skipping test analysis" (status_info kept; server URL removed); posted via asyncio.to_thread.
Health-check test assertion
webhook_server/tests/test_test_oracle.py
Updated test_health_check_non_200_posts_comment to assert the exact comment: "Test Oracle server is not responding (status 503), skipping test analysis" instead of checking substring presence.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels: Review effort 3/5

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix: remove server URL from test oracle GitHub comment' directly and clearly describes the main change: removing the server URL from the GitHub comment posted by the test oracle code.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/test-oracle-comment-url

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@webhook_server/tests/test_test_oracle.py`:
- Line 91: Update the assertion in test_health_check_failure_posts_comment to
match the exact static message used elsewhere: replace the substring check that
asserts "not responding" in call_args[0][1] with an equality assertion comparing
call_args[0][1] to "Test Oracle server is not responding, skipping test
analysis" so both tests use the same exact-match style (target the assertion
inside test_health_check_failure_posts_comment that currently inspects
call_args[0][1]).

ℹ️ Review info

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a3418ad and d64abd7.

📒 Files selected for processing (2)
  • webhook_server/libs/test_oracle.py
  • webhook_server/tests/test_test_oracle.py

Comment thread webhook_server/tests/test_test_oracle.py Outdated
Only the server URL is removed from the PR comment. The HTTP
status code (e.g., "(status 503)") is still included to help
users understand the failure mode.
@rnetser

rnetser commented Mar 1, 2026

Copy link
Copy Markdown
Collaborator Author

/verified

@myakove myakove merged commit 02cc72c into main Mar 1, 2026
8 of 9 checks passed
@myakove myakove deleted the fix/test-oracle-comment-url branch March 1, 2026 14:31
@myakove-bot

Copy link
Copy Markdown
Collaborator

New container for ghcr.io/myk-org/github-webhook-server:latest published

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants