Skip to content

chore: add safe-synthesizer test for kubernetes - #654

Merged
mckornfield merged 3 commits into
mainfrom
nss-gpu-kube-test/mck
Jul 13, 2026
Merged

chore: add safe-synthesizer test for kubernetes#654
mckornfield merged 3 commits into
mainfrom
nss-gpu-kube-test/mck

Conversation

@mckornfield

@mckornfield mckornfield commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Summary by CodeRabbit

  • New Features

    • Added opt-in GPU-only end-to-end coverage for Safe Synthesizer container jobs.
    • Introduced repeatable --feature NAME support for enabling optional E2E test sets, with automatic skipping for GPU tests unless gpu is enabled.
    • Enhanced local upgrade script with GPU environment configuration and an optional Safe Synthesizer tasks build.
  • Documentation

    • Documented the new requires_gpu test marker and GPU/Safe Synthesizer build flags.
  • Chores

    • Increased uv HTTP retry behavior in the Safe Synthesizer tasks container image.

@mckornfield
mckornfield requested review from a team as code owners July 13, 2026 18:13
@github-actions github-actions Bot added the chore label Jul 13, 2026
Signed-off-by: Matt Kornfield <mkornfield@nvidia.com>
@mckornfield
mckornfield force-pushed the nss-gpu-kube-test/mck branch from b4018ec to fb35778 Compare July 13, 2026 18:16
@coderabbitai

coderabbitai Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

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: ad97ef84-89fe-4c44-87f8-fa6606aa715b

📥 Commits

Reviewing files that changed from the base of the PR and between fb35778 and e6743a7.

📒 Files selected for processing (2)
  • docker/Dockerfile.safe-synthesizer-tasks
  • e2e/test_safe_synthesizer.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • e2e/test_safe_synthesizer.py

📝 Walkthrough

Walkthrough

Adds opt-in GPU feature gating, GPU-aware local Minikube and image builds, Safe Synthesizer container retry settings, and a GPU-only E2E test with deterministic input and output validation.

Changes

Safe Synthesizer E2E

Layer / File(s) Summary
GPU feature gating
conftest.py, pytest.ini
Adds repeatable --feature selection, implicit GPU enablement for NSS Kubernetes E2E runs, and skipping for tests marked requires_gpu.
GPU build and deployment
e2e/k8s/scripts/local_build_and_upgrade.sh, docker/Dockerfile.safe-synthesizer-tasks
Selects CPU or GPU Minikube setup, optionally builds Safe Synthesizer task images, reports the additional build, and increases uv HTTP retries.
Safe Synthesizer container validation
e2e/test_safe_synthesizer.py
Registers model filesets, generates deterministic tabular input, submits a GPU container job, waits for completion, and validates timing fields and output row count.

Sequence Diagram(s)

sequenceDiagram
  participant Test as Safe Synthesizer E2E test
  participant Setup as Model fileset setup script
  participant Platform as NeMoPlatform
  participant Job as Safe Synthesizer container job
  Test->>Setup: register required model filesets
  Test->>Platform: submit Safe Synthesizer job
  Platform->>Job: execute GPU container job
  Test->>Platform: wait and fetch job summary
  Platform-->>Test: return timing fields
  Test->>Platform: fetch generated dataset
  Platform-->>Test: return synthetic records
Loading

Possibly related PRs

Suggested reviewers: crookedstorm, a2bondar, tylersbray

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title matches the main change: adding a Safe Synthesizer Kubernetes end-to-end test.
Docstring Coverage ✅ Passed Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%.
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 docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch nss-gpu-kube-test/mck

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

@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

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@e2e/test_safe_synthesizer.py`:
- Around line 62-75: Update the subprocess.run invocation in the module-scoped
fileset setup fixture to include a finite timeout, preventing stalled
setup_model_filesets.py or HF downloads from hanging the test run indefinitely.
Preserve the existing command, working directory, output capture, and check
behavior.
🪄 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: Enterprise

Run ID: 3a7cdc51-3472-4ef8-931f-8dc57089e641

📥 Commits

Reviewing files that changed from the base of the PR and between 275c8cd and fb35778.

📒 Files selected for processing (4)
  • conftest.py
  • e2e/k8s/scripts/local_build_and_upgrade.sh
  • e2e/test_safe_synthesizer.py
  • pytest.ini

Comment thread e2e/test_safe_synthesizer.py
@github-actions

github-actions Bot commented Jul 13, 2026

Copy link
Copy Markdown
Contributor
Suite Lines Covered Line Rate Branch Rate
Unit Tests 23845/31009 76.9% 61.7%
Integration Tests 13817/29658 46.6% 19.6%

Signed-off-by: Matt Kornfield <mkornfield@nvidia.com>
Signed-off-by: Matt Kornfield <mkornfield@nvidia.com>
@mckornfield
mckornfield requested a review from tylersbray July 13, 2026 23:05

@tylersbray tylersbray 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.

lgtm

@mckornfield
mckornfield added this pull request to the merge queue Jul 13, 2026
Merged via the queue into main with commit 3eedbdf Jul 13, 2026
57 checks passed
@mckornfield
mckornfield deleted the nss-gpu-kube-test/mck branch July 13, 2026 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants