diff --git a/TECHNICAL_REPORTS/2017-common-url-helper-tests-20260928.en.md b/TECHNICAL_REPORTS/2017-common-url-helper-tests-20260928.en.md new file mode 100644 index 000000000..d2e64db5b --- /dev/null +++ b/TECHNICAL_REPORTS/2017-common-url-helper-tests-20260928.en.md @@ -0,0 +1,30 @@ +# Technical Report: PR #2017 (Cover the `_common.py` URL normalization helpers) + +**Date:** 2026-09-28 +**Issue:** #1674 +**Scope:** Test-only addition; no production code changed. + +## Problem and decision + +`normalize_base_url`, `native_base_url`, and `connect_base_url` in `python/src/mlxcel/_common.py` (lines 70, 78, 83) are pure functions that append or strip `/v1` and derive the Unix-socket default base URL. They had no direct test coverage. Worse, every existing test that touched a `base_url` already passed a pre-normalized value ending in `/v1` (`python/tests/test_client_mock.py:254`, `:264`, `:445`), so the branch that appends `/v1` and the branch that strips it were never exercised under test, even though the suite reports full pass status. + +The fix adds a parametrized test section, `# -- URL normalization --`, to `python/tests/test_client_mock.py`, placed between the existing `mode-selection / validation` and `sampling unit` sections to match the file's established grouping: + +- `test_normalize_base_url`: four cases (a bare root, a bare root with a trailing slash, an already-`/v1` URL, and a `/v1` URL with a trailing slash), asserting the `rstrip("/")` and conditional `/v1` append both work. +- `test_native_base_url`: the `/v1` strip and the passthrough case where the input has no `/v1` suffix. +- `test_connect_base_url_normalizes_given_base_url` and `test_connect_base_url_defaults_to_socket_base`: an explicit `base_url` routes through `normalize_base_url`, and `None` falls back to `f"{UDS_BASE}/v1"`. + +No alternative design was considered; this is direct unit coverage of pure functions, following the same `@pytest.mark.parametrize` pattern already used for `_sampling.py` in the same file. + +## Validation + +- Premise re-verified on current `main` before implementing: the three helpers were still at the line numbers the issue cited, and a grep of `python/tests/` for their names returned zero hits. +- `pytest python/tests -m "not e2e" -q`: 43 passed before the change, 51 passed after (8 new tests, 2 e2e tests deselected as before). +- `ruff check python` and `ruff format --check python`: clean. +- `mypy python/src`: no issues in 7 source files. +- Independent implementation, security, and performance review found no CRITICAL, HIGH, or MEDIUM findings and made no fix commits; a separate finalization pass confirmed no documentation references these internal helpers and made no changes. +- `python3 scripts/ci/check_cross_repo_refs.py`: no bare cross-repo issue references introduced. + +## Limits + +This PR does not change any production behavior; it only closes a coverage gap. Review noted two optional LOW-severity gaps left for a future PR if ever needed: `connect_base_url("")` (empty string, not `None`) is not separately tested, and `native_base_url` on a URL ending in `/v1/` (trailing slash after `/v1`) returns the input unchanged rather than stripping it, which is undocumented but matches current caller behavior since callers always pass already-normalized URLs. diff --git a/TECHNICAL_REPORTS/2017-common-url-helper-tests-20260928.ko.md b/TECHNICAL_REPORTS/2017-common-url-helper-tests-20260928.ko.md new file mode 100644 index 000000000..728500792 --- /dev/null +++ b/TECHNICAL_REPORTS/2017-common-url-helper-tests-20260928.ko.md @@ -0,0 +1,30 @@ +# 기술 리포트: PR #2017 (`_common.py` URL 정규화 헬퍼 테스트 보강) + +**작성일:** 2026-09-28 +**이슈:** #1674 +**범위:** 테스트 전용 추가. 프로덕션 코드 변경 없음. + +## 문제와 결정 + +`python/src/mlxcel/_common.py`(70, 78, 83번째 줄)의 `normalize_base_url`, `native_base_url`, `connect_base_url`은 `/v1`을 붙이거나 제거하고 Unix 소켓 기본 base URL을 도출하는 순수 함수다. 이 세 함수에는 직접 테스트가 없었다. 더 나아가 `base_url`을 다루는 기존 테스트는 모두 이미 `/v1`로 끝나는 값을 전달했으므로(`python/tests/test_client_mock.py:254`, `:264`, `:445`), 스위트가 전부 통과한다고 보고하는데도 `/v1`을 붙이는 분기와 제거하는 분기는 실제로 테스트를 거친 적이 없었다. + +수정은 `python/tests/test_client_mock.py`에 `# -- URL normalization --` 파라미터화 테스트 섹션을 추가하며, 파일이 이미 사용하는 구획 방식에 맞춰 기존 `mode-selection / validation` 섹션과 `sampling unit` 섹션 사이에 배치했다. + +- `test_normalize_base_url`: 네 가지 케이스, 즉 단순 루트, 끝에 슬래시가 붙은 루트, 이미 `/v1`인 URL, 끝에 슬래시가 붙은 `/v1` URL을 통해 `rstrip("/")`와 조건부 `/v1` 추가를 모두 검증한다. +- `test_native_base_url`: `/v1` 제거 케이스와 `/v1` 접미사가 없을 때 값을 그대로 통과시키는 케이스. +- `test_connect_base_url_normalizes_given_base_url`, `test_connect_base_url_defaults_to_socket_base`: 명시적 `base_url`이 `normalize_base_url`을 거치는 경우와, `None`일 때 `f"{UDS_BASE}/v1"`로 폴백하는 경우. + +다른 설계안은 검토하지 않았다. 이는 순수 함수에 대한 직접적인 단위 테스트이며, 같은 파일에서 `_sampling.py`에 이미 쓰이고 있는 `@pytest.mark.parametrize` 패턴을 그대로 따른다. + +## 검증 + +- 구현 전에 현재 `main`에서 전제를 다시 확인했다. 세 헬퍼는 이슈가 언급한 줄 번호에 그대로 있었고, `python/tests/`에서 세 함수명을 grep하면 결과가 0건이었다. +- `pytest python/tests -m "not e2e" -q`: 변경 전 43개 통과, 변경 후 51개 통과(신규 테스트 8개 추가, e2e 테스트 2개는 이전과 동일하게 제외). +- `ruff check python`, `ruff format --check python`: 이상 없음. +- `mypy python/src`: 소스 파일 7개 모두 문제 없음. +- 독립적으로 수행한 구현, 보안, 성능 검토에서 CRITICAL, HIGH, MEDIUM 등급 발견 사항이 없었고 수정 커밋도 없었다. 별도의 마무리 점검에서 이 내부 헬퍼를 참조하는 문서가 없음을 확인했고 변경하지 않았다. +- `python3 scripts/ci/check_cross_repo_refs.py`: 순수 번호(bare) 형태의 크로스 저장소 이슈 참조가 새로 추가되지 않았음을 확인. + +## 한계 + +이 PR은 프로덕션 동작을 전혀 변경하지 않으며, 테스트 커버리지 공백만 메운다. 검토 과정에서 필요시 향후 PR에서 다룰 수 있는 LOW 등급 선택 사항 두 가지를 남겼다. `connect_base_url("")`(빈 문자열, `None`이 아닌 경우)는 별도로 테스트되지 않았고, `/v1/`(끝에 슬래시가 붙은 `/v1`)로 끝나는 URL에 대해 `native_base_url`은 입력을 그대로 반환한다. 이는 호출자가 항상 이미 정규화된 URL을 전달하기 때문에 현재 동작과 일치하지만 문서화되지는 않았다. diff --git a/python/tests/test_client_mock.py b/python/tests/test_client_mock.py index 19abbb14d..9bf94688e 100644 --- a/python/tests/test_client_mock.py +++ b/python/tests/test_client_mock.py @@ -9,6 +9,7 @@ import pytest import mlxcel +from mlxcel._common import UDS_BASE, connect_base_url, native_base_url, normalize_base_url from mlxcel._sampling import build_params MODEL_ID = "mock-model" @@ -264,6 +265,41 @@ def test_base_url_and_socket_is_error() -> None: mlxcel.LLM(base_url="http://x/v1", socket="/tmp/x.sock") +# -- URL normalization ------------------------------------------------------- + + +@pytest.mark.parametrize( + ("base_url", "expected"), + [ + ("http://localhost:8080", "http://localhost:8080/v1"), + ("http://localhost:8080/", "http://localhost:8080/v1"), + ("http://localhost:8080/v1", "http://localhost:8080/v1"), + ("http://localhost:8080/v1/", "http://localhost:8080/v1"), + ], +) +def test_normalize_base_url(base_url: str, expected: str) -> None: + assert normalize_base_url(base_url) == expected + + +@pytest.mark.parametrize( + ("openai_base_url", "expected"), + [ + ("http://localhost:8080/v1", "http://localhost:8080"), + ("http://localhost:8080", "http://localhost:8080"), + ], +) +def test_native_base_url(openai_base_url: str, expected: str) -> None: + assert native_base_url(openai_base_url) == expected + + +def test_connect_base_url_normalizes_given_base_url() -> None: + assert connect_base_url("http://localhost:8080") == "http://localhost:8080/v1" + + +def test_connect_base_url_defaults_to_socket_base() -> None: + assert connect_base_url(None) == f"{UDS_BASE}/v1" + + # -- sampling unit ----------------------------------------------------------