Skip to content

[https://nvbugs/6720250][test] Waive test_openai_completion_list_prompt_stream_reuses_stream_metadata - #18706

Draft
luyiyun1021 wants to merge 1 commit into
NVIDIA:mainfrom
luyiyun1021:waive/llmapi-adjusted-steady-clock
Draft

[https://nvbugs/6720250][test] Waive test_openai_completion_list_prompt_stream_reuses_stream_metadata#18706
luyiyun1021 wants to merge 1 commit into
NVIDIA:mainfrom
luyiyun1021:waive/llmapi-adjusted-steady-clock

Conversation

@luyiyun1021

@luyiyun1021 luyiyun1021 commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Description

unittest/llmapi/test_llm.py::test_openai_completion_list_prompt_stream_reuses_stream_metadata has been failing on A100X-PyTorch-Post-Merge-1 since the clock refactor in #17491 landed, and is currently unwaived.

AttributeError: 'OpenAIServer' object has no attribute '_adjusted_steady_clock'
tensorrt_llm/serve/openai_server.py:2506: AttributeError

The test builds its server with object.__new__(OpenAIServer), bypassing __init__, and hand-sets only the attributes the code path needed when it was written. #17491 changed the openai_completion streaming path (generator_wrapper) from the module-level get_steady_clock_now_in_seconds() to self._adjusted_steady_clock.now(), so the hand-built stub is now missing an attribute it previously never had to set.

Waived under NVBug 6720250 because it shares its root cause with the VisualGen timing breakage tracked there: #17491 rewrote every clock call site present in its own tree, and the sites it did not see are what broke. Regression window confirmed against post-merge builds — L0_PostMerge #2946 (30316da103a0) green, #2947 (53bb31fdcf8b) red, and #17491 is in that range.

Follow-up required

This is a stopgap. The real fix is two lines and is proposed on #18686 (#18686 (comment)):

 from tensorrt_llm import LLM
+from tensorrt_llm._utils import AdjustedSteadyClock
 from tensorrt_llm.bindings import executor as tllm
@@ def test_openai_completion_list_prompt_stream_reuses_stream_metadata @@
         server._input_proc_executor = None
+        server._adjusted_steady_clock = AdjustedSteadyClock()

This waiver must be removed when that fix lands, otherwise closing NVBug 6720250 would leave an orphan waiver masking a test that is green again. This exact test has gone stale the same way twice before — #16841 (NVBug 6507081) added reasoning_parser = None, #17401 (NVBug 6555875) swapped perf_metrics for _collect_perf_metrics — and both times the fake was refreshed and the waiver deleted in the same PR.

Test Coverage

Waiver-only change; no product or test code is touched.

The failure and the proposed fix were both reproduced on a local build of main at 98c176f042 (B200, --cuda_architectures 100-real): the test fails with the AttributeError above, still fails with #18686 applied (same error, shifted to line 2509 by the SPDX header that PR adds), and passes once the two lines above are added.

PR Checklist

  • PR title complies with the PR title format
  • Commits are signed off (DCO)
  • Test list / waiver change only; no code paths modified

Dev Engineer Review

  • Added one temporary waiver in tests/integration/test_lists/waives.txt.
  • The waiver targets unittest/llmapi/test_llm.py::test_openai_completion_list_prompt_stream_reuses_stream_metadata.
  • The waiver references NVBug 6720250.
  • No product or test code changed.
  • The change has the intended scope and low regression risk.
  • Remove the waiver after the planned test fix in PR #18686 lands.

QA Engineer Review

  • Modified tests/integration/test_lists/waives.txt.
  • Added one waiver entry.
  • The test identifier and NVBug reference are valid and appropriately scoped.
  • No duplicate waiver entry was found.
  • The waiver format is consistent with the file.
  • CBTS touchmap coverage is not applicable because this is a test-list-only change.
  • Verdict: sufficient.

…pt_stream_reuses_stream_metadata

The test builds its server with object.__new__(OpenAIServer), bypassing
__init__, and hand-sets only the attributes the code path needed when it was
written. PR NVIDIA#17491 moved the openai_completion streaming path from the
module-level get_steady_clock_now_in_seconds() to
self._adjusted_steady_clock.now(), so the stub now raises AttributeError.

Waived under the same NVBug as the VisualGen timing breakage, which has the
same root cause. This is a stopgap; the fix is proposed on PR NVIDIA#18686.

Signed-off-by: Yiyun Lu <55233584+luyiyun1021@users.noreply.github.com>
@luyiyun1021
luyiyun1021 marked this pull request as draft September 4, 2026 05:30
@coderabbitai

coderabbitai Bot commented Sep 4, 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: ea468e49-f73c-49ca-ba01-dc533774b377

📥 Commits

Reviewing files that changed from the base of the PR and between b916389 and 814e747.

📒 Files selected for processing (1)
  • tests/integration/test_lists/waives.txt

Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review.


Walkthrough

The integration waiver list now skips the OpenAI completion list-prompt streaming metadata reuse test and references NVBUG 6720250.

Changes

Test waiver

Layer / File(s) Summary
Add OpenAI streaming waiver
tests/integration/test_lists/waives.txt
Adds a skip entry for test_openai_completion_list_prompt_stream_reuses_stream_metadata.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Merge Risk: ⚪ Minimal · up to 814e7

This change temporarily skips the failing OpenAI streaming metadata test and does not modify product or test implementation code. The waiver entry is valid and scoped to the intended test, so it is ready to merge.

Possibly related PRs

Suggested reviewers: brnguyen2

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title identifies NVBug 6720250, uses the valid test type, and clearly states that the specified test is waived.
Description check ✅ Passed The description explains the failure, root cause, temporary waiver, follow-up fix, test coverage, and checklist status. It is complete and directly related to the waiver-only change.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
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 unit tests (beta)
  • Create PR with unit tests

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant