Repository navigation
Restore --no-verify CLI flag - #1186
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Docs preview: https://84ce9e39-httpx2-docs.pydantic.workers.dev |
Merging this PR will not alter performance
Comparing Footnotes
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d307da905
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
1 issue found across 2 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="tests/httpx2/test_main.py">
<violation number="1" location="tests/httpx2/test_main.py:40">
P2: The new test only asserts that click parses `verify` into the expected boolean. The removed `test_no_verify` sent a real HTTPS request with `--no-verify` to a self-signed server and asserted `exit_code == 0`, which was the only coverage proving the flag actually disables verification end-to-end. Restore an integration test (e.g. invoking `main` against an `https_server` with `--no-verify` and asserting success) so a wiring or SSL-path regression in the very flag this PR restores is still caught.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| (["--no-verify"], False), | ||
| ], | ||
| ) | ||
| def test_verify_option(option: list[str], expected: bool) -> None: |
There was a problem hiding this comment.
P2: The new test only asserts that click parses verify into the expected boolean. The removed test_no_verify sent a real HTTPS request with --no-verify to a self-signed server and asserted exit_code == 0, which was the only coverage proving the flag actually disables verification end-to-end. Restore an integration test (e.g. invoking main against an https_server with --no-verify and asserting success) so a wiring or SSL-path regression in the very flag this PR restores is still caught.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At tests/httpx2/test_main.py, line 40:
<comment>The new test only asserts that click parses `verify` into the expected boolean. The removed `test_no_verify` sent a real HTTPS request with `--no-verify` to a self-signed server and asserted `exit_code == 0`, which was the only coverage proving the flag actually disables verification end-to-end. Restore an integration test (e.g. invoking `main` against an `https_server` with `--no-verify` and asserting success) so a wiring or SSL-path regression in the very flag this PR restores is still caught.</comment>
<file context>
@@ -28,10 +29,17 @@ def test_help() -> None:
+ (["--no-verify"], False),
+ ],
+)
+def test_verify_option(option: list[str], expected: bool) -> None:
+ with main.make_context("httpx2", ["https://example.com", *option]) as context:
+ assert context.params["verify"] is expected
</file context>
Summary
Preserve the existing
--no-verifyCLI flag while adding the explicit--verifycounterpart. Update the custom help output and cover the parsed values for both flags.Tests
scripts/test -quv run pytest tests/httpx2/test_main.py -qAI Disclaimer
This PR was developed with the assistance of either Claude or Codex. I've reviewed and verified the changes.