Skip to content

test: skip GPU tests when CUDA is unavailable - #679

Merged
binaryaaron merged 4 commits into
mainfrom
binaryaaron/auto-skip-gpu-tests
Aug 7, 2026
Merged

test: skip GPU tests when CUDA is unavailable#679
binaryaaron merged 4 commits into
mainfrom
binaryaaron/auto-skip-gpu-tests

Conversation

@binaryaaron

@binaryaaron binaryaaron commented Jul 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • automatically skip requires_gpu tests when CUDA is unavailable
  • avoid importing PyTorch or probing CUDA when no GPU tests are collected
  • document the centralized behavior and cover it with hardware-independent regression tests

Test plan

  • focused pytest configuration tests
  • mise run format
  • mise run check
  • CPU-only pod with zero CUDA devices: all 5 collected requires_gpu tests skipped as intended
  • mise run test: 1,688 passed and 5 skipped; unrelated telemetry failures are tracked separately
  • run on macOS

Summary by CodeRabbit

  • New Features
    • Added automatic GPU test handling: tests skip without CUDA and run when CUDA is available.
    • Added a GPU CI mode with clear errors for missing PyTorch or unavailable CUDA.
  • Documentation
    • Updated testing and GPU smoke-test guidance for automatic GPU test handling.
  • Tests
    • Added coverage for GPU test behavior, CI validation, and avoiding unnecessary CUDA checks.
  • Chores
    • Updated GPU validation and CI workflows for more reliable environment checks.

@coderabbitai

coderabbitai Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: f94f18d5-9732-42b9-949c-1b268a669c89

📥 Commits

Reviewing files that changed from the base of the PR and between 06fab77 and 00cc334.

📒 Files selected for processing (1)
  • tests/generation/test_vllm_shutdown.py
📜 Recent review details
⏰ Context from checks skipped due to timeout. (11)
  • GitHub Check: Unit Tests (3.14)
  • GitHub Check: Unit Tests (3.11)
  • GitHub Check: Unit Tests (3.13)
  • GitHub Check: Unit Tests (3.12)
  • GitHub Check: Smoke Tests
  • GitHub Check: End-user Wheel Install
  • GitHub Check: Greptile Review
  • GitHub Check: Analyze (Python)
  • GitHub Check: Typecheck
  • GitHub Check: Analyze (python)
  • GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (10)
**/*.{md,markdown,py}

📄 CodeRabbit inference engine (.cursor/rules/agent-markdown-style.mdc)

**/*.{md,markdown,py}: Avoid decorative bold (**text**) in list items, body text, and docstrings; use structural cues (headers, list markers, colons, backticks) for emphasis instead
Use backticks for code identifiers, paths, and CLI commands in markdown and docstrings

Files:

  • tests/generation/test_vllm_shutdown.py
tests/**

📄 CodeRabbit inference engine (.cursor/rules/repo-navigation.mdc)

tests/**: Mirror src/ directory structure in tests/ directory for test organization
Auto-mark tests by directory: tests/e2e/e2e, tests/smoke/smoke, otherwise default to unit

Files:

  • tests/generation/test_vllm_shutdown.py
**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

**/*.py: Use uv for Python dependency management and execution; never use pip or raw python. Target Python 3.11–3.14 and use modern syntax such as X | Y, list[str], and Self.
Run project tooling through mise tasks or wrapper scripts in tools/; do not invoke ruff or ty directly. Use uv run for Python execution.
Put durable implementation guidance in public function and class docstrings; put local invariants in source comments.
Use Python 3.11–3.14-compatible modern type syntax, including X | Y, list[str], and Self.

**/*.py: Use American English spelling in Python code, comments, and documentation; use from __future__ import annotations in every module.
Use BaseSettings for environment/CLI settings; prefer AliasChoices for fields accepting both Python and environment-variable names.
Pydantic model fields must include Field(description=...); prefer assignment-style Field() and use Annotated only for additional metadata or constraints.
Use @dataclass(frozen=True) for immutable value objects and validators, and field(default_factory=...) for mutable defaults; never use mutable default values directly.
Use StrEnum for string-valued configuration or serialization enums and plain Enum for internal-only constants.
Obtain loggers with observability.get_logger(__name__); do not call logging.getLogger() or structlog.get_logger() directly.
Do not use print() for operational library output; use the approved logger, click.echo() for CLI output, or sys.stdout.write() for raw tool output.
Use logger extra={} for metrics, counts, durations, and other data intended for machine querying; use f-strings for human-readable context.
Raise known errors through the Safe Synthesizer custom hierarchy, using dual inheritance where callers should also catch a built-in exception.
Use X | Y, built-in generic types, Self for fluent returns, collection ABCs for arguments, Protocol for structural boundaries, and avoid Any.
Pre...

Files:

  • tests/generation/test_vllm_shutdown.py
**/tests/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Use the unit pytest marker instead of the deprecated unit_test marker; async tests do not need @pytest.mark.asyncio because asyncio_mode = auto.

Files:

  • tests/generation/test_vllm_shutdown.py
**/*

📄 CodeRabbit inference engine (AGENTS.md)

**/*: Read AGENTS.local.md if it exists and give its instructions top priority.
When a task matches a repository-specific skill, read the corresponding skill under .agents/skills/ instead of duplicating its workflow instructions.
Do not commit unless the user explicitly asks for a commit or PR work.
When committing, require DCO sign-off and GPG signing using git commit --signoff --gpg-sign (or -s -S); never manually add Signed-off-by or use --no-gpg-sign.
Use feature branches based on main; branch names commonly include an issue-number prefix such as <author>/123-short-name.
For recurring testing, building, syncing, bootstrapping, worktree, and GitHub workflows, use the matching skill under .agents/skills/.
For a full GPU/development environment, use uv sync --frozen --extra cu129 --extra engine --group dev; bare uv sync --frozen is incomplete and can cause ty, import-check, and GPU-test failures.

End files with a newline, contain no trailing whitespace, use one space between sentences, and keep code, comments, and docstrings within 120 characters.

Files:

  • tests/generation/test_vllm_shutdown.py

⚙️ CodeRabbit configuration file

**/*: Review as a senior maintainer for NeMo Safe Synthesizer. Prioritize issues that can change behavior, break user workflows, weaken privacy guarantees, hide failures, make tests unreliable, or create maintenance risk. Avoid generic style commentary unless it points to a concrete project convention that automated tools will not catch.
Comment only when the finding is actionable and tied to changed code. For each finding, state the impact, the condition that triggers it, and the smallest practical fix. Prefer one precise comment over broad advice. Do not ask for refactors outside the PR scope unless the changed code creates the problem.
Review type guidance: - Potential issue: use for correctness bugs, data loss, privacy leaks,
security risks, broken public APIs, invalid config behavior, missing
validation, hidden failures, nondeterministic tests, or CI breakage.

  • Refactor suggestion: use for local maintainability problems introduced
    by the diff when they have clear future cost, such as duplicated setup,
    unclear boundaries, over-mocking, avoidable complexity, or opaque test
    helpers.
  • Nitpick: avoid in chill mode. Do not emit formatting, import-order,
    wording, or style-only comments unless automated tools cannot catch the
    issue and it affects maintainability.

Severity guidance: - Critical: security/privacy leaks, data loss, training/test/holdout
contamination, or broken release/package/core pipeline execution.

  • Major: incorrect generation/training/evaluation behavior, broken
    CLI/SDK public API, invalid config defaults or validators, or GPU/vLLM
    cleanup and process-isolation bugs likely to fail CI or production
    runs.
  • Minor: localized bugs, missing focused tests for changed behavior, or
    bad test patterns that weaken regression coverage.
  • Trivial: small cleanup with no behavior impact. Usually suppress in
    chill mode.
  • Info: context only. Avoid unless it helps reviewers understand risk.
    Safe-Synthesizer-specific review focus: - Data ...

Files:

  • tests/generation/test_vllm_shutdown.py
**/*.{md,py,sh,toml,yml,yaml,Dockerfile}

📄 CodeRabbit inference engine (AGENTS.md)

Follow the detailed language and file-format conventions defined in STYLE_GUIDE.md.

Files:

  • tests/generation/test_vllm_shutdown.py
tests/**/*.py

📄 CodeRabbit inference engine (STYLE_GUIDE.md)

tests/**/*.py: Use absolute imports in tests.
Name test files test_*.py, classes Test*, and functions test_<module>_<expected_behavior>; prefix fixtures with fixture_ and give each fixture a one-line purpose docstring.
Use function-scoped fixtures by default, bare assert, pytest.raises(match=...), pytest.approx(), and pytest.mark.parametrize for input combinations.
Use tmp_path for file operations, mock only external boundaries, avoid shared mutable state and order dependencies, and mark CUDA-dependent tests appropriately.

All existing tests must pass before submitting a pull request; new features must include tests and bug fixes must include regression tests.

tests/**/*.py: Every test must have exactly one category marker: unit, smoke, or e2e; use additional markers such as slow, requires_gpu, or vllm only as modifiers.
Use pytest.importorskip for optional dependencies that require specific extras, such as sentence_transformers and vllm.
Use the shared dataset loaders load_test_dataset and load_test_dataframe from the root test configuration instead of duplicating dataset-loading logic.
Use the fixture_ prefix consistently for dataset and tokenizer fixtures.
Use fixture_mock_processor or fixture_mock_processor_without_valid_records for processor mocks, with ParsedResponse fields valid_records, invalid_records, errors, and integer prompt_number.
Tests that require optional GPU resources should use requires_gpu; ordinary local runs should skip them when CUDA is unavailable.
Mirror the source structure in the test directory, such as placing training, generation, and evaluation tests under corresponding directories.
Use relative imports from a directory's conftest.py when sharing non-fixture methods across test files; do not import shared helpers directly from another test file.

Files:

  • tests/generation/test_vllm_shutdown.py

⚙️ CodeRabbit configuration file

Review tests against tests/TESTING.md. Check marker usage, fixture naming, tmp_path usage, determinism, and GPU/vLLM process-isolation requirements. Flag slop tests that only check that code runs, assert result is not None when stronger invariants exist, over-mock internal implementation details, patch around the bug instead of reproducing it, or add broad snapshot/golden churn without a clear contract. Flag change detector tests that fail on harmless refactors, formatting, record ordering, incidental wording, or private implementation details without demonstrating a behavior regression. Prefer existing fixtures or focused new fixtures for repeated setup; keep tests DRY when reasonable without making the behavior under test opaque. print() is allowed in tests.

Files:

  • tests/generation/test_vllm_shutdown.py
**/*.{py,sh,yaml,yml,md}

📄 CodeRabbit inference engine (STYLE_GUIDE.md)

Include the required SPDX copyright and Apache-2.0 license headers, using comment syntax appropriate to the file format.

Include SPDX copyright headers in all source files, except files explicitly listed in .copyrightignore.

Files:

  • tests/generation/test_vllm_shutdown.py
**/*.{py,pyi}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*.{py,pyi}: Keep shared Python package code compatible with Python 3.11; do not use Python 3.12-only syntax such as PEP 695 type statements or bracketed generic parameters.
Use the repository's pinned Ruff tasks for Python formatting, import sorting, and linting rather than invoking unpinned tools directly.
Run the repository's pinned ty type checker and maintain type-correct Python code.

Files:

  • tests/generation/test_vllm_shutdown.py
**/*.{py,sh,yaml,yml,toml,md}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Use mise tasks with the repository's pinned tool versions for formatting, checking, and testing before submitting changes.

Files:

  • tests/generation/test_vllm_shutdown.py
🧠 Learnings (5)
📚 Learning: 2026-05-27T22:20:37.354Z
Learnt from: kendrickb-nvidia
Repo: NVIDIA-NeMo/Safe-Synthesizer PR: 520
File: tests/generation/test_vllm_backend.py:556-587
Timestamp: 2026-05-27T22:20:37.354Z
Learning: In NVIDIA-NeMo/Safe-Synthesizer, `tests/conftest.py`’s `pytest_collection_modifyitems` hook applies pytest category markers automatically based on each test file’s path: tests under `/e2e/` get `pytest.mark.e2e`, tests under `/smoke/` get `pytest.mark.smoke`, and all other tests get `pytest.mark.unit`. Therefore, when reviewing pytest tests outside `tests/e2e/` and `tests/smoke/`, do not flag missing explicit `pytest.mark.unit` decorators on test classes/functions as an issue (the hook will add them during collection). If a new test directory/category is introduced, ensure the hook is updated so it’s categorized correctly.

Applied to files:

  • tests/generation/test_vllm_shutdown.py
📚 Learning: 2026-07-27T22:07:22.590Z
Learnt from: binaryaaron
Repo: NVIDIA-NeMo/Safe-Synthesizer PR: 673
File: tests/pii_replacer/test_edit.py:327-327
Timestamp: 2026-07-27T22:07:22.590Z
Learning: When tests read structured logging context from Python `logging.LogRecord` instances, don’t access `record.ctx` directly (it isn’t declared on `LogRecord` and will break static typing). Instead, use `getattr(record, "ctx", default)` (or an appropriate fallback) to safely handle cases where `ctx` may or may not be attached. This applies even if Ruff rule `B009` isn’t enabled in the repo.

Applied to files:

  • tests/generation/test_vllm_shutdown.py
📚 Learning: 2026-07-29T17:12:32.642Z
Learnt from: zywind
Repo: NVIDIA-NeMo/Safe-Synthesizer PR: 654
File: tests/config/test_parameters.py:116-118
Timestamp: 2026-07-29T17:12:32.642Z
Learning: In Pydantic validation tests (e.g., models configured with `from_attributes`), when asserting failures from `model_validate(...)`, assert the structured error details (such as `ValidationError.errors()[0]["type"]`, e.g. `"model_attributes_type"`) rather than relying on the human-readable error message text. This keeps tests stable even if wording changes, while still verifying the correct validation rule is triggered.

Applied to files:

  • tests/generation/test_vllm_shutdown.py
📚 Learning: 2026-08-05T19:07:15.856Z
Learnt from: binaryaaron
Repo: NVIDIA-NeMo/Safe-Synthesizer PR: 679
File: tests/conftest.py:89-96
Timestamp: 2026-08-05T19:07:15.856Z
Learning: In the Safe Synthesizer test suite, treat CPU and CUDA installation profiles as the supported pytest collection profiles because they install PyTorch. A bare installation without PyTorch is an incomplete, unsupported profile. GPU-marked test modules may import PyTorch before tests/conftest.py::pytest_collection_modifyitems executes, so do not require that collection hook to prevent such imports.

Applied to files:

  • tests/generation/test_vllm_shutdown.py
📚 Learning: 2026-06-04T16:14:09.868Z
Learnt from: binaryaaron
Repo: NVIDIA-NeMo/Safe-Synthesizer PR: 526
File: tests/generation/test_vllm_backend.py:399-509
Timestamp: 2026-06-04T16:14:09.868Z
Learning: In this repo, only apply `pytest.mark.vllm` to smoke tests under `tests/smoke/` that actually run real vLLM GPU generation and therefore require per-file process isolation (e.g., `test-smoke-gpu-*` Makefile targets). Do not apply `pytest.mark.vllm` to unit-style tests under `tests/generation/` that merely import `vllm_backend` but never instantiate a real vLLM engine and never call `.generate()` (GPU not required). Note that `tests/conftest.py` auto-marks these as `unit` via `pytest_collection_modifyitems`, and `vllm` is not among the auto-mark categories—so if a test in `tests/generation/` has `vllm`, it should be treated as a review issue unless it meets the real GPU generation criteria above.

Applied to files:

  • tests/generation/test_vllm_shutdown.py
🔇 Additional comments (1)
tests/generation/test_vllm_shutdown.py (1)

55-56: LGTM!


Walkthrough

The pytest configuration now applies CUDA-aware handling to requires_gpu tests. GPU CI requires PyTorch and CUDA during collection. Workflows, setup checks, documentation, and tests reflect this policy. A teardown test now isolates cleanup mock assertions.

Changes

GPU test collection and CI enforcement

Layer / File(s) Summary
GPU collection policy
tests/conftest.py
The collection hook detects requires_gpu tests, avoids PyTorch checks when none exist, skips tests without CUDA, and raises pytest.UsageError when NSS_REQUIRE_CUDA=1.
Collection policy validation
tests/test_pytest_configuration.py
Pytester tests cover skipped and successful GPU tests, required-CUDA errors, CUDA probe counts, and avoiding PyTorch imports when no GPU tests are collected.
GPU CI integration and guidance
.mise.gpu-ci.toml, .github/workflows/gpu-tests.yml, .github/actions/setup-gpu-test-env/action.yml, .github/actions/detect-changes/action.yml, tests/TESTING.md, tests/smoke/README.md
GPU workflows use the required-CUDA environment. Setup reports missing PyTorch or unavailable CUDA. Documentation describes automatic GPU test handling.
Teardown test isolation
tests/generation/test_vllm_shutdown.py
The teardown test resets cleanup mocks before it invokes backend.teardown().

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

Suggested labels: chore

Suggested reviewers: kendrickb-nvidia

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 46.15% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: skipping GPU tests when CUDA is unavailable.
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 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch binaryaaron/auto-skip-gpu-tests

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

@codecov

codecov Bot commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.84211% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
tests/test_pytest_configuration.py 97.22% 2 Missing ⚠️
tests/conftest.py 95.23% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@binaryaaron
binaryaaron marked this pull request as ready for review July 28, 2026 23:17
@binaryaaron
binaryaaron requested a review from a team as a code owner July 28, 2026 23:17
@coderabbitai coderabbitai Bot added feature New feature or request test Test-only addition or change labels Jul 28, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 9d9ce968-4ccb-4527-bdbc-1b64e71db3ed

📥 Commits

Reviewing files that changed from the base of the PR and between 14f56e4 and 5132ba8.

📒 Files selected for processing (4)
  • tests/TESTING.md
  • tests/conftest.py
  • tests/smoke/README.md
  • tests/test_pytest_configuration.py
📜 Review details
⏰ Context from checks skipped due to timeout. (1)
  • GitHub Check: Greptile Review
🧰 Additional context used
📓 Path-based instructions (14)
**/*.{md,markdown,py}

📄 CodeRabbit inference engine (.cursor/rules/agent-markdown-style.mdc)

**/*.{md,markdown,py}: Avoid decorative bold (**text**) in list items, body text, and docstrings; use structural cues (headers, list markers, colons, backticks) for emphasis instead
Use backticks for code identifiers, paths, and CLI commands in markdown and docstrings

Files:

  • tests/smoke/README.md
  • tests/TESTING.md
  • tests/test_pytest_configuration.py
  • tests/conftest.py
**/*.{md,markdown}

📄 CodeRabbit inference engine (.cursor/rules/agent-markdown-style.mdc)

**/*.{md,markdown}: Bold is acceptable only in markdown tables where it's the conventional way to mark header-like cells in the body
Use ## headers to segment markdown sections instead of bold text
Use -- (em-dash) instead of - (hyphen) for asides in markdown

Files:

  • tests/smoke/README.md
  • tests/TESTING.md
tests/**

📄 CodeRabbit inference engine (.cursor/rules/repo-navigation.mdc)

tests/**: Mirror src/ directory structure in tests/ directory for test organization
Auto-mark tests by directory: tests/e2e/e2e, tests/smoke/smoke, otherwise default to unit

Files:

  • tests/smoke/README.md
  • tests/TESTING.md
  • tests/test_pytest_configuration.py
  • tests/conftest.py
**/*.md

📄 CodeRabbit inference engine (STYLE_GUIDE.md)

Do not use decorative bold in Markdown body text, list items, or docstrings; use single backticks for code identifiers, paths, and commands.

Files:

  • tests/smoke/README.md
  • tests/TESTING.md
**/*

📄 CodeRabbit inference engine (STYLE_GUIDE.md)

**/*: Every source file requires the SPDX copyright and license header appropriate to its file format.
End files with a newline, remove trailing whitespace, use one space between sentences, and keep code, comments, and docstrings within 120 characters.

**/*: All contributions must use verified Git commits and DCO sign-off; unsigned or unsigned-off commits cannot be merged.
Branches other than main must follow <author>/<description>, optionally including an issue ID or type; branch names must use lowercase alphanumeric characters and hyphens.
Commits merged to main must follow Conventional Commits, using a valid lowercase type and a description of at most 100 characters.

Files:

  • tests/smoke/README.md
  • tests/TESTING.md
  • tests/test_pytest_configuration.py
  • tests/conftest.py

⚙️ CodeRabbit configuration file

**/*: Review as a senior maintainer for NeMo Safe Synthesizer. Prioritize issues that can change behavior, break user workflows, weaken privacy guarantees, hide failures, make tests unreliable, or create maintenance risk. Avoid generic style commentary unless it points to a concrete project convention that automated tools will not catch.
Comment only when the finding is actionable and tied to changed code. For each finding, state the impact, the condition that triggers it, and the smallest practical fix. Prefer one precise comment over broad advice. Do not ask for refactors outside the PR scope unless the changed code creates the problem.
Review type guidance: - Potential issue: use for correctness bugs, data loss, privacy leaks,
security risks, broken public APIs, invalid config behavior, missing
validation, hidden failures, nondeterministic tests, or CI breakage.

  • Refactor suggestion: use for local maintainability problems introduced
    by the diff when they have clear future cost, such as duplicated setup,
    unclear boundaries, over-mocking, avoidable complexity, or opaque test
    helpers.
  • Nitpick: avoid in chill mode. Do not emit formatting, import-order,
    wording, or style-only comments unless automated tools cannot catch the
    issue and it affects maintainability.

Severity guidance: - Critical: security/privacy leaks, data loss, training/test/holdout
contamination, or broken release/package/core pipeline execution.

  • Major: incorrect generation/training/evaluation behavior, broken
    CLI/SDK public API, invalid config defaults or validators, or GPU/vLLM
    cleanup and process-isolation bugs likely to fail CI or production
    runs.
  • Minor: localized bugs, missing focused tests for changed behavior, or
    bad test patterns that weaken regression coverage.
  • Trivial: small cleanup with no behavior impact. Usually suppress in
    chill mode.
  • Info: context only. Avoid unless it helps reviewers understand risk.
    Safe-Synthesizer-specific review focus: - Data ...

Files:

  • tests/smoke/README.md
  • tests/TESTING.md
  • tests/test_pytest_configuration.py
  • tests/conftest.py
**/*.{py,sh,yaml,yml,md}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

All Python, shell, YAML, YML, and Markdown source files require SPDX copyright headers.

Files:

  • tests/smoke/README.md
  • tests/TESTING.md
  • tests/test_pytest_configuration.py
  • tests/conftest.py
**/*.{py,md}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Use the repository's documented Python and Markdown style conventions and validate changes with the pinned mise formatting and checking tasks.

Files:

  • tests/smoke/README.md
  • tests/TESTING.md
  • tests/test_pytest_configuration.py
  • tests/conftest.py
**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

**/*.py: Place durable implementation guidance in function and class docstrings for public contracts and source comments for local invariants
Target Python 3.11–3.13 with modern syntax (X | Y, list[str], Self). Python 3.14+ is not supported

**/*.py: Use American English spelling in Python code, documentation, and messages.
Use Field(description=...) for every Pydantic model field.
Use assignment-style Field() by default; use Annotated only for additional metadata such as validators, constrained aliases, or discriminated unions.
Use @dataclass(frozen=True) for immutable value objects and validators; use mutable dataclasses only for builders, accumulators, and pipeline state.
Use field(default_factory=list) instead of mutable list defaults.
Use StrEnum for string-valued configuration or serialization enums and plain Enum for internal constants.
Obtain loggers with observability.get_logger(__name__); do not call logging.getLogger() or structlog.get_logger() directly.
Use .runtime, .user, and .system category loggers appropriately.
Do not use print() for operational library output; use the approved logger, click.echo(), or sys.stdout.write() where appropriate.
Use extra={} for machine-queryable logging data and f-strings only for human-readable context.
Raise errors from the custom Safe Synthesizer error hierarchy, using the documented dual inheritance for user and internal errors.
Keep shared package code compatible with Python 3.11; do not use Python 3.12-only syntax such as PEP 695 type statements or bracketed generic parameters.
Prefer X | Y, built-in collection generics, and Self over Optional, Union, and legacy typing collections.
Use collection ABCs for function arguments and concrete collection types for return values.
Use Protocol for structural subtyping and avoid Any when object, generics, or protocols are suitable.
Use TYPE_CHECKING guards for heavy imports such as pandas, torch, and transformers.
...

Files:

  • tests/test_pytest_configuration.py
  • tests/conftest.py
**/test_*.py

📄 CodeRabbit inference engine (AGENTS.md)

Use the unit marker instead of the deprecated unit_test marker for test identification

Files:

  • tests/test_pytest_configuration.py
tests/**/*.py

📄 CodeRabbit inference engine (STYLE_GUIDE.md)

tests/**/*.py: Use absolute imports from the package in tests.
Prefix fixtures with fixture_, use function scope by default, and document each fixture with a one-line docstring.
Use bare assert, pytest.raises(..., match=...), and pytest.approx() as the primary assertion styles.
Use tmp_path for file operations and never write test artifacts into the repository tree.
Use pytest.mark.parametrize for multiple input combinations and mock only external boundaries.

New features must include tests, and bug fixes must include regression tests; all existing tests must pass before submitting a pull request.

tests/**/*.py: Every test should have exactly one category marker: unit, smoke, or e2e; use modifier markers such as slow, requires_gpu, and vllm only as appropriate.
Use pytest.importorskip for optional dependencies that require specific extras, such as sentence_transformers and vllm.
Use the shared fixture_mock_processor or fixture_mock_processor_without_valid_records fixtures and construct ParsedResponse with valid_records, invalid_records, errors, and integer prompt_number.
Use mock_workdir(tmp_path) from cli/conftest.py when mocking a Workdir in CLI tests.
Use the fixture_ prefix consistently for dataset and tokenizer fixture names.
Seed Faker with fake.seed_instance(seed) and the Python random generator with random.seed(seed) for reproducible tests.
Tests should mirror the source structure, using directories such as tests/training/, tests/generation/, and tests/evaluation/.
print() is permitted in tests for debug output; the test configuration suppresses Ruff rule T201 for the tests directory.

Files:

  • tests/test_pytest_configuration.py
  • tests/conftest.py

⚙️ CodeRabbit configuration file

Review tests against tests/TESTING.md. Check marker usage, fixture naming, tmp_path usage, determinism, and GPU/vLLM process-isolation requirements. Flag slop tests that only check that code runs, assert result is not None when stronger invariants exist, over-mock internal implementation details, patch around the bug instead of reproducing it, or add broad snapshot/golden churn without a clear contract. Flag change detector tests that fail on harmless refactors, formatting, record ordering, incidental wording, or private implementation details without demonstrating a behavior regression. Prefer existing fixtures or focused new fixtures for repeated setup; keep tests DRY when reasonable without making the behavior under test opaque. print() is allowed in tests.

Files:

  • tests/test_pytest_configuration.py
  • tests/conftest.py
tests/test_*.py

📄 CodeRabbit inference engine (STYLE_GUIDE.md)

Name test files test_*.py, classes Test*, and functions test_<module>_<expected_behavior>.

Files:

  • tests/test_pytest_configuration.py
**/*.{py,pyi}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Shared Python package code must remain compatible with Python 3.11 syntax; do not use Python 3.12-only syntax such as PEP 695 type statements or bracketed generic class/function parameters.

Files:

  • tests/test_pytest_configuration.py
  • tests/conftest.py
**/*.{py,sh}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Use the repository's pinned mise tasks for formatting, linting, type checking, and testing rather than invoking ruff or ty directly for project-wide checks.

Files:

  • tests/test_pytest_configuration.py
  • tests/conftest.py
tests/conftest.py

📄 CodeRabbit inference engine (.cursor/rules/repo-navigation.mdc)

Store shared test fixtures in tests/conftest.py

Files:

  • tests/conftest.py
🧠 Learnings (2)
📚 Learning: 2026-05-27T22:20:37.354Z
Learnt from: kendrickb-nvidia
Repo: NVIDIA-NeMo/Safe-Synthesizer PR: 520
File: tests/generation/test_vllm_backend.py:556-587
Timestamp: 2026-05-27T22:20:37.354Z
Learning: In NVIDIA-NeMo/Safe-Synthesizer, `tests/conftest.py`’s `pytest_collection_modifyitems` hook applies pytest category markers automatically based on each test file’s path: tests under `/e2e/` get `pytest.mark.e2e`, tests under `/smoke/` get `pytest.mark.smoke`, and all other tests get `pytest.mark.unit`. Therefore, when reviewing pytest tests outside `tests/e2e/` and `tests/smoke/`, do not flag missing explicit `pytest.mark.unit` decorators on test classes/functions as an issue (the hook will add them during collection). If a new test directory/category is introduced, ensure the hook is updated so it’s categorized correctly.

Applied to files:

  • tests/test_pytest_configuration.py
  • tests/conftest.py
📚 Learning: 2026-07-27T22:07:22.590Z
Learnt from: binaryaaron
Repo: NVIDIA-NeMo/Safe-Synthesizer PR: 673
File: tests/pii_replacer/test_edit.py:327-327
Timestamp: 2026-07-27T22:07:22.590Z
Learning: When tests read structured logging context from Python `logging.LogRecord` instances, don’t access `record.ctx` directly (it isn’t declared on `LogRecord` and will break static typing). Instead, use `getattr(record, "ctx", default)` (or an appropriate fallback) to safely handle cases where `ctx` may or may not be attached. This applies even if Ruff rule `B009` isn’t enabled in the repo.

Applied to files:

  • tests/test_pytest_configuration.py
  • tests/conftest.py
🔇 Additional comments (4)
tests/conftest.py (1)

19-20: LGTM!

Also applies to: 59-95

tests/TESTING.md (1)

101-101: LGTM!

Also applies to: 119-121

tests/smoke/README.md (1)

63-64: LGTM!

tests/test_pytest_configuration.py (1)

1-35: LGTM!

Comment thread tests/test_pytest_configuration.py Outdated
Comment thread tests/conftest.py Outdated
@greptile-apps

greptile-apps Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR centralizes CUDA availability handling for GPU-marked tests while preserving strict failure behavior in GPU CI.

  • Automatically skips collected requires_gpu tests when CUDA is unavailable locally.
  • Adds a dedicated mise environment that requires CUDA in GPU CI lanes.
  • Updates GPU workflow invocations, change detection, documentation, and regression coverage.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
tests/conftest.py Adds collection-time CUDA detection and local skip behavior, with strict errors in GPU CI.
tests/test_pytest_configuration.py Covers CUDA availability, strict GPU CI behavior, and avoidance of unnecessary PyTorch imports.
.github/workflows/gpu-tests.yml Runs GPU test tasks using the dedicated gpu-ci mise environment.
.mise.gpu-ci.toml Enables strict CUDA requirements for GPU CI test execution.
.github/actions/setup-gpu-test-env/action.yml Improves GPU environment validation and failure diagnostics.
.github/actions/detect-changes/action.yml Ensures profile-specific mise configuration changes trigger relevant CI jobs.

Reviews (5): Last reviewed commit: "test: isolate vLLM teardown cleanup asse..." | Re-trigger Greptile

zywind
zywind previously approved these changes Jul 30, 2026
mckornfield
mckornfield previously approved these changes Jul 30, 2026

@mckornfield mckornfield left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

testing pytest config, rad

import pytest


def _root_conftest_module(config: pytest.Config) -> ModuleType:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

lol who tests the tests

@kendrickb-nvidia kendrickb-nvidia left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This makes sense for development. But is there a way to not skip the tests in CI though (the gpu CI jobs), in other words only skip the tests for the development but never do this auto disable in CI? I guess controlling it via an env var we set in CI?

I worry about something in CI messing up the cuda stuff and then we just skip all the tests, everything looks green, but we've suddenly stopped running any gpu based tests and don't find out about it until a release has a bunch of broken stuff or such.

@binaryaaron

Copy link
Copy Markdown
Collaborator Author

This makes sense for development. But is there a way to not skip the tests in CI though (the gpu CI jobs), in other words only skip the tests for the development but never do this auto disable in CI? I guess controlling it via an env var we set in CI?

I worry about something in CI messing up the cuda stuff and then we just skip all the tests, everything looks green, but we've suddenly stopped running any gpu based tests and don't find out about it until a release has a bunch of broken stuff or such.

like that's ever happened before ... :) - @zywind - since you're working on related stuff too, just want to coordinate a bit. we can add the env handling here, which i think is prudent. we'll also have to modify this if we add support for Metal.

@binaryaaron
binaryaaron dismissed stale reviews from mckornfield and zywind via 9799cc3 August 5, 2026 18:25
@binaryaaron
binaryaaron requested a review from a team as a code owner August 5, 2026 18:25
Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
@coderabbitai coderabbitai Bot added chore Maintenance not tied to a user-visible change and removed feature New feature or request test Test-only addition or change labels Aug 5, 2026
Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2


ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 81598b64-4292-4eaa-a399-c86d12014e3f

📥 Commits

Reviewing files that changed from the base of the PR and between 059d12c and 9799cc3.

📒 Files selected for processing (7)
  • .github/actions/detect-changes/action.yml
  • .github/actions/setup-gpu-test-env/action.yml
  • .github/workflows/gpu-tests.yml
  • .mise.gpu-ci.toml
  • tests/TESTING.md
  • tests/conftest.py
  • tests/test_pytest_configuration.py
📜 Review details
⏰ Context from checks skipped due to timeout. (4)
  • GitHub Check: Greptile Review
  • GitHub Check: Analyze (python)
  • GitHub Check: conventional-commit / semantic-pull-request
  • GitHub Check: Analyze (python)
🧰 Additional context used
📓 Path-based instructions (16)
.github/**

⚙️ CodeRabbit configuration file

Review GitHub configuration for branch protection expectations, CODEOWNERS alignment, least privilege permissions, pinned actions where practical, and consistency with CONTRIBUTING.md.

Files:

  • .github/actions/detect-changes/action.yml
  • .github/workflows/gpu-tests.yml
  • .github/actions/setup-gpu-test-env/action.yml
.github/workflows/*.yml

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Keep documentation deployment behavior consistent with the rule that changes to docs/, mkdocs.yml, or src/ deploy documentation when pushed to main.

Files:

  • .github/workflows/gpu-tests.yml
**/*.{md,markdown,py}

📄 CodeRabbit inference engine (.cursor/rules/agent-markdown-style.mdc)

**/*.{md,markdown,py}: Avoid decorative bold (**text**) in list items, body text, and docstrings; use structural cues (headers, list markers, colons, backticks) for emphasis instead
Use backticks for code identifiers, paths, and CLI commands in markdown and docstrings

Files:

  • tests/conftest.py
  • tests/test_pytest_configuration.py
  • tests/TESTING.md
tests/**

📄 CodeRabbit inference engine (.cursor/rules/repo-navigation.mdc)

tests/**: Mirror src/ directory structure in tests/ directory for test organization
Auto-mark tests by directory: tests/e2e/e2e, tests/smoke/smoke, otherwise default to unit

Files:

  • tests/conftest.py
  • tests/test_pytest_configuration.py
  • tests/TESTING.md
tests/conftest.py

📄 CodeRabbit inference engine (.cursor/rules/repo-navigation.mdc)

Store shared test fixtures in tests/conftest.py

Files:

  • tests/conftest.py
**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

**/*.py: Use uv for Python dependency management and execution; never use pip or raw python. Target Python 3.11–3.14 and use modern syntax such as X | Y, list[str], and Self.
Run project tooling through mise tasks or wrapper scripts in tools/; do not invoke ruff or ty directly. Use uv run for Python execution.
Put durable implementation guidance in public function and class docstrings; put local invariants in source comments.
Use Python 3.11–3.14-compatible modern type syntax, including X | Y, list[str], and Self.

**/*.py: Use American English spelling in Python code, comments, and documentation; use from __future__ import annotations in every module.
Use BaseSettings for environment/CLI settings; prefer AliasChoices for fields accepting both Python and environment-variable names.
Pydantic model fields must include Field(description=...); prefer assignment-style Field() and use Annotated only for additional metadata or constraints.
Use @dataclass(frozen=True) for immutable value objects and validators, and field(default_factory=...) for mutable defaults; never use mutable default values directly.
Use StrEnum for string-valued configuration or serialization enums and plain Enum for internal-only constants.
Obtain loggers with observability.get_logger(__name__); do not call logging.getLogger() or structlog.get_logger() directly.
Do not use print() for operational library output; use the approved logger, click.echo() for CLI output, or sys.stdout.write() for raw tool output.
Use logger extra={} for metrics, counts, durations, and other data intended for machine querying; use f-strings for human-readable context.
Raise known errors through the Safe Synthesizer custom hierarchy, using dual inheritance where callers should also catch a built-in exception.
Use X | Y, built-in generic types, Self for fluent returns, collection ABCs for arguments, Protocol for structural boundaries, and avoid Any.
Pre...

Files:

  • tests/conftest.py
  • tests/test_pytest_configuration.py
**/tests/**/*.py

📄 CodeRabbit inference engine (AGENTS.md)

Use the unit pytest marker instead of the deprecated unit_test marker; async tests do not need @pytest.mark.asyncio because asyncio_mode = auto.

Files:

  • tests/conftest.py
  • tests/test_pytest_configuration.py
**/*

📄 CodeRabbit inference engine (AGENTS.md)

**/*: Read AGENTS.local.md if it exists and give its instructions top priority.
When a task matches a repository-specific skill, read the corresponding skill under .agents/skills/ instead of duplicating its workflow instructions.
Do not commit unless the user explicitly asks for a commit or PR work.
When committing, require DCO sign-off and GPG signing using git commit --signoff --gpg-sign (or -s -S); never manually add Signed-off-by or use --no-gpg-sign.
Use feature branches based on main; branch names commonly include an issue-number prefix such as <author>/123-short-name.
For recurring testing, building, syncing, bootstrapping, worktree, and GitHub workflows, use the matching skill under .agents/skills/.
For a full GPU/development environment, use uv sync --frozen --extra cu129 --extra engine --group dev; bare uv sync --frozen is incomplete and can cause ty, import-check, and GPU-test failures.

End files with a newline, contain no trailing whitespace, use one space between sentences, and keep code, comments, and docstrings within 120 characters.

Files:

  • tests/conftest.py
  • tests/test_pytest_configuration.py
  • tests/TESTING.md

⚙️ CodeRabbit configuration file

**/*: Review as a senior maintainer for NeMo Safe Synthesizer. Prioritize issues that can change behavior, break user workflows, weaken privacy guarantees, hide failures, make tests unreliable, or create maintenance risk. Avoid generic style commentary unless it points to a concrete project convention that automated tools will not catch.
Comment only when the finding is actionable and tied to changed code. For each finding, state the impact, the condition that triggers it, and the smallest practical fix. Prefer one precise comment over broad advice. Do not ask for refactors outside the PR scope unless the changed code creates the problem.
Review type guidance: - Potential issue: use for correctness bugs, data loss, privacy leaks,
security risks, broken public APIs, invalid config behavior, missing
validation, hidden failures, nondeterministic tests, or CI breakage.

  • Refactor suggestion: use for local maintainability problems introduced
    by the diff when they have clear future cost, such as duplicated setup,
    unclear boundaries, over-mocking, avoidable complexity, or opaque test
    helpers.
  • Nitpick: avoid in chill mode. Do not emit formatting, import-order,
    wording, or style-only comments unless automated tools cannot catch the
    issue and it affects maintainability.

Severity guidance: - Critical: security/privacy leaks, data loss, training/test/holdout
contamination, or broken release/package/core pipeline execution.

  • Major: incorrect generation/training/evaluation behavior, broken
    CLI/SDK public API, invalid config defaults or validators, or GPU/vLLM
    cleanup and process-isolation bugs likely to fail CI or production
    runs.
  • Minor: localized bugs, missing focused tests for changed behavior, or
    bad test patterns that weaken regression coverage.
  • Trivial: small cleanup with no behavior impact. Usually suppress in
    chill mode.
  • Info: context only. Avoid unless it helps reviewers understand risk.
    Safe-Synthesizer-specific review focus: - Data ...

Files:

  • tests/conftest.py
  • tests/test_pytest_configuration.py
  • tests/TESTING.md
**/*.{md,py,sh,toml,yml,yaml,Dockerfile}

📄 CodeRabbit inference engine (AGENTS.md)

Follow the detailed language and file-format conventions defined in STYLE_GUIDE.md.

Files:

  • tests/conftest.py
  • tests/test_pytest_configuration.py
  • tests/TESTING.md
tests/**/*.py

📄 CodeRabbit inference engine (STYLE_GUIDE.md)

tests/**/*.py: Use absolute imports in tests.
Name test files test_*.py, classes Test*, and functions test_<module>_<expected_behavior>; prefix fixtures with fixture_ and give each fixture a one-line purpose docstring.
Use function-scoped fixtures by default, bare assert, pytest.raises(match=...), pytest.approx(), and pytest.mark.parametrize for input combinations.
Use tmp_path for file operations, mock only external boundaries, avoid shared mutable state and order dependencies, and mark CUDA-dependent tests appropriately.

All existing tests must pass before submitting a pull request; new features must include tests and bug fixes must include regression tests.

tests/**/*.py: Every test must have exactly one category marker: unit, smoke, or e2e; use modifier markers such as slow, requires_gpu, and vllm only in addition to a category marker.
Use pytest.importorskip for optional dependencies that require specific extras, such as sentence_transformers and vllm.
Use the shared test-data helpers load_test_dataset(filename) and load_test_dataframe(filename) from the root test configuration when loading test datasets.
Use the fixture_ prefix for dataset and tokenizer fixtures; CLI helpers such as mock_workdir may use descriptive names.
Represent ParsedResponse test values with valid_records, invalid_records, errors, and integer prompt_number; prefer fixture_mock_processor or fixture_mock_processor_without_valid_records for processor mocks.
Seed both Faker and the standard random module for reproducible generated test data: call fake.seed_instance(seed) and random.seed(seed).
Tokenizers should generally use function-scoped fixtures; use session scope for fixture_session_cache_dir where specified.
Tests should mirror the source structure, using directories such as tests/training/, tests/generation/, and corresponding source areas.
print() is allowed in tests for debug output; Ruff rule T201 is suppr...

Files:

  • tests/conftest.py
  • tests/test_pytest_configuration.py

⚙️ CodeRabbit configuration file

Review tests against tests/TESTING.md. Check marker usage, fixture naming, tmp_path usage, determinism, and GPU/vLLM process-isolation requirements. Flag slop tests that only check that code runs, assert result is not None when stronger invariants exist, over-mock internal implementation details, patch around the bug instead of reproducing it, or add broad snapshot/golden churn without a clear contract. Flag change detector tests that fail on harmless refactors, formatting, record ordering, incidental wording, or private implementation details without demonstrating a behavior regression. Prefer existing fixtures or focused new fixtures for repeated setup; keep tests DRY when reasonable without making the behavior under test opaque. print() is allowed in tests.

Files:

  • tests/conftest.py
  • tests/test_pytest_configuration.py
**/*.{py,sh,yaml,yml,md}

📄 CodeRabbit inference engine (STYLE_GUIDE.md)

Include the required SPDX copyright and Apache-2.0 license headers, using comment syntax appropriate to the file format.

Include SPDX copyright headers in all source files, except files explicitly listed in .copyrightignore.

Files:

  • tests/conftest.py
  • tests/test_pytest_configuration.py
  • tests/TESTING.md
**/*.{py,pyi}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

**/*.{py,pyi}: Keep shared Python package code compatible with Python 3.11; do not use Python 3.12-only syntax such as PEP 695 type statements or bracketed generic parameters.
Use the repository's pinned Ruff tasks for Python formatting, import sorting, and linting rather than invoking unpinned tools directly.
Run the repository's pinned ty type checker and maintain type-correct Python code.

Files:

  • tests/conftest.py
  • tests/test_pytest_configuration.py
**/*.{py,sh,yaml,yml,toml,md}

📄 CodeRabbit inference engine (CONTRIBUTING.md)

Use mise tasks with the repository's pinned tool versions for formatting, checking, and testing before submitting changes.

Files:

  • tests/conftest.py
  • tests/test_pytest_configuration.py
  • tests/TESTING.md
**/*.{md,markdown}

📄 CodeRabbit inference engine (.cursor/rules/agent-markdown-style.mdc)

**/*.{md,markdown}: Bold is acceptable only in markdown tables where it's the conventional way to mark header-like cells in the body
Use ## headers to segment markdown sections instead of bold text
Use -- (em-dash) instead of - (hyphen) for asides in markdown

Files:

  • tests/TESTING.md
tests/TESTING.md

📄 CodeRabbit inference engine (AGENTS.md)

Keep test-suite guidance in tests/TESTING.md.

Files:

  • tests/TESTING.md
**/*.md

📄 CodeRabbit inference engine (STYLE_GUIDE.md)

Do not use decorative bold text in body content; use single backticks for inline code and -- for asides. In Python docstrings, use double backticks and MkDocs autorefs rather than Sphinx roles.

Files:

  • tests/TESTING.md
🧠 Learnings (3)
📚 Learning: 2026-05-27T22:20:37.354Z
Learnt from: kendrickb-nvidia
Repo: NVIDIA-NeMo/Safe-Synthesizer PR: 520
File: tests/generation/test_vllm_backend.py:556-587
Timestamp: 2026-05-27T22:20:37.354Z
Learning: In NVIDIA-NeMo/Safe-Synthesizer, `tests/conftest.py`’s `pytest_collection_modifyitems` hook applies pytest category markers automatically based on each test file’s path: tests under `/e2e/` get `pytest.mark.e2e`, tests under `/smoke/` get `pytest.mark.smoke`, and all other tests get `pytest.mark.unit`. Therefore, when reviewing pytest tests outside `tests/e2e/` and `tests/smoke/`, do not flag missing explicit `pytest.mark.unit` decorators on test classes/functions as an issue (the hook will add them during collection). If a new test directory/category is introduced, ensure the hook is updated so it’s categorized correctly.

Applied to files:

  • tests/conftest.py
  • tests/test_pytest_configuration.py
📚 Learning: 2026-07-27T22:07:22.590Z
Learnt from: binaryaaron
Repo: NVIDIA-NeMo/Safe-Synthesizer PR: 673
File: tests/pii_replacer/test_edit.py:327-327
Timestamp: 2026-07-27T22:07:22.590Z
Learning: When tests read structured logging context from Python `logging.LogRecord` instances, don’t access `record.ctx` directly (it isn’t declared on `LogRecord` and will break static typing). Instead, use `getattr(record, "ctx", default)` (or an appropriate fallback) to safely handle cases where `ctx` may or may not be attached. This applies even if Ruff rule `B009` isn’t enabled in the repo.

Applied to files:

  • tests/conftest.py
  • tests/test_pytest_configuration.py
📚 Learning: 2026-07-29T17:12:32.642Z
Learnt from: zywind
Repo: NVIDIA-NeMo/Safe-Synthesizer PR: 654
File: tests/config/test_parameters.py:116-118
Timestamp: 2026-07-29T17:12:32.642Z
Learning: In Pydantic validation tests (e.g., models configured with `from_attributes`), when asserting failures from `model_validate(...)`, assert the structured error details (such as `ValidationError.errors()[0]["type"]`, e.g. `"model_attributes_type"`) rather than relying on the human-readable error message text. This keeps tests stable even if wording changes, while still verifying the correct validation rule is triggered.

Applied to files:

  • tests/conftest.py
  • tests/test_pytest_configuration.py
🔇 Additional comments (4)
.mise.gpu-ci.toml (1)

4-5: LGTM!

.github/workflows/gpu-tests.yml (1)

108-128: LGTM!

Also applies to: 161-161

tests/TESTING.md (1)

101-101: LGTM!

Also applies to: 119-132

.github/actions/detect-changes/action.yml (1)

87-87: 🗄️ Data Integrity & Integration

No change needed. src_test_deps includes the ci filter, which matches .mise.gpu-ci.toml, and both GPU jobs use this output.

			> Likely an incorrect or invalid review comment.

Comment thread .github/actions/setup-gpu-test-env/action.yml
Comment thread tests/conftest.py
@binaryaaron
binaryaaron force-pushed the binaryaaron/auto-skip-gpu-tests branch from 9799cc3 to 06fab77 Compare August 5, 2026 18:37
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@binaryaaron
binaryaaron requested a review from mckornfield August 5, 2026 20:12
Signed-off-by: Aaron Gonzales <aagonzales@nvidia.com>
@binaryaaron
binaryaaron added this pull request to the merge queue Aug 7, 2026
Merged via the queue into main with commit 5e10d76 Aug 7, 2026
24 checks passed
@binaryaaron
binaryaaron deleted the binaryaaron/auto-skip-gpu-tests branch August 7, 2026 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:ci area:tests chore Maintenance not tied to a user-visible change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants