lablup / lablup/mlxcel

test(python): cover the _common.py URL normalization helpers

Open Beginner friendly
#1,674 0 comments 0 reactions 0 assignees View on GitHub
priority:low status:ready type:test
Dominant language
Rust
Stars
467
Forks
54
Avg merge
4h 25m
Merged PRs (30d)
310

Description

## Summary

The URL helpers in the Python client's `_common.py` have no direct test coverage, and the existing tests only ever pass pre-normalized URLs, so the normalization branches never run under test.

## Background

`normalize_base_url`, `native_base_url`, and `connect_base_url` are pure functions that append or strip `/v1` and derive the socket base. Every test that touches a `base_url` passes one that already ends in `/v1`, so the "append `/v1`" branch and the `native_base_url` strip are never exercised. The sibling `_sampling.py` already has four dedicated tests, so the pattern is established.

## Proposed Solution

Add roughly 15 lines of parametrized asserts over the three helpers, covering trailing slash, missing `/v1`, already-normalized input, and the socket-derived default. These are pure functions, so no fixtures or server are needed. `.github/workflows/python.yml` already runs the suite on 3.9/3.11/3.13.

## Implementation Notes

- `python/src/mlxcel/_common.py:70` defines `normalize_base_url` (rstrip `/`, append `/v1` unless already present), `:78` `native_base_url` (strip trailing `/v1`), and `:83` `connect_base_url` (normalize when a base_url is given, else `{UDS_BASE}/v1`).
- Existing tests at `python/tests/test_client_mock.py:254`, `:264`, `:445` all pass pre-normalized URLs.
- `python/tests/test_client_mock.py:270-306` shows the `_sampling.py` test pattern to imitate.

## Acceptance Criteria

- [ ] All three helpers have direct tests, including the un-normalized input paths

---

## Original Suggestion

### Title: test(python): cover the _common.py URL normalization helpers

The URL helpers in the Python client's `_common.py` have no direct test coverage, and the existing tests only ever pass pre-normalized URLs — so the normalization branches are entirely unexercised.

## Evidence

- `python/src/mlxcel/_common.py:70-85` — `normalize_base_url`, `native_base_url`, `connect_base_url`; a grep of `python/tests/` for the exported helpers finds zero hits
- No test passes a `base_url` without a trailing `/v1` (`python/tests/test_client_mock.py:254`, `:264`, `:445` all pass pre-normalized URLs), so the "append `/v1`" branch and the `native_base_url` strip never run under test
- The sibling `_sampling.py` has four dedicated tests (`python/tests/test_client_mock.py:270-306`), so the pattern is established

## Suggested fix

~15 lines of parametrized asserts over the three helpers (trailing slash, missing `/v1`, already-normalized, socket-derived). Pure functions — no fixtures, no server. `.github/workflows/python.yml` already runs the suite on 3.9/3.11/3.13.

## Acceptance criteria

- [ ] All three helpers have direct tests, including the un-normalized input paths

Contributor guide

Open the contributing guide

Research direction

Start with the helpers at python/src/mlxcel/_common.py:70-85, then follow the dedicated test pattern in python/tests/test_client_mock.py:270-306. Add direct parametrized coverage for normalize_base_url, native_base_url, and connect_base_url, including trailing slashes, missing or existing /v1, and the socket-derived default. Run the Python test suite; done means all three helpers and the un-normalized paths are covered.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
testing
Issue type
Feature
Difficulty
2/5
Estimated time
1-3 hours
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
88/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.