Add vLLM-metax vllm metax cmake presets output flag#307
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds an --output command-line argument to generate_cmake_presets.py to allow specifying a custom output path, and includes a test to verify this behavior. The review feedback points out a dangerous global mock of os.path.exists in the new test and suggests a safer alternative of creating a dummy file in the temporary directory.
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) | ||
| monkeypatch.setattr(presets, "which", lambda name: None) | ||
| monkeypatch.setattr(presets, "get_cpu_cores", lambda: 8) |
There was a problem hiding this comment.
Mocking os.path.exists globally via monkeypatch.setattr(presets.os.path, "exists", ...) is highly discouraged and dangerous. Since os.path is a shared module, this mock affects the entire Python process during the test execution, which can cause unpredictable failures or hangs in other tests, pytest internals, or teardown processes.\n\nInstead of mocking os.path.exists, you can simply create the dummy nvcc file in the temporary directory tmp_path. This allows the real os.path.exists to return True naturally and safely.
nvcc_bin = tmp_path / "bin"\n nvcc_bin.mkdir(parents=True, exist_ok=True)\n (nvcc_bin / "nvcc").touch()\n\n monkeypatch.setattr(presets, "CUDA_HOME", str(tmp_path))\n monkeypatch.setattr(presets, "which", lambda name: None)\n monkeypatch.setattr(presets, "get_cpu_cores", lambda: 8)
Summary
Validation
Review notes