tests/dragon_ci: standalone pre-release contract suite for Dragon by andre-merzky · Pull Request #64 · radical-cybertools/rhapsody

andre-merzky · 2026-05-24T21:51:18Z

NOTE: This PR is against main, not dev.

Pin every Dragon API surface our V3 backend and telemetry adapter depend on, so the Dragon team can detect contract changes before they ship. The suite has no Rhapsody dependency — only dragon, pytest, pytest-asyncio, pytest-timeout, cloudpickle, and the stdlib.

Coverage:

Batch (ctor, num_workers/num_managers, function/process/job, fence, close/join/terminate, results_ddict 5-tuple shape on success/failure)
ProcessTemplate (cwd/env/policy/args/kwargs/argdata)
Policy (Distribution, Placement, gpu_affinity)
System (nnodes, hostname_policies)
ProcessGroup, native Queue/Event, DragonUserCodeError
DDict core ops
dragon.telemetry.collector + AccVendor
async-coroutine wrapping via asyncio.run shim
worker failure-mode detection boundary

Three known Dragon bugs are pinned as deliberately-failing @pytest.mark.timeout tests, one per file (the leaked test thread prevents combining them):

test_ddict_unknown_key_blocks: DDict[unknown_key] blocks instead of raising KeyError when wait_for_keys=True
test_sigkill_in_worker_hangs: abnormal worker termination is undetected; task.get() blocks forever; timeout= kwarg is ignored on the blocking DDict read
test_unpickleable_function_hangs: same silent-hang failure mode for functions from modules unreachable via the worker's PYTHONPATH

Pin every Dragon API surface our V3 backend and telemetry adapter depend on, so the Dragon team can detect contract changes before they ship. The suite has no Rhapsody dependency — only dragon, pytest, pytest-asyncio, pytest-timeout, cloudpickle, and the stdlib. Coverage: - Batch (ctor, num_workers/num_managers, function/process/job, fence, close/join/terminate, results_ddict 5-tuple shape on success/failure) - ProcessTemplate (cwd/env/policy/args/kwargs/argdata) - Policy (Distribution, Placement, gpu_affinity) - System (nnodes, hostname_policies) - ProcessGroup, native Queue/Event, DragonUserCodeError - DDict core ops - dragon.telemetry.collector + AccVendor - async-coroutine wrapping via asyncio.run shim - worker failure-mode detection boundary Three known Dragon bugs are pinned as deliberately-failing @pytest.mark.timeout tests, one per file (the leaked test thread prevents combining them): - test_ddict_unknown_key_blocks: DDict[unknown_key] blocks instead of raising KeyError when wait_for_keys=True - test_sigkill_in_worker_hangs: abnormal worker termination is undetected; task.get() blocks forever; timeout= kwarg is ignored on the blocking DDict read - test_unpickleable_function_hangs: same silent-hang failure mode for functions from modules unreachable via the worker's PYTHONPATH Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

gemini-code-assist

Code Review

This pull request adds a standalone CI test suite for the Dragon runtime to ensure API stability for downstream projects. It includes tests for Batch, DDict, and ProcessGroup components, along with pins for known bugs. Reviewer feedback suggests improving the PYTHONPATH construction in conftest.py to prevent trailing separators and using shlex.quote in test_batch_process.py for safer shell command generation.

gemini-code-assist · 2026-05-24T21:53:05Z

+
+from __future__ import annotations
+
+import pytest


Import the shlex module to support safe shell-quoting of file paths in generated scripts.

Suggested change

import pytest

import shlex

import pytest

Copilot

Pull request overview

Adds a standalone tests/dragon_ci contract test suite intended to pin Dragon runtime APIs and behaviors that Rhapsody’s V3 backend / telemetry adapter depend on, so upstream Dragon changes are detected pre-release.

Changes:

Introduces a dedicated tests/dragon_ci pytest suite (with its own pytest.ini, conftest.py, and README) designed to run under the Dragon launcher.
Adds “happy path” contract tests covering Batch / DDict / ProcessGroup / ProcessTemplate / Policy / System / Queue+Event / telemetry collector surfaces used by Rhapsody.
Adds three “known Dragon bug” timeout-based tests to pin currently-hanging failure modes.

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
tests/dragon_ci/README.md	Documents purpose, coverage, known-bug pins, and how to run the standalone suite.
tests/dragon_ci/pytest.ini	Suite-local pytest configuration and marker definitions.
tests/dragon_ci/conftest.py	Suite bootstrap: worker-importability via `PYTHONPATH`, environment detection, skip markers, Batch fixtures.
tests/dragon_ci/_dragon_ci_helpers.py	Worker-importable helper callables used as Batch / ProcessGroup targets.
tests/dragon_ci/_offpath/marker_helper.py	Deliberately off-PYTHONPATH helper to reproduce cloudpickle-by-reference hang.
tests/dragon_ci/test_batch_lifecycle.py	Contract tests for Batch constructor kwargs, surface methods, and basic teardown behavior.
tests/dragon_ci/test_batch_function.py	Contract tests for `Batch.function()` handle semantics and results DDict 5-tuple shape.
tests/dragon_ci/test_batch_process.py	Contract tests for `Batch.process()` tuple shape, stdio behavior, and failure visibility.
tests/dragon_ci/test_batch_job.py	Contract tests for `Batch.job()` signature and opt-in PMIx launch.
tests/dragon_ci/test_process_template.py	Contract tests for ProcessTemplate ctor/attrs and cloudpickle `argdata` round-trip.
tests/dragon_ci/test_policy.py	Contract tests for Policy kwargs/attrs and Distribution/Placement enums.
tests/dragon_ci/test_machine_system.py	Contract tests for `System().nnodes` and `hostname_policies()` behavior.
tests/dragon_ci/test_process_group.py	Contract tests for native ProcessGroup lifecycle and `DragonUserCodeError`.
tests/dragon_ci/test_queue_event.py	Contract tests for native Queue/Event methods and cross-process smoke checks.
tests/dragon_ci/test_ddict.py	Contract tests for DDict constructor surface and core operations.
tests/dragon_ci/test_telemetry_collector.py	Contract tests for telemetry collector symbol availability and basic return shapes.
tests/dragon_ci/test_async_in_batch.py	Pins Rhapsody’s coroutine wrapping pattern via `asyncio.run` shim and results DDict.
tests/dragon_ci/test_worker_failure_modes.py	Pins detected worker-failure paths: raised exception and clean `sys.exit`.
tests/dragon_ci/test_ddict_unknown_key_blocks.py	Known-bug pin: DDict blocks on unknown key with `wait_for_keys=True`.
tests/dragon_ci/test_sigkill_in_worker_hangs.py	Known-bug pin: SIGKILL’d worker leaves `task.get()` wedged.
tests/dragon_ci/test_unpickleable_function_hangs.py	Known-bug pin: unpickleable-by-reference function hangs `task.get()`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+    for p in System().hostname_policies():
+        assert p.host_name or p.host_id >= 0 or p.placement == Policy.Placement.HOST_NAME, (
+            f"hostname_policies returned a Policy with no node identifier: {p}"
+        )


+from dragon.infrastructure.gpu_desc import AccVendor, find_accelerators
+from dragon.telemetry.collector import (
+    get_amd_metrics,
+    get_intel_metrics,
+    get_nvidia_metrics,
+    identify_gpu,
+)
+
+# All tests here need dragon.telemetry.collector — auto-skip if unavailable.
+pytestmark = pytest.mark.requires_telemetry_collector


Pure formatting (ruff-format + docformatter) across the contract suite, plus one ruff B011 fix: fn_assert_false now raises AssertionError explicitly instead of `assert False`, so the failure probe still fires under `python -O` (the test already expects that exact AssertionError). No behavioural change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

andre-merzky · 2026-06-30T10:37:27Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces a comprehensive standalone contract test suite for the Dragon runtime to prevent breaking API changes affecting downstream stacks like Rhapsody. The suite covers various subsystems, including batch lifecycles, process templates, and telemetry collectors. The reviewer provided valuable feedback to improve the robustness and security of the tests, suggesting the use of try...finally blocks to guarantee resource cleanup in process group and batch lifecycle tests, avoiding trailing colons in PYTHONPATH to mitigate security risks, narrowing exception handling in MPI launch tests to prevent silent skips of unrelated errors, and using time.monotonic() instead of time.time() for reliable duration measurements.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

gemini-code-assist · 2026-06-30T10:39:00Z

+    msg = q.get(timeout=30.0)
+    assert msg.get("pid") and msg.get("host")
+
+    shutdown.set()
+    grp.join(timeout=30.0)
+    grp.close()


If q.get() times out or the assertion fails, the shutdown event is never set and grp.close() is never called. This will cause the background worker process (pg_worker) to loop indefinitely in the background, leaking CPU and Dragon resources. Wrap the test logic in a try...finally block to guarantee proper cleanup, matching the robust pattern used in test_queue_event.py.

Suggested change

msg = q.get(timeout=30.0)

assert msg.get("pid") and msg.get("host")

shutdown.set()

grp.join(timeout=30.0)

grp.close()

try:

msg = q.get(timeout=30.0)

assert msg.get("pid") and msg.get("host")

finally:

shutdown.set()

grp.join(timeout=30.0)

grp.close()

gemini-code-assist · 2026-06-30T10:39:00Z

+_HERE = os.path.dirname(os.path.abspath(__file__))
+if _HERE not in sys.path:
+    sys.path.insert(0, _HERE)
+os.environ["PYTHONPATH"] = _HERE + os.pathsep + os.environ.get("PYTHONPATH", "")


Appending a trailing path separator when PYTHONPATH is empty or unset results in a trailing colon (e.g., /path/to/dir:). In Python and Unix shells, a trailing colon in search paths is implicitly treated as including the current working directory (.), which can lead to unexpected import behaviors or security risks if untrusted files are present in the working directory. Only append the separator if PYTHONPATH is already populated.

Suggested change

_HERE = os.path.dirname(os.path.abspath(__file__))

if _HERE not in sys.path:

sys.path.insert(0, _HERE)

os.environ["PYTHONPATH"] = _HERE + os.pathsep + os.environ.get("PYTHONPATH", "")

_HERE = os.path.dirname(os.path.abspath(__file__))

if _HERE not in sys.path:

sys.path.insert(0, _HERE)

existing_pythonpath = os.environ.get("PYTHONPATH")

os.environ["PYTHONPATH"] = f"{_HERE}{os.pathsep}{existing_pythonpath}" if existing_pythonpath else _HERE

gemini-code-assist · 2026-06-30T10:39:01Z

+    b = Batch(disable_telem=True)
+    b.close()
+    b.join(timeout=30.0)
+    b.terminate()


If b.join() raises an exception or times out, b.terminate() will never be called, potentially leaking background processes and Dragon resources. Wrapping the teardown in a try...finally block ensures that terminate() is always executed to clean up resources.

Suggested change

b = Batch(disable_telem=True)

b.close()

b.join(timeout=30.0)

b.terminate()

b = Batch(disable_telem=True)

try:

b.close()

b.join(timeout=30.0)

finally:

b.terminate()

gemini-code-assist · 2026-06-30T10:39:01Z

+    except Exception as exc:  # noqa: BLE001
+        pytest.skip(f"PMIX launch unavailable on this host: {exc!r}")


Catching all Exceptions and skipping the test is too broad. If there is a regression or bug in batch.job (such as a TypeError, AttributeError, or NameError), it will be caught and the test will be silently skipped instead of failing. Restrict the exception handling to RuntimeError and verify that the error message is indeed related to PMIx initialization before skipping.

Suggested change

except Exception as exc: # noqa: BLE001

pytest.skip(f"PMIX launch unavailable on this host: {exc!r}")

except RuntimeError as exc:

if "PMIx" in str(exc) or "PMIX" in str(exc) or "pmi" in str(exc).lower():

pytest.skip(f"PMIX launch unavailable on this host: {exc!r}")

raise

gemini-code-assist · 2026-06-30T10:39:01Z

+    t0 = time.time()
+    rv = e.wait(timeout=0.2)
+    assert not rv, f"Event.wait should be falsy on timeout, got {rv!r}"
+    assert time.time() - t0 >= 0.15, "Event.wait returned before its timeout"


Using time.time() to measure elapsed time intervals can be unreliable if the system clock is adjusted (e.g., via NTP sync) during test execution. Use time.monotonic() instead, which is guaranteed to be non-decreasing and is the standard way to measure durations.

Suggested change

t0 = time.time()

rv = e.wait(timeout=0.2)

assert not rv, f"Event.wait should be falsy on timeout, got {rv!r}"

assert time.time() - t0 >= 0.15, "Event.wait returned before its timeout"

t0 = time.monotonic()

rv = e.wait(timeout=0.2)

assert not rv, f"Event.wait should be falsy on timeout, got {rv!r}"

assert time.monotonic() - t0 >= 0.15, "Event.wait returned before its timeout"

- test_process_group / test_batch_lifecycle: wrap the body in try/finally so a timed-out get() or failed assertion can't leak the worker ProcessGroup / Batch. - conftest: don't append a trailing PYTHONPATH separator when it's empty (an empty entry is implicitly the cwd). - test_batch_job: narrow the skip to RuntimeError + an actual PMIx message, and re-raise everything else so real regressions don't get silently skipped. - test_queue_event: measure the wait with time.monotonic() instead of time.time(). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

andre-merzky requested a review from Copilot May 24, 2026 21:51

andre-merzky self-assigned this May 24, 2026

Copilot started reviewing on behalf of andre-merzky May 24, 2026 21:51 View session

gemini-code-assist Bot reviewed May 24, 2026

View reviewed changes

Copilot AI reviewed May 24, 2026

View reviewed changes

andre-merzky and others added 4 commits May 25, 2026 08:56

linting

3edae6d

finalize dragon API tests

935ee53

Merge remote-tracking branch 'origin/main' into feature/dragon_ci

bc5565f

gemini-code-assist Bot reviewed Jun 30, 2026

View reviewed changes

Navigation Menu

Uh oh!

tests/dragon_ci: standalone pre-release contract suite for Dragon - #64

tests/dragon_ci: standalone pre-release contract suite for Dragon#64
andre-merzky wants to merge 6 commits into
mainfrom
feature/dragon_ci

andre-merzky commented May 24, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

Uh oh!

gemini-code-assist Bot May 24, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

andre-merzky commented Jun 30, 2026

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Jun 30, 2026

Uh oh!

gemini-code-assist Bot Jun 30, 2026

Uh oh!

gemini-code-assist Bot Jun 30, 2026

Uh oh!

gemini-code-assist Bot Jun 30, 2026

Uh oh!

gemini-code-assist Bot Jun 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		except Exception as exc: # noqa: BLE001
		pytest.skip(f"PMIX launch unavailable on this host: {exc!r}")

Uh oh!

Conversation

andre-merzky commented May 24, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

gemini-code-assist Bot May 24, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

andre-merzky commented Jun 30, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist Bot Jun 30, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants