[None][test] Add doc test#13152
Conversation
📝 WalkthroughWalkthroughAdded a new integration test module that discovers Markdown files and validates all URLs within them, performing HTTP HEAD/GET checks with retry logic and concurrent execution. Updated test configuration to exclude this new test from a specific test list. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
tests/integration/test_lists/test-db/l0_h100.yml (1)
365-366: Consider moving this CPU-only doc test to a non-GPU lane.At Line 365-Line 366, the comment says no GPU is needed, but this is in an H100 post-merge suite. Moving it to a CPU-capable stage would reduce GPU queue pressure without changing coverage.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/integration/test_lists/test-db/l0_h100.yml` around lines 365 - 366, The GPU post-merge test list includes a CPU-only test identifier test_doc.py::test_url_validity; remove that line from the H100 GPU lane list and add the same test identifier to the CPU-capable test list (or a non-GPU lane) so the doc URL validation runs on CPU workers instead of occupying GPU queue; locate the string "- test_doc.py::test_url_validity" in the H100 test list and move it to the CPU lane list or equivalent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/integration/defs/test_doc.py`:
- Around line 16-241: Run the ruff formatter locally and commit the resulting
changes so the file is pre-commit clean; specifically reformat
tests/integration/defs/test_doc.py (where functions like test_url_validity,
_extract_urls, and _check_url are defined) using your project's ruff/pre-commit
configuration (e.g., ruff format or pre-commit run --all-files) and then stage
and push the updated file.
- Around line 191-192: The current broad catch "except Exception as e" around
the URL check hides non-network bugs; replace it with catching only expected
network-related exceptions (for example requests.exceptions.RequestException,
urllib.error.URLError, socket.timeout, and possibly ssl.SSLError) where the code
constructs the return tuple (False, url, line_num, f"Error: {e}"), and remove or
re-raise any other unexpected exceptions so they fail the test (i.e., change the
except block to specific except clauses or a single except
RequestException/URLError that returns the error tuple and let other exceptions
propagate).
- Around line 145-149: The test currently lets non-HTTP(S) schemes like "ftp://"
fall through to HTTP validation; update the URL filtering around the `url`
variable so only "http://" or "https://" URLs are processed — for any URL that
does not start with "http" (and is not normalized from a leading "www."),
skip/continue, i.e., treat "ftp://" and other schemes as non-HTTP and continue;
adjust the existing startswith checks to normalize "www." to "https://" but
otherwise continue for any non-http scheme before running HTTP validation.
---
Nitpick comments:
In `@tests/integration/test_lists/test-db/l0_h100.yml`:
- Around line 365-366: The GPU post-merge test list includes a CPU-only test
identifier test_doc.py::test_url_validity; remove that line from the H100 GPU
lane list and add the same test identifier to the CPU-capable test list (or a
non-GPU lane) so the doc URL validation runs on CPU workers instead of occupying
GPU queue; locate the string "- test_doc.py::test_url_validity" in the H100 test
list and move it to the CPU lane list or equivalent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1489ccde-96a2-4dfc-b7f5-3bc61db9a818
📒 Files selected for processing (2)
tests/integration/defs/test_doc.pytests/integration/test_lists/test-db/l0_h100.yml
StanleySun639
force-pushed
the
add-doc-test
branch
from
23c80fe to
78c40f7
Compare
|
/bot run |
|
PR_Github #44023 [ run ] triggered by Bot. Commit: |
|
PR_Github #44023 [ run ] completed with state
|
StanleySun639
force-pushed
the
add-doc-test
branch
from
78c40f7 to
785e043
Compare
|
/bot run --skip-test |
|
PR_Github #44248 [ run ] triggered by Bot. Commit: |
|
PR_Github #44248 [ run ] completed with state
|
StanleySun639
force-pushed
the
add-doc-test
branch
from
785e043 to
9470395
Compare
|
/bot run --skip-test |
|
PR_Github #44296 [ run ] triggered by Bot. Commit: |
|
PR_Github #44296 [ run ] completed with state
|
|
doc fix in main: #13038 |
Signed-off-by: Stanley Sun <stsun@nvidia.com>
Signed-off-by: Stanley Sun <stsun@nvidia.com>
Signed-off-by: Stanley Sun <stsun@nvidia.com>
Signed-off-by: Stanley Sun <stsun@nvidia.com>
Signed-off-by: Stanley Sun <stsun@nvidia.com>
Signed-off-by: Stanley Sun <stsun@nvidia.com>
StanleySun639
force-pushed
the
add-doc-test
branch
from
9470395 to
6cb4d95
Compare
|
/bot run --stage-list "" |
|
PR_Github #44356 [ run ] triggered by Bot. Commit: |
|
PR_Github #44356 [ run ] completed with state
|
|
/bot run --stage-list "" |
1 similar comment
|
/bot run --stage-list "" |
|
PR_Github #44411 [ run ] triggered by Bot. Commit: |
|
PR_Github #44412 [ run ] triggered by Bot. Commit: |
|
PR_Github #44411 [ run ] completed with state |
|
PR_Github #44412 [ run ] completed with state |
|
/bot reuse-pipeline |
|
PR_Github #44432 [ reuse-pipeline ] triggered by Bot. Commit: |
|
PR_Github #44432 [ reuse-pipeline ] completed with state |
3 participants
Summary by CodeRabbit
Description
Test Coverage
PR Checklist
Please review the following before submitting your PR:
PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.
PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.
Test cases are provided for new code paths (see test instructions)
Any new dependencies have been scanned for license and vulnerabilities
CODEOWNERS updated if ownership changes
Documentation updated as needed
Update tava architecture diagram if there is a significant design change in PR.
The reviewers assigned automatically/manually are appropriate for the PR.
Please check this after reviewing the above items as appropriate for this PR.
GitHub Bot Help
To see a list of available CI bot commands, please comment
/bot help.