[TRTLLM-13022][test] remove deprecated models from tests - #14660
Conversation
📝 WalkthroughWalkthroughThis PR systematically removes test coverage for deprecated models (Llama 2, Gemma 3 variants, EXAONE4, GLM4Flash) and LoRA features across the entire test suite. Changes include removing accuracy reference data, test classes and methods, example tests, unit test modules, end-to-end tests, fixtures, performance configurations, Triton tests, and updating all test selection lists in CI configurations. ChangesTest suite deprecation cleanup
🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/integration/test_lists/test-db/l0_h100.yml (1)
51-56:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winComment scope is inconsistent with the listed model tests.
Line 51 says H100 key models are
llama/mixtral/gpt-oss, but this block also includesmodeling_gemma3andtest_modeling_nemotron_h. Please update the comment (or the list) so scope reviewers aren’t misled.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integration/test_lists/test-db/l0_h100.yml` around lines 51 - 56, The comment above the H100 block currently states "Only key models in H100: llama/mixtral/gpt-oss" but the test list also includes modeling_gemma3 and test_modeling_nemotron_h; update either the comment or the list so they match: either remove modeling_gemma3 and test_modeling_nemotron_h from the block (leaving modeling_llama, modeling_mixtral, modeling_gpt_oss) or expand the comment to include gemma3 and nemotron_h (reference the entries modeling_gemma3 and unittest/_torch/modeling/test_modeling_nemotron_h::test_nemotron_h_sanity) to make the scope accurate.
🧹 Nitpick comments (2)
tests/integration/defs/perf/test_perf.py (1)
1069-1073: ⚡ Quick winGuard fallback tokenizer path with an explicit check.
If this fallback path is missing in a given environment, failure currently happens later in command execution. A direct check here gives a clearer error.
♻️ Proposed change
else: tokenizer_dir = os.path.join(llm_models_root(), "llama-models-v2", "TinyLlama-1.1B-Chat-v1.0") + if not os.path.exists(tokenizer_dir): + raise FileNotFoundError( + f"Fallback tokenizer path does not exist: {tokenizer_dir}" + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integration/defs/perf/test_perf.py` around lines 1069 - 1073, Add an explicit existence check for the computed tokenizer_dir (the fallback path built with llm_models_root() and "TinyLlama-1.1B-Chat-v1.0") immediately after it is assigned; if os.path.exists(tokenizer_dir) is False, raise a clear exception or call process exit with a descriptive error message indicating the missing tokenizer directory so failures fail fast and clearly rather than later during command execution.tests/integration/defs/conftest.py (1)
1234-1279: ⚡ Quick winFail fast on unsupported LoRA adapter names.
This fixture silently drops unknown adapter IDs and can return an empty/partial path list, which makes downstream failures harder to diagnose—especially after removing deprecated mappings.
♻️ Proposed change
def llm_lora_model_root(request): @@ - for item in model_list: + unsupported_items = [] + for item in model_list: if item == "Japanese-Alpaca-LoRA-7b-v0": @@ elif item == "gpt-oss-20b-lora-adapter_NIM_r8": model_root_list.append( os.path.join(models_root, "gpt_oss", "gpt-oss-20b-lora-adapter_NIM_r8")) + else: + unsupported_items.append(item) + + if unsupported_items: + raise ValueError( + f"Unsupported LoRA adapter(s): {unsupported_items}. " + "Please update tests/integration/defs/conftest.py mappings." + ) return ",".join(model_root_list)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integration/defs/conftest.py` around lines 1234 - 1279, The loop over model_list (building model_root_list and returning ",".join(model_root_list)) currently ignores unknown adapter IDs; change it to fail fast by detecting any items not matched by the existing if/elif branches and raising a clear exception (e.g., ValueError) listing the unsupported adapter names; implement this by collecting unmatched_items while iterating (or compute set(model_list) - set(supported_names)) and raising with the offending IDs so callers get an immediate, actionable error instead of a silent partial/empty result.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/integration/defs/test_e2e.py`:
- Line 31: Remove the duplicate import of convert_weights from the absolute
module and keep only the relative import style used elsewhere in this file:
consolidate imports so convert_weights is imported from .common (remove the
import from defs.common) to prevent symbol shadowing and ambiguous resolution;
update the import line(s) in tests/integration/defs/test_e2e.py to reference
.common for convert_weights and ensure any other imported names
(get_mmlu_accuracy, venv_check_call) remain correctly imported from their
intended module.
In `@tests/unittest/trt/model/test_llama.py`:
- Around line 439-453: get_loader_test_cases currently builds test_cases using
product([], ...), which yields an empty list and causes
parameterized.expand(get_loader_test_cases, ...) to raise ValueError; fix it by
ensuring get_loader_test_cases returns at least one valid loader pair (e.g., add
a non-deprecated HF/meta pair to the iterable instead of the empty list) or add
a guard so the decorator never receives an empty iterable (for example, change
the first argument to product([( "hf", "meta" )], ...) or wrap the parameter
provider so it returns a default single-case list when empty); update references
to get_loader_test_cases and the test_loaders parameterization accordingly.
---
Outside diff comments:
In `@tests/integration/test_lists/test-db/l0_h100.yml`:
- Around line 51-56: The comment above the H100 block currently states "Only key
models in H100: llama/mixtral/gpt-oss" but the test list also includes
modeling_gemma3 and test_modeling_nemotron_h; update either the comment or the
list so they match: either remove modeling_gemma3 and test_modeling_nemotron_h
from the block (leaving modeling_llama, modeling_mixtral, modeling_gpt_oss) or
expand the comment to include gemma3 and nemotron_h (reference the entries
modeling_gemma3 and
unittest/_torch/modeling/test_modeling_nemotron_h::test_nemotron_h_sanity) to
make the scope accurate.
---
Nitpick comments:
In `@tests/integration/defs/conftest.py`:
- Around line 1234-1279: The loop over model_list (building model_root_list and
returning ",".join(model_root_list)) currently ignores unknown adapter IDs;
change it to fail fast by detecting any items not matched by the existing
if/elif branches and raising a clear exception (e.g., ValueError) listing the
unsupported adapter names; implement this by collecting unmatched_items while
iterating (or compute set(model_list) - set(supported_names)) and raising with
the offending IDs so callers get an immediate, actionable error instead of a
silent partial/empty result.
In `@tests/integration/defs/perf/test_perf.py`:
- Around line 1069-1073: Add an explicit existence check for the computed
tokenizer_dir (the fallback path built with llm_models_root() and
"TinyLlama-1.1B-Chat-v1.0") immediately after it is assigned; if
os.path.exists(tokenizer_dir) is False, raise a clear exception or call process
exit with a descriptive error message indicating the missing tokenizer directory
so failures fail fast and clearly rather than later during command execution.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 7074a434-7487-4541-89fd-671b790023d0
📒 Files selected for processing (67)
tests/integration/defs/.test_durationstests/integration/defs/accuracy/references/cnn_dailymail.yamltests/integration/defs/accuracy/references/gsm8k.yamltests/integration/defs/accuracy/references/json_mode_eval.yamltests/integration/defs/accuracy/references/mmlu.yamltests/integration/defs/accuracy/references/mmmu.yamltests/integration/defs/accuracy/test_cli_flow.pytests/integration/defs/accuracy/test_disaggregated_serving.pytests/integration/defs/accuracy/test_llm_api_autodeploy.pytests/integration/defs/accuracy/test_llm_api_pytorch.pytests/integration/defs/accuracy/test_llm_api_pytorch_encode.pytests/integration/defs/accuracy/test_llm_api_pytorch_multimodal.pytests/integration/defs/conftest.pytests/integration/defs/examples/test_exaone.pytests/integration/defs/examples/test_gemma.pytests/integration/defs/examples/test_llama.pytests/integration/defs/kv_cache/test_kv_cache_v2_scheduler.pytests/integration/defs/llmapi/test_llm_e2e.pytests/integration/defs/perf/_model_paths.pytests/integration/defs/perf/allowed_configs.pytests/integration/defs/perf/pytorch_model_config.pytests/integration/defs/perf/sampler_options_config.pytests/integration/defs/perf/test_perf.pytests/integration/defs/test_e2e.pytests/integration/defs/triton_server/build_engines.pytests/integration/defs/triton_server/build_model.shtests/integration/defs/triton_server/test_triton.pytests/integration/defs/triton_server/test_triton_llm.pytests/integration/defs/triton_server/test_triton_rcca.pytests/integration/test_lists/qa/llm_function_core.txttests/integration/test_lists/qa/llm_function_l20.txttests/integration/test_lists/qa/llm_function_rtx6k.txttests/integration/test_lists/qa/llm_spark_func.ymltests/integration/test_lists/qa/llm_spark_perf.ymltests/integration/test_lists/qa/llm_triton_integration.txttests/integration/test_lists/test-db/l0_a10.ymltests/integration/test_lists/test-db/l0_a100.ymltests/integration/test_lists/test-db/l0_a30.ymltests/integration/test_lists/test-db/l0_b200.ymltests/integration/test_lists/test-db/l0_dgx_b300.ymltests/integration/test_lists/test-db/l0_dgx_h100.ymltests/integration/test_lists/test-db/l0_dgx_h200.ymltests/integration/test_lists/test-db/l0_gb203.ymltests/integration/test_lists/test-db/l0_gh200.ymltests/integration/test_lists/test-db/l0_h100.ymltests/integration/test_lists/test-db/l0_l40s.ymltests/integration/test_lists/waives.txttests/microbenchmarks/build_time_benchmark.pytests/test_common/llm_data.pytests/unittest/_torch/modeling/test_modeling_exaone4.pytests/unittest/_torch/modeling/test_modeling_gemma3.pytests/unittest/llmapi/apps/_test_openai_consistent_chat.pytests/unittest/llmapi/apps/_test_openai_lora.pytests/unittest/llmapi/apps/_test_trtllm_serve_lora.pytests/unittest/llmapi/lora_test_utils.pytests/unittest/llmapi/test_executor.pytests/unittest/llmapi/test_grpc.pytests/unittest/llmapi/test_llm.pytests/unittest/llmapi/test_llm_multi_gpu.pytests/unittest/llmapi/test_llm_multi_gpu_pytorch.pytests/unittest/llmapi/test_llm_pytorch.pytests/unittest/others/test_layer.pytests/unittest/others/test_llama_conversion.shtests/unittest/trt/model/test_llama.pytests/unittest/trt/model_api/test_model_api_multi_gpu.pytests/unittest/trt/model_api/test_model_level_api.pytests/unittest/trt/model_api/test_model_quantization.py
💤 Files with no reviewable changes (52)
- tests/unittest/others/test_llama_conversion.sh
- tests/integration/test_lists/qa/llm_function_l20.txt
- tests/integration/defs/accuracy/references/json_mode_eval.yaml
- tests/integration/test_lists/test-db/l0_gh200.yml
- tests/unittest/llmapi/apps/_test_trtllm_serve_lora.py
- tests/integration/defs/examples/test_exaone.py
- tests/integration/defs/perf/sampler_options_config.py
- tests/integration/test_lists/test-db/l0_dgx_b300.yml
- tests/integration/test_lists/test-db/l0_gb203.yml
- tests/integration/test_lists/test-db/l0_a10.yml
- tests/integration/defs/accuracy/test_llm_api_pytorch_encode.py
- tests/integration/test_lists/test-db/l0_dgx_h100.yml
- tests/unittest/trt/model_api/test_model_quantization.py
- tests/integration/test_lists/qa/llm_triton_integration.txt
- tests/integration/defs/examples/test_gemma.py
- tests/integration/defs/accuracy/references/gsm8k.yaml
- tests/integration/test_lists/test-db/l0_dgx_h200.yml
- tests/integration/defs/accuracy/references/mmmu.yaml
- tests/microbenchmarks/build_time_benchmark.py
- tests/integration/test_lists/test-db/l0_l40s.yml
- tests/unittest/trt/model_api/test_model_level_api.py
- tests/unittest/_torch/modeling/test_modeling_gemma3.py
- tests/integration/defs/perf/pytorch_model_config.py
- tests/integration/test_lists/test-db/l0_b200.yml
- tests/integration/test_lists/qa/llm_function_rtx6k.txt
- tests/unittest/llmapi/apps/_test_openai_lora.py
- tests/unittest/_torch/modeling/test_modeling_exaone4.py
- tests/integration/test_lists/qa/llm_spark_perf.yml
- tests/integration/test_lists/test-db/l0_a100.yml
- tests/integration/defs/llmapi/test_llm_e2e.py
- tests/integration/defs/accuracy/references/cnn_dailymail.yaml
- tests/integration/defs/accuracy/test_llm_api_pytorch_multimodal.py
- tests/integration/defs/accuracy/references/mmlu.yaml
- tests/integration/defs/triton_server/test_triton.py
- tests/integration/test_lists/qa/llm_spark_func.yml
- tests/unittest/others/test_layer.py
- tests/integration/defs/.test_durations
- tests/integration/defs/perf/allowed_configs.py
- tests/integration/defs/triton_server/build_model.sh
- tests/integration/defs/accuracy/test_llm_api_autodeploy.py
- tests/integration/defs/triton_server/build_engines.py
- tests/integration/defs/perf/_model_paths.py
- tests/integration/test_lists/waives.txt
- tests/unittest/llmapi/test_llm_multi_gpu_pytorch.py
- tests/integration/defs/accuracy/test_cli_flow.py
- tests/integration/defs/accuracy/test_llm_api_pytorch.py
- tests/integration/defs/triton_server/test_triton_llm.py
- tests/integration/test_lists/qa/llm_function_core.txt
- tests/test_common/llm_data.py
- tests/integration/defs/kv_cache/test_kv_cache_v2_scheduler.py
- tests/integration/defs/accuracy/test_disaggregated_serving.py
- tests/integration/test_lists/test-db/l0_a30.yml
88792e9 to
9829d94
Compare
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. An unexpected error occurred while generating fixes: Not Found - https://docs.github.com/rest/git/refs#get-a-reference |
|
/bot run |
|
PR_Github #50749 [ run ] triggered by Bot. Commit: |
|
PR_Github #50749 [ run ] completed with state
|
|
Thanks for doing this! |
|
/bot run --skip-test |
|
PR_Github #51553 [ run ] triggered by Bot. Commit: |
|
/bot run --skip-test |
|
PR_Github #51553 [ run ] completed with state
|
|
PR_Github #51586 [ run ] triggered by Bot. Commit: |
|
PR_Github #51586 [ run ] completed with state |
Signed-off-by: xinhe-nv <200704525+xinhe-nv@users.noreply.github.com>
|
/bot run --skip-test |
|
PR_Github #51710 [ run ] triggered by Bot. Commit: |
|
PR_Github #51710 [ run ] completed with state
|
A missing comma at the boundary of two merged duration blocks made .test_durations invalid JSON, causing pytest_split to abort collection with an INTERNALERROR during --collect-only. Signed-off-by: Xin He (SW-GPU) <200704525+xinhe-nv@users.noreply.github.com>
|
/bot run --skip-test |
|
PR_Github #51734 [ run ] triggered by Bot. Commit: |
|
PR_Github #51734 [ run ] completed with state |
|
/bot reuse-pipeline |
|
PR_Github #51770 [ reuse-pipeline ] triggered by Bot. Commit: |
|
PR_Github #51770 [ reuse-pipeline ] completed with state |
|
/bot reuse-pipeline |
|
PR_Github #51775 [ reuse-pipeline ] triggered by Bot. Commit: |
|
PR_Github #51775 [ reuse-pipeline ] completed with state |
Summary by CodeRabbit
Tests
Chores
Description
Test Coverage
PR Checklist
Please review the following before submitting your PR:
PR description clearly explains what and why. If using CodeRabbit's summary, please make sure it makes sense.
PR Follows TRT-LLM CODING GUIDELINES to the best of your knowledge.
Test cases are provided for new code paths (see test instructions)
If PR introduces API changes, an appropriate PR label is added - either
api-compatibleorapi-breaking. Forapi-breaking, includeBREAKINGin the PR title.Any new dependencies have been scanned for license and vulnerabilities
CODEOWNERS updated if ownership changes
Documentation updated as needed
Update tava architecture diagram if there is a significant design change in PR.
The reviewers assigned automatically/manually are appropriate for the PR.
Please check this after reviewing the above items as appropriate for this PR.
GitHub Bot Help
To see a list of available CI bot commands, please comment
/bot help.