test: add deterministic Slurm fakes

[andreatnvidia]

📋 Summary

Adds the F4 foundation for Gate B with deterministic test doubles for Slurm commands, dependency installation, plugin discovery, and multi-backend serving. Stage 2 lanes can test observable infrastructure boundaries against the canonical F3 contracts without Slurm, GPUs, containers, model servers, or new production abstractions.

🔗 Related Issue

Closes #872

Part of #850

🔄 Changes

✨ Added

• Add a scriptable fake sbatch, squeue, sacct, scancel, and bounded sinfo runner with arrays, strict output formats, active and terminal states, cancellation, accounting lag, malformed output, command failures, call logs, and explicit state control.
• Add raw scheduler goldens for normal and retry terminal spellings, plus canonical single-node and multi-node rendered scripts bound to resolved-plan and runtime digests.
• Add deterministic dependency resolver and installer doubles covering compatible, incompatible, missing, digest-mismatch, cancellation, and unexpected-call outcomes.
• Add an installed-layout fake Data Designer plugin overlay with entry-point discovery.
• Add single- and multi-backend fake serving infrastructure covering startup, readiness, overload, rank/backend failure, endpoint publication, terminal restart rejection, and idempotent cleanup.
• Add explicitly controlled clocks, identifiers, ports, command responses, failure injection, and reusable plan, manifest, image, benchmark, collection, accounting-lag, and readiness fixtures.

🔧 Changed

• Promote the canonical feat: define Slurm config and plan contracts #879 authored-plan/client-result and feat: integrate Slurm plan and runtime state #889 finalization fixtures to package-wide fixtures for downstream lanes.
• Extend readiness golden coverage with pending, starting, publication-failed, failed, and stopped transition branches.

🔍 Attention Areas

⚠️ Reviewers: Please pay special attention to the following:

• slurm.py - scheduler boundary behavior, terminal precedence, bounded queries, and failure scripting.
• serving.py - logical endpoint lifecycle, overload routing, backend failure, and cleanup behavior.
• conftest.py - package-wide canonical F3 fixture ownership for Stage 2 consumers.

🧪 Testing

• make test passes (not run; the scoped Slurm targets cover this test-only package change)
• Unit tests added/updated
• E2E tests added/updated (not applicable; no live Slurm or serving processes are used)
• .venv/bin/ruff check --fix .
• .venv/bin/ruff format .
• make check-slurm
• make test-slurm (399 passed)
• Rendered scripts pass bash -n
• git diff --check
• make test-slurm-wheel-install (not applicable; packaging and installed behavior are unchanged)

✅ Checklist

• Follows commit message conventions
• Commits are signed off (DCO)
• Architecture docs updated (not applicable; test infrastructure follows the approved F4 plan)

---

Description updated with AI

[greptile-apps]

Greptile Summary

Adds deterministic test infrastructure for Slurm commands, dependency installation, plugin discovery, serving lifecycles, and readiness-state fixtures.

• Introduces scriptable Slurm scheduler and multi-backend serving fakes.
• Centralizes shared contract fixtures in the package-level test configuration.
• Adds deterministic clocks, dependency doubles, plugin-overlay fixtures, golden scheduler output, and readiness transitions.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
packages/data-designer-slurm/tests/slurm_test_fakes/slurm.py	Adds a stateful fake runner for deterministic sbatch, squeue, sacct, scancel, and bounded sinfo behavior.
packages/data-designer-slurm/tests/slurm_test_fakes/serving.py	Adds explicit backend and logical-endpoint lifecycle, publication, routing, overload, failure, and cleanup behavior.
packages/data-designer-slurm/tests/conftest.py	Centralizes shared contract records and deterministic infrastructure fixtures at the Slurm test-suite root.
packages/data-designer-slurm/tests/slurm_test_fakes/dependencies.py	Adds scripted resolver and installer doubles with exact call and outcome verification.
packages/data-designer-slurm/tests/state/test_transition_fixtures.py	Extends canonical readiness-transition fixture coverage across pending, starting, failure, and stopped states.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart LR
    Tests[Slurm integration tests] --> SlurmFake[Fake Slurm runner]
    Tests --> DependencyFakes[Dependency resolver and installer fakes]
    Tests --> PluginOverlay[Installed-layout plugin overlay]
    Tests --> ServingFake[Logical endpoint and backend fakes]
    SlurmFake --> SchedulerStates[Scripted scheduler states and outputs]
    ServingFake --> ReadinessStates[Startup, readiness, failure, and cleanup states]

Loading

_{Reviews (2): Last reviewed commit: "test: complete post-F3 Slurm fake covera..." | Re-trigger Greptile}

[github-actions]

Fern preview: https://nvidia-preview-pr-888.docs.buildwithfern.com/nemo/datadesigner

Fern previews include the docs-website version archive with PR changes synced into latest. Notebook tutorials are rendered without execution outputs in previews.

[github-actions]

Code Review: PR #888 — test: add deterministic Slurm fakes

Author: andreatnvidia (Andre Manoel) · Base: main · State: OPEN
Reported size: 89 files, +6143/-136 · Labels: none

Summary

This PR adds the F4 foundation for Gate B (closes #872, part of #850): a set of
deterministic test doubles for the data-designer-slurm package that let Stage 2 test
lanes exercise infrastructure boundaries — Slurm scheduling, dependency install, plugin
discovery, and multi-backend model serving — without a real Slurm cluster, GPUs,
containers, or model servers. It also extends the #878 readiness golden-record coverage
with new transition states.

Scope note (important for reading the diff): The GitHub PR diff shows 89 files /
+6143 lines because the branch is stacked on top of already-merged commits (#856,
#854, #863, #878). Verified via git log origin/main..HEAD: only the single
9bc21533 test: add deterministic Slurm fakes commit is genuinely new to main. Its
real footprint is test-only — new files under
packages/data-designer-slurm/tests/ (fakes, their unit tests, golden fixtures, plugin
overlay, and additions to two state test files). No production/src code is modified by
the net diff. This substantially lowers the effective blast radius relative to the
pre-computed structural analysis (see Structural Impact section).

The net-new content is well-structured, idiomatic, and — unusually for test infrastructure
— the fakes are themselves thoroughly unit-tested (each double has a dedicated
test_*.py). The author reports make test-slurm = 129 passed and make check-slurm
clean.

Findings

Correctness — no blocking issues found

• FakeSlurmRunner scheduler modeling is sound. sbatch pops from a pending-array
  deque and records the submission; squeue/sacct project independent queue vs.
  accounting states (correctly modeling accounting lag — a task can be gone from squeue
  but not yet terminal in sacct); scancel sets terminal CANCELLED/0:15. The
  -j/--jobs/--jobs= selector parsing (_selected_submitted_tasks) computes the value
  index as argv[index + 2], which is correct given enumerate(argv[1:]) (index 0 →
  argv[1], value at argv[2]). Malformed selectors raise AssertionError, exercised by
  a parametrized test.
• Terminal-precedence and unscripted-command rejection are explicitly tested
  (test_fake_slurm_runner_rejects_unscripted_commands_and_submissions,
  _run_sinfo raising on unexpected queries). Good "fail loud on the unexpected" hygiene
  for a test double — silent defaults are what make fakes drift from reality.
• FakeLogicalEndpoint overload routing (429 → try next backend, return the last 429
  if all overloaded) matches its three dedicated tests including the "return the final
  overload response" case that preserves the last backend's Retry-After. Round-robin
  pointer (_next_backend) advances only on generation requests, not /health.
• Cleanup idempotency is correctly guarded at the endpoint level (if self.state is STOPPED: return), so backends are cleaned exactly once even across repeated
  endpoint.cleanup() calls — asserted as [1, 1].
• FakeClock rejects naive/non-UTC/negative inputs in __post_init__ and advances
  both wall and monotonic clocks together; the UTC guard using
  utcoffset() != timedelta(0) correctly rejects non-UTC tz-aware datetimes.
• Golden files match the values the tests assert against
  (squeue_active.txt, sacct_terminal.txt, sinfo_gres.txt, squeue_malformed.txt),
  and the plugin-overlay entry_points.txt group/name (data_designer.plugins /
  fake-slurm-column) aligns with test_dependencies.py's discovery assertions.

Minor / non-blocking

• FakeVllmBackend.cleanup() docstring vs. behavior. The docstring says "Stop the
  backend once while allowing cleanup re-entry," but the method unconditionally
  increments cleanup_calls and re-sets STOPPED on every direct call. The "once"
  guarantee only holds because FakeLogicalEndpoint.cleanup() short-circuits when already
  stopped. If a future test calls backend.cleanup() directly twice it will see
  cleanup_calls == 2, contradicting the docstring's implication. Consider either
  guarding on state inside FakeVllmBackend.cleanup() or rewording the docstring to "each
  call records a cleanup; the owning endpoint invokes it once." (serving.py)
• FakeSlurmTask.exit_code default "0:0" is retained after a state is cleared to
  None. In set_task_state(..., queue_state=None, accounting_state=None) the
  exit_code keeps its prior/default value. This is harmless today because sacct only
  emits rows where accounting_state is not None, but it's a latent inconsistency if a
  future query ever reads exit_code independently. Not worth changing now.
• scancel selection precision. _run_scancel filters out any arg starting with -
  and requires exactly one remaining target. Real scancel accepts multiple job IDs and
  various filters (-u, --state); the fake deliberately narrows this and returns a
  clear error otherwise. Reasonable for a controlled double — just noting the intentional
  divergence from real Slurm semantics in case a lane needs multi-target cancel later.
• test-only package boundary. All new modules live under tests/ and import
  slurm_test_fakes as a top-level package (not via the data_designer namespace), so
  they don't participate in the runtime namespace-merge and don't affect import direction
  or fast-import budgets. This is the right place for them.

Conventions

• SPDX headers present on every new .py file (verified). from __future__ import annotations present throughout. Modern typing (str | None, list[...],
  Generic[...]), absolute imports, and frozen/@dataclass usage all match project style.
• The generic FakeDependencyResolver/FakeDependencyInstaller (Generic[RequestT, ...])
  are contract-agnostic on purpose — the PR body notes feat: define Slurm config and plan contracts #879/Integrate Slurm plan and runtime-state contracts #880 canonical contracts are
  deliberately deferred "without temporary records or compatibility shims," which the
  generic parameterization supports cleanly.

Test coverage

• Excellent. Every double has a focused unit test covering the happy path and the
  failure/edge paths it advertises (overload, backend/rank failure, publication failure,
  cancellation signals KeyboardInterrupt/SystemExit, digest mismatch, malformed
  output, command failure, unexpected-call rejection). Readiness transition fixtures are
  validated against validate_readiness_transition with revision monotonicity (1→5).
• Could not execute make test-slurm in this review environment (pytest/venv not on the
  reviewer PATH). Relying on the author's reported "129 passed" and clean check-slurm.

Structural Impact — reviewer interpretation

(Appended below from the pre-computed analysis. Note the caveat: the analysis was run
over the full stacked branch, not the net diff. Because this PR's net commit is test-only
and touches no src/ code, the reported HIGH risk — 113 "import direction violations" and
the DataDesignerConfigBuilder god-node modification — originates from the already-merged
underlying commits (#854/#863/#878), not from this PR's changes. The analysis tool itself
flags that data-designer-slurm is an unknown package to its _PACKAGE_SUBDIRS map, so
its verdict for this package is explicitly incomplete. Treat the HIGH label as an artifact
of branch stacking rather than a property of the fakes being added here.)

---

Structural Impact (graphify, 2.3s)

Risk: HIGH (1 core abstraction(s) modified; 113 import direction violation(s))

• 48 Python files, 302 AST entities, 8/78 clusters

• Note: changes touch unknown package(s) (data-designer-slurm); analysis may be incomplete - update _PACKAGE_SUBDIRS in structural_impact.py.

Import Direction Violations (113)

Legal direction: interface -> engine -> config

• __getattr__() (config) --calls--> .getattr() (engine)
• .create_report_section() (config) --calls--> .keys() (interface)
• inject_sampler_type_into_params() (config) --calls--> .items() (interface)
• _resolve_sampler_kwargs() (config) --calls--> .items() (interface)
• allowed_references() (config) --calls--> .keys() (interface)
• +108 more

Core Abstractions Modified

• DataDesignerConfigBuilder - Add support for images in preview_result.display_sample_record() #5 most connected (154 deps)

High-Connectivity Changes

• DataDesignerConfigBuilder (154 deps) in packages/data-designer-config/src/data_designer/config/config_builder.py
• Return list of available exports for tab-completion. (91 deps) in packages/data-designer-config/src/data_designer/config/__init__.py
• Lazily import config module exports when accessed. This allows fast imports (91 deps) in packages/data-designer-config/src/data_designer/config/__init__.py
• .items() (87 deps) in packages/data-designer/src/data_designer/interface/composite_workflow.py
• DataDesigner (85 deps) in packages/data-designer/src/data_designer/interface/data_designer.py
• BuilderConfig (69 deps) in packages/data-designer-config/src/data_designer/config/config_builder.py
• DatasetCreationResults (66 deps) in packages/data-designer/src/data_designer/interface/results.py
• CompositeWorkflow (43 deps) in packages/data-designer/src/data_designer/interface/composite_workflow.py
• +175 more

Cross-Package Dependencies

• GenerationController (interface) --uses--> DataDesignerConfigBuilder (config)
• Controller for dataset generation workflows (previ (interface) --uses--> DataDesignerConfigBuilder (config)
• Load config, generate a preview dataset, and displ (interface) --uses--> DataDesignerConfigBuilder (config)
• Load config and validate it against the engine. (interface) --uses--> DataDesignerConfigBuilder (config)
• Load config and probe every referenced model and M (interface) --uses--> DataDesignerConfigBuilder (config)
• Load config, create a full dataset, and save resul (interface) --uses--> DataDesignerConfigBuilder (config)
• +1309 more

Verdict

Approve (non-blocking nits). This is clean, well-tested, correctly-scoped test
infrastructure. The net change is test-only, fails loud on unexpected input (the right
default for fakes), and each double is independently verified. The only actionable item is
the cosmetic FakeVllmBackend.cleanup() docstring/behavior mismatch; the exit_code
staleness and scancel narrowing are informational. No correctness, security, or
convention blockers.

Note: per CI-agent constraints, this review does not approve or request changes on GitHub
and is not posted by the reviewer; the workflow handles posting.

[nabinchha]

Thanks for putting this together, @andreatnvidia!

Summary

This adds deterministic, test-only doubles for Slurm commands, dependency installation, plugin discovery, controlled time, and single-/multi-backend serving, along with canonical readiness-transition fixtures. The implementation matches the stated F4 foundation and stays cleanly isolated from production code.

Findings

No code correctness findings. I also checked the existing PR feedback and did not repeat the already-raised FakeVllmBackend.cleanup() docstring nit.

The branch does need a refresh before merge: #879 has since merged into feat/slurm-execution and renamed packages/data-designer-slurm/tests/state/test_golden_records.py to test_state_golden_records.py, while this PR modifies the old path. GitHub currently reports the PR as conflicting. Please rebase onto the latest base, carry the five new readiness-golden entries into the renamed file, and rerun CI.

What Looks Good

• The scheduler fake separates queue and accounting observations, supports bounded command failure injection, and preserves deterministic ordering, which gives downstream reconciliation lanes useful observable boundaries without embedding production orchestration.
• The serving fake covers partial startup, endpoint publication failure, overload retry, coordinated backend failure, and cleanup re-entry with focused behavioral tests.
• The dependency and plugin fixtures are strict about expected calls and exercise real entry-point discovery, while the readiness goldens round-trip through the shared state contracts and validate monotonic transitions.

Verdict

Needs changes — the implementation itself looks ready, but the branch must be rebased and the rename conflict resolved before merge. The changed tests passed locally (51 passed), Ruff check and format validation passed, and the previous GitHub CI run was green; CI should run again after the rebase.

---

This review was generated by an AI assistant.

[nabinchha]

Thanks for the thorough follow-up, @andreatnvidia!

Summary

This adds the shared deterministic Slurm test harness, promotes the merged F3 contract fixtures for package-wide reuse, and fills out scheduler, serving, dependency, plugin, script, and readiness-transition coverage. The updated implementation matches PR #888 and issue #872’s stated scope; the post-F3 additions are the contract-bound fixtures that earlier commits could not include before #879 and #889 landed.

Findings

No actionable findings on the updated head.

What Looks Good

• The fake scheduler keeps state and failure injection explicit while covering arrays, accounting lag, cancellation, retry terminal spellings, malformed output, and bounded command shapes.
• The package-wide fixtures reuse the reviewed planning and finalization contracts instead of creating lane-specific substitutes, which is exactly what downstream #866–#869 need.
• The serving doubles exercise lifecycle boundaries, multi-backend routing, overload preservation, publication failure, terminal restart rejection, and idempotent endpoint cleanup without external processes.

Verdict

Ship it — ready to merge as-is. I re-ran the complete Slurm package suite in an isolated installed-package environment (399 passed), Ruff check/format passed, and both rendered scripts passed bash -n.

---

This review was generated by an AI assistant.