Skip to content

Conversation

@furionw
Copy link
Contributor

@furionw furionw commented Jan 27, 2026

Prior

Prior to this PR, we have inconsistent guidance on where we place tests

  1. in lib/, we have src and tests at the same level
  2. in components/, we have tests inside src

This can create confusion on locating test files.

additionally, the 2nd guidance doesn't bring clarify on how test in nested folder works. e.g. say we have

components/
|-- src/dynamo
    |--planner/
        |--foo/
            |--bar.py
    |--tests/

it's unclear which option should we go for

components/
|-- src/dynamo
    |--planner/
        |--foo/
            |--bar.py
            |--tests/
                |--test_bar.py  // option 1
    |--tests/
        |--planner
            |--foo/
                |--test_bar.py  // option 2

@furionw furionw requested a review from a team as a code owner January 27, 2026 17:42
@github-actions github-actions bot added test documentation Improvements or additions to documentation ci Issues/PRs that reference CI build/test labels Jan 27, 2026
@furionw furionw force-pushed the qiwa/python_test_structure branch from 9aaa49c to 426ad0a Compare January 27, 2026 17:44
@pull-request-size pull-request-size bot added size/S and removed size/M labels Jan 27, 2026
@github-actions github-actions bot added ci Issues/PRs that reference CI build/test and removed ci Issues/PRs that reference CI build/test labels Jan 27, 2026
@github-actions github-actions bot removed the ci Issues/PRs that reference CI build/test label Jan 27, 2026
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 27, 2026

Walkthrough

This pull request expands CI coverage for component tests and reorganizes the test directory structure by consolidating Python tests from per-component nested folders into a unified layout under tests/dynamo/.

Changes

Cohort / File(s) Summary
CI Configuration
.github/filters.yaml
Added components/tests/** filter path under core section to extend CI coverage for component tests
Test Structure Documentation
tests/README.md
Reorganized documentation to reflect consolidated test layout, moving Python unit/integration tests from per-component nested tests/ folders into unified tests/dynamo/ structure with subdirectories for planner/, router/, vllm/, etc.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A rabbit hops with glee,
Tests consolidated, organized free!
CI filters expand their scope,
A better structure, a streamlined hope!

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately and concisely summarizes the main change: reorganizing Python test structure for consistency.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check ✅ Passed The PR description clearly explains the problem (inconsistent test placement conventions), provides concrete examples, and details the solution adopted from existing patterns.

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


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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@tests/README.md`:
- Around line 23-34: Update the README text under the diagram so the bullets and
explanatory table refer explicitly to the new layout shown in the diagram (use
the components/tests/dynamo/<component>/... pattern rather than vague phrasing
like <component>/tests/ or dynamo/tests/); modify the three bullet lines and any
table rows that mention test locations to use the explicit
"components/tests/dynamo/<component>/..." wording and ensure examples and
descriptions match the diagram labels so there is no ambiguity.

@furionw furionw force-pushed the qiwa/python_test_structure branch from 426ad0a to 488d327 Compare January 27, 2026 19:05
@furionw furionw requested a review from a team as a code owner January 27, 2026 19:05
@pull-request-size pull-request-size bot added size/L and removed size/S labels Jan 27, 2026
@github-actions github-actions bot added the ci Issues/PRs that reference CI build/test label Jan 27, 2026
@furionw furionw force-pushed the qiwa/python_test_structure branch from 488d327 to c4b0688 Compare January 27, 2026 19:05
@github-actions github-actions bot added ci Issues/PRs that reference CI build/test and removed ci Issues/PRs that reference CI build/test labels Jan 27, 2026
@furionw furionw force-pushed the qiwa/python_test_structure branch from c4b0688 to d435248 Compare January 27, 2026 19:06
@github-actions github-actions bot removed the ci Issues/PRs that reference CI build/test label Jan 27, 2026
@furionw furionw enabled auto-merge (squash) January 27, 2026 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/XS test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants