Skip to content

Update the test-python-binding Skill with instructions about how tests should be written#96

Merged
rapids-bot[bot] merged 3 commits into
NVIDIA:mainfrom
dagardner-nv:david-testing-skills
May 13, 2026
Merged

Update the test-python-binding Skill with instructions about how tests should be written#96
rapids-bot[bot] merged 3 commits into
NVIDIA:mainfrom
dagardner-nv:david-testing-skills

Conversation

@dagardner-nv
Copy link
Copy Markdown
Contributor

@dagardner-nv dagardner-nv commented May 13, 2026

Overview

  • This instructs the agent with our team's typical style for using pytest.
  • I confirm this contribution is my own work, or I have the right to submit it under this project's license.
  • I searched existing issues and open pull requests, and this does not duplicate existing work.

Summary by CodeRabbit

  • Documentation
    • Updated Python test style guidelines with best practices for pytest conventions, mocking strategies, fixtures, and test parametrization.

Review Change Stack

Signed-off-by: David Gardner <dagardner@nvidia.com>
Signed-off-by: David Gardner <dagardner@nvidia.com>
@dagardner-nv dagardner-nv requested a review from a team as a code owner May 13, 2026 15:51
@dagardner-nv dagardner-nv added the lang:python PR changes/introduces Python code label May 13, 2026
@dagardner-nv dagardner-nv self-assigned this May 13, 2026
@github-actions github-actions Bot added size:S PR is small and removed lang:python PR changes/introduces Python code labels May 13, 2026
…-skills

Signed-off-by: David Gardner <dagardner@nvidia.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 13, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Enterprise

Run ID: b9caa95d-4721-4375-b876-c9f1cb97034b

📥 Commits

Reviewing files that changed from the base of the PR and between 64a9710 and 808aeef.

📒 Files selected for processing (1)
  • .agents/skills/test-python-binding/SKILL.md
📜 Recent review details
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: NVIDIA/NeMo-Flow

Timestamp: 2026-05-13T15:54:40.127Z
Learning: Run full Python suite with `just test-python` before review
Learnt from: CR
Repo: NVIDIA/NeMo-Flow

Timestamp: 2026-05-13T15:54:40.127Z
Learning: Run `just build-python` for an explicit build-only pass
🔇 Additional comments (2)
.agents/skills/test-python-binding/SKILL.md (2)

36-42: LGTM!


35-35: ⚡ Quick win

Do not add @pytest.mark.asyncio to any test. Async tests are automatically detected and run by pytest-asyncio with the project's asyncio_mode = "auto" configuration; the decorator is unnecessary.


Walkthrough

The PR adds a "Python Test Style" section to the test-python-binding skill documentation, establishing concrete pytest conventions for async testing, mock naming, fixture usage, and parametrization patterns.

Changes

Python Test Style Guidance

Layer / File(s) Summary
Python Test Style guidance section
.agents/skills/test-python-binding/SKILL.md
Documents pytest conventions: avoid @pytest.mark.asyncio, omit -> None return annotations, use MagicMock/AsyncMock with mock* name prefixes, prefer fixtures over helper methods, centralize shared fixtures via conftest.py, and use pytest.mark.parametrize for parameterized tests.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes


Important

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

❌ Failed checks (1 error, 1 warning)

Check name Status Explanation Resolution
Title check ❌ Error Title exceeds 72-character limit (90 chars) and does not follow Conventional Commits format with type(scope): prefix. Reformat as 'docs(test-python-binding): add pytest style guidelines' to comply with Conventional Commits and stay under 72 characters.
Description check ⚠️ Warning Description lacks required 'Details' and 'Where should the reviewer start?' sections, and contains no 'Related Issues' entry. Add missing sections: expand 'Details' with what was changed, add 'Where should the reviewer start?' pointing to SKILL.md, and include 'Related Issues' section (even if none).
✅ Passed checks (3 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@dagardner-nv
Copy link
Copy Markdown
Contributor Author

/merge

@rapids-bot rapids-bot Bot merged commit 34731aa into NVIDIA:main May 13, 2026
22 checks passed
@dagardner-nv dagardner-nv deleted the david-testing-skills branch May 13, 2026 16:28
@willkill07 willkill07 added this to the 0.2 milestone May 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S PR is small

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants