Add vLLM-metax vllm metax cmake presets build type#302
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds support for specifying a custom CMake build type (such as Debug, Release, RelWithDebInfo, or MinSizeRel) when generating CMake presets, both programmatically and via a new --build-type CLI argument. A new test has been added to verify this functionality. The review feedback points out that mocking os.path.exists globally in the test can cause side effects and suggests creating a dummy compiler file within the tmp_path directory instead.
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.
| monkeypatch.setattr(presets, "CUDA_HOME", str(tmp_path)) | ||
| monkeypatch.setattr(presets.os.path, "exists", lambda path: True) |
There was a problem hiding this comment.
Mocking os.path.exists globally on the os.path module (via presets.os.path) has global side effects because os.path is a shared module across the entire Python process. Any other test, fixture, or pytest teardown process that calls os.path.exists during this test's execution will receive True, which can lead to flaky tests, unexpected failures, or cleanup issues.
Instead of mocking os.path.exists, we can leverage the tmp_path fixture to create a dummy nvcc file at the expected path (tmp_path / "bin" / "nvcc"). This allows the real os.path.exists to return True naturally for the compiler check, while avoiding any global side effects and keeping the test clean and robust.
| monkeypatch.setattr(presets, "CUDA_HOME", str(tmp_path)) | |
| monkeypatch.setattr(presets.os.path, "exists", lambda path: True) | |
| monkeypatch.setattr(presets, "CUDA_HOME", str(tmp_path)) | |
| nvcc_path = tmp_path / "bin" / "nvcc" | |
| nvcc_path.parent.mkdir(parents=True, exist_ok=True) | |
| nvcc_path.touch() |
Summary
Validation
Review notes