Skip to content

test: stand up the config tiers two unit tests read from - #33294

Merged
ch-wan merged 1 commit into
mainfrom
cheng/gc-fix-unpublished-config-tests
Aug 2, 2026
Merged

ch-wan merged 1 commit into
mainfrom
cheng/gc-fix-unpublished-config-tests

Conversation

@ch-wan

@ch-wan ch-wan commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator

What

Two registered tests fail on main today. Both read process state that only a
served engine establishes, and nothing in the test published it:

  • test/registered/chunked_prefill/test_mm_chunked_embedding_unit.py — 3 of 5
    tests fail with AssertionError: attention tensor model parallel group is not initialized. The embedding path reads the attention-TP rank and the config;
    the test set up neither.
  • test/registered/unit/hardware_backend/mlx/test_attn_dp_request_capacity.py
    fails on the MLX runner with ValueError: config namespace 'exec' not published. It handed MlxModelRunnerStub a SimpleNamespace carrying the four
    fields initialize() used to read off the instance; those reads moved to the
    namespace accessors, so the stand-in stopped intercepting.

Failure logs: mm,
mlx.

Fix

Both tests publish a real config instead of faking one, which is what the
runtime-context testing contract asks for — a faked accessor or a stubbed
server_args silently stops intercepting the moment a reader migrates, which is
exactly what happened here. The mm test also overrides the parallel tier
(get_parallel().override(attn_tp_rank=0, ...)) for the duration of each test.
The MLX stub still receives the published instance, so its own instance reads are
unchanged.

While here, one production line: _acknowledge_deferred_cuda_ipc_cache_hits
computed getattr(server_args, "tp_size", parallel.attn_tp_size). Python
evaluates that default eagerly, so every call read the live attention-TP size
even though tp_size is always present on a resolved ServerArgs — a config read
that required a process group for no reason, and the reason the mm test tripped
the assert before reaching anything else. It reads tp_size directly now.

Validation

The two test files: 5 passed + 4 skipped (MLX skips without mlx installed; the
four namespace reads its stub makes were verified to resolve under the new
publish). test/registered/unit/{multimodal,managers} plus the config ratchets:
358 passed. The MLX path itself needs the MLX runner in CI.

Split out of a larger ServerArgs.override burndown stack (#33242) so the
main-side fix can land on its own.


CI States

Latest PR Test (Base): 🚫 Run #30770816029
Latest PR Test (Extra): ❌ Run #30770815906

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

Both of these fail on main, not because of anything in this stack — they read
process state that only a served engine establishes, and nothing published it.

`test_mm_chunked_embedding_unit` drives the embedding path, which reads the
attention-TP rank and the config. It now publishes a config and overrides the
parallel tier for the duration of each test, which is what the runtime-context
testing contract asks for.

`test_attn_dp_request_capacity` handed the MLX runner stub a `SimpleNamespace`
carrying the four fields `initialize()` used to read off the instance. Those
reads went through the namespace accessors some time ago, so the stand-in stopped
intercepting and the accessors raised "config namespace 'exec' not published".
It publishes a real config with the same values instead; the stub still gets the
published instance for its own reads.

While here: `_acknowledge_deferred_cuda_ipc_cache_hits` computed
`getattr(server_args, "tp_size", parallel.attn_tp_size)`. Python evaluates that
default eagerly, so every call read the live attention-TP size even though
`tp_size` is always present on a resolved ServerArgs — a config read that needed
a process group for no reason. It reads `tp_size` directly now.
@ch-wan

ch-wan commented Aug 2, 2026

Copy link
Copy Markdown
Collaborator Author

Renamed the fixture to publish_config_and_parallel_state and said in its docstring that it is autouse — applied to every test in the module and named by none of them, which makes it look unused on a grep.

It is load-bearing: flipping autouse=True to False puts the file straight back to 3 failed / 2 passed, and restoring it returns 5 passed.

@ch-wan
ch-wan force-pushed the cheng/gc-fix-unpublished-config-tests branch from 0fd339f to f0f000d Compare August 2, 2026 22:45
@ch-wan
ch-wan merged commit 88e5a0f into main Aug 2, 2026
52 of 90 checks passed
@ch-wan
ch-wan deleted the cheng/gc-fix-unpublished-config-tests branch August 2, 2026 22:46
kfhfar pushed a commit to kfhfar/sglang that referenced this pull request Aug 7, 2026
saturn-acc pushed a commit to saturn-acc/sglang that referenced this pull request Aug 16, 2026
jakki-amd pushed a commit to jakki-amd/sglang that referenced this pull request Sep 9, 2026
Atituiset pushed a commit to Atituiset/sglang that referenced this pull request Sep 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant