[CI] Stabilize multinode DP internal LB completion tests

[AndreasKaratzas]

Fixes flaky test_api_only_multinode_dp_completion and test_multinode_dp_completion which intermittently fail with empty model responses during concurrent load balancer testing.

Motivation

> assert len(choice.text) >= 1
E AssertionError: assert 0 >= 1
E  +  where 0 = len('')

These tests intentionally use temperature=1.0 to produce diverse outputs across 200 concurrent requests for realistic load balancer distribution testing. However, at temperature=1.0 the model can legitimately emit a stop token as its very first token, producing text='' with finish_reason='stop'. Over 400 requests (two bursts of 200), the probability of at least one empty response is high. Rather than changing temperature=0.0 (which would undermine the test's intent of exercising load balancing with diverse requests), the fix tolerates the valid edge case: when finish_reason='stop', empty text is accepted. The non-empty text assertion is only enforced when finish_reason='length'.

• _make_completion_request helper: Extracted the duplicated make_request() closure from both non-streaming completion tests into a shared module-level function with diagnostic assertion messages that print actual values on failure.

• _run_request_bursts helper: Extracted the duplicated two-burst loop pattern (create tasks -> gather -> validate -> sleep) shared by both non-streaming tests.

• Streaming tests unchanged: They already use temperature=0.0 and have adequate assertions.

cc @kenroche

[gemini-code-assist]

Code Review

This pull request effectively addresses a flaky test issue by correctly handling empty model responses when finish_reason is 'stop'. The refactoring into _make_completion_request and _run_request_bursts helper functions significantly improves code readability and maintainability by removing duplication. The changes are well-justified and implemented correctly. I have one suggestion to further improve the robustness of the new test helper.

[gemini-code-assist]

Using asyncio.gather without return_exceptions=True can lead to unhandled exceptions and resource leaks if one of the tasks fails. When a task in gather raises an exception, gather propagates that exception immediately, and other tasks might not be cancelled, potentially continuing to run in the background. This can affect the stability of subsequent tests in the suite.

By setting return_exceptions=True, gather will wait for all tasks to complete and return exceptions as results. You can then explicitly check for and handle any exceptions, ensuring a cleaner test shutdown. This improves test robustness.

        results = await asyncio.gather(*all_tasks, return_exceptions=True)
        assert len(results) == num_requests, (
            f"Burst {burst}: expected {num_requests} results, got {len(results)}"
        )

        # Raise any exceptions that were caught
        for result in results:
            if isinstance(result, BaseException):
                raise result

        assert all(completion is not None for completion in results), (
            f"Burst {burst}: some completions were None"
        )

[AndreasKaratzas]

Using return_exceptions=True and re-raising to ensure clean task shutdown. Done :)

[njhill]

Thanks @AndreasKaratzas