mirror of https://github.com/razor-ai/soup.git
test: add missing coverage for _parse_json_array, _validate_example, SSRF guards
Addresses TDD review findings: - Tests for _parse_json_array (markdown fences, NDJSON fallback, empty, invalid) - Tests for _validate_example (alpaca, sharegpt, chatml, unknown format) - Tests for _generate_server SSRF validation (scheme whitelist, remote HTTP block) - Tests for SGLang SSRF validation (URL model path rejection) - Fix vacuous test_server_provider_accepted assertion 1369 tests, 58.84% coverage.
This commit is contained in:
Alpamys
parent
bc743e2814
commit
fb1f49afc8
|
|
@ -11,12 +11,20 @@ import pytest
|
|||
class TestServerProviderValidation:
|
||||
"""Test that 'server' is a valid provider for soup data generate."""
|
||||
|
||||
def test_server_provider_accepted(self):
|
||||
"""'server' should be a valid provider choice."""
|
||||
def test_server_provider_routes_without_error(self):
|
||||
"""'server' should route to _generate_server without error."""
|
||||
from soup_cli.commands.generate import _generate_batch
|
||||
|
||||
# _generate_batch dispatches to the correct provider
|
||||
# Verify it doesn't reject 'server' as invalid
|
||||
assert "server" in ("openai", "local", "server")
|
||||
with mock_patch(
|
||||
"soup_cli.commands.generate._generate_server",
|
||||
return_value=[],
|
||||
):
|
||||
result = _generate_batch(
|
||||
prompt="test", count=1, fmt="alpaca",
|
||||
provider="server", model_name="m", api_key=None,
|
||||
api_base=None, temperature=0.8, seed_examples=[],
|
||||
)
|
||||
assert result == []
|
||||
|
||||
def test_invalid_provider_rejected(self):
|
||||
"""Invalid providers should cause an exit."""
|
||||
|
|
@ -305,3 +313,172 @@ class TestGenerateBatchRouting:
|
|||
)
|
||||
|
||||
mock_local.assert_called_once()
|
||||
|
||||
|
||||
# ─── _parse_json_array Tests ─────────────────────────────────────────────
|
||||
|
||||
|
||||
class TestParseJsonArray:
|
||||
"""Test the JSON array parsing function used by all providers."""
|
||||
|
||||
def test_parse_valid_json_array(self):
|
||||
"""Should parse a clean JSON array."""
|
||||
from soup_cli.commands.generate import _parse_json_array
|
||||
|
||||
result = _parse_json_array('[{"a": 1}, {"b": 2}]')
|
||||
assert len(result) == 2
|
||||
assert result[0]["a"] == 1
|
||||
|
||||
def test_parse_markdown_code_fence(self):
|
||||
"""Should strip markdown code fences."""
|
||||
from soup_cli.commands.generate import _parse_json_array
|
||||
|
||||
content = '```json\n[{"instruction": "test", "output": "ok"}]\n```'
|
||||
result = _parse_json_array(content)
|
||||
assert len(result) == 1
|
||||
assert result[0]["instruction"] == "test"
|
||||
|
||||
def test_parse_json_with_surrounding_text(self):
|
||||
"""Should extract JSON array from surrounding text."""
|
||||
from soup_cli.commands.generate import _parse_json_array
|
||||
|
||||
content = 'Here are the examples:\n[{"a": 1}]\nDone!'
|
||||
result = _parse_json_array(content)
|
||||
assert len(result) == 1
|
||||
|
||||
def test_parse_ndjson_fallback(self):
|
||||
"""Should fall back to line-by-line JSON parsing."""
|
||||
from soup_cli.commands.generate import _parse_json_array
|
||||
|
||||
content = '{"a": 1}\n{"b": 2}\n{"c": 3}'
|
||||
result = _parse_json_array(content)
|
||||
assert len(result) == 3
|
||||
|
||||
def test_parse_empty_array(self):
|
||||
"""Should return empty list for empty array."""
|
||||
from soup_cli.commands.generate import _parse_json_array
|
||||
|
||||
assert _parse_json_array("[]") == []
|
||||
|
||||
def test_parse_invalid_json_returns_empty(self):
|
||||
"""Should return empty list for completely invalid JSON."""
|
||||
from soup_cli.commands.generate import _parse_json_array
|
||||
|
||||
assert _parse_json_array("not json at all") == []
|
||||
|
||||
def test_parse_filters_non_dict_items(self):
|
||||
"""Should filter out non-dict items from the array."""
|
||||
from soup_cli.commands.generate import _parse_json_array
|
||||
|
||||
result = _parse_json_array('[{"a": 1}, "string", 42, {"b": 2}]')
|
||||
assert len(result) == 2
|
||||
|
||||
def test_parse_code_fence_without_language(self):
|
||||
"""Should strip code fences without language specifier."""
|
||||
from soup_cli.commands.generate import _parse_json_array
|
||||
|
||||
content = '```\n[{"x": 1}]\n```'
|
||||
result = _parse_json_array(content)
|
||||
assert len(result) == 1
|
||||
|
||||
|
||||
# ─── _validate_example Tests ─────────────────────────────────────────────
|
||||
|
||||
|
||||
class TestValidateExample:
|
||||
"""Test format validation for generated examples."""
|
||||
|
||||
def test_validate_alpaca_valid(self):
|
||||
from soup_cli.commands.generate import _validate_example
|
||||
|
||||
assert _validate_example({"instruction": "Q", "output": "A"}, "alpaca")
|
||||
|
||||
def test_validate_alpaca_missing_output(self):
|
||||
from soup_cli.commands.generate import _validate_example
|
||||
|
||||
assert not _validate_example({"instruction": "Q"}, "alpaca")
|
||||
|
||||
def test_validate_sharegpt_valid(self):
|
||||
from soup_cli.commands.generate import _validate_example
|
||||
|
||||
row = {
|
||||
"conversations": [
|
||||
{"from": "human", "value": "Hi"},
|
||||
{"from": "gpt", "value": "Hello"},
|
||||
]
|
||||
}
|
||||
assert _validate_example(row, "sharegpt")
|
||||
|
||||
def test_validate_sharegpt_too_few_turns(self):
|
||||
from soup_cli.commands.generate import _validate_example
|
||||
|
||||
row = {"conversations": [{"from": "human", "value": "Hi"}]}
|
||||
assert not _validate_example(row, "sharegpt")
|
||||
|
||||
def test_validate_chatml_valid(self):
|
||||
from soup_cli.commands.generate import _validate_example
|
||||
|
||||
row = {
|
||||
"messages": [
|
||||
{"role": "user", "content": "Hi"},
|
||||
{"role": "assistant", "content": "Hello"},
|
||||
]
|
||||
}
|
||||
assert _validate_example(row, "chatml")
|
||||
|
||||
def test_validate_chatml_empty_messages(self):
|
||||
from soup_cli.commands.generate import _validate_example
|
||||
|
||||
assert not _validate_example({"messages": []}, "chatml")
|
||||
|
||||
def test_validate_unknown_format(self):
|
||||
from soup_cli.commands.generate import _validate_example
|
||||
|
||||
assert not _validate_example({"a": 1}, "unknown")
|
||||
|
||||
|
||||
# ─── SSRF Validation Tests ───────────────────────────────────────────────
|
||||
|
||||
|
||||
class TestServerSSRFValidation:
|
||||
"""Test SSRF protection in _generate_server."""
|
||||
|
||||
def test_server_blocks_non_http_scheme(self):
|
||||
"""file:// scheme should be rejected."""
|
||||
from soup_cli.commands.generate import _generate_server
|
||||
|
||||
with pytest.raises(ValueError, match="HTTP or HTTPS"):
|
||||
_generate_server(
|
||||
prompt="test", count=1, fmt="alpaca",
|
||||
model_name="m", api_base="file:///etc/passwd",
|
||||
temperature=0.8, seed_examples=[],
|
||||
)
|
||||
|
||||
def test_server_blocks_remote_http(self):
|
||||
"""Remote HTTP (non-localhost) should be rejected."""
|
||||
from soup_cli.commands.generate import _generate_server
|
||||
|
||||
with pytest.raises(ValueError, match="HTTPS for remote"):
|
||||
_generate_server(
|
||||
prompt="test", count=1, fmt="alpaca",
|
||||
model_name="m", api_base="http://169.254.169.254/latest",
|
||||
temperature=0.8, seed_examples=[],
|
||||
)
|
||||
|
||||
def test_server_allows_localhost_http(self):
|
||||
"""HTTP to localhost should be allowed."""
|
||||
from soup_cli.commands.generate import _generate_server
|
||||
|
||||
mock_response = MagicMock()
|
||||
mock_response.status_code = 200
|
||||
mock_response.json.return_value = {
|
||||
"choices": [{"message": {"content": "[]"}}]
|
||||
}
|
||||
|
||||
with mock_patch("httpx.post", return_value=mock_response):
|
||||
result = _generate_server(
|
||||
prompt="test", count=1, fmt="alpaca",
|
||||
model_name="m", api_base="http://127.0.0.1:8000",
|
||||
temperature=0.8, seed_examples=[],
|
||||
)
|
||||
assert result == []
|
||||
|
|
|
|||
|
|
@ -3,6 +3,8 @@
|
|||
from unittest.mock import MagicMock
|
||||
from unittest.mock import patch as mock_patch
|
||||
|
||||
import pytest
|
||||
|
||||
# ─── SGLang Detection Tests ──────────────────────────────────────────────
|
||||
|
||||
|
||||
|
|
@ -254,3 +256,49 @@ class TestDoctorSGLang:
|
|||
|
||||
pkg_names = [dep[1] for dep in DEPS]
|
||||
assert "librosa" in pkg_names
|
||||
|
||||
|
||||
# ─── SGLang SSRF Validation Tests ────────────────────────────────────────
|
||||
|
||||
|
||||
class TestSGLangSSRF:
|
||||
"""Test SSRF protection in SGLang runtime creation."""
|
||||
|
||||
def test_create_runtime_blocks_http_model_path(self):
|
||||
"""HTTP URLs for model_path should be rejected."""
|
||||
mock_sgl = MagicMock()
|
||||
|
||||
with mock_patch.dict("sys.modules", {"sglang": mock_sgl}):
|
||||
from soup_cli.utils.sglang import create_sglang_runtime
|
||||
|
||||
with pytest.raises(ValueError, match="not a URL"):
|
||||
create_sglang_runtime(
|
||||
model_path="http://evil.com/model",
|
||||
)
|
||||
|
||||
def test_create_runtime_blocks_http_base_model(self):
|
||||
"""HTTP URLs for base_model should be rejected."""
|
||||
mock_sgl = MagicMock()
|
||||
|
||||
with mock_patch.dict("sys.modules", {"sglang": mock_sgl}):
|
||||
from soup_cli.utils.sglang import create_sglang_runtime
|
||||
|
||||
with pytest.raises(ValueError, match="not a URL"):
|
||||
create_sglang_runtime(
|
||||
model_path="/path/to/adapter",
|
||||
base_model="https://evil.com/model",
|
||||
is_adapter=True,
|
||||
)
|
||||
|
||||
def test_create_runtime_allows_hf_model_id(self):
|
||||
"""HuggingFace model IDs should be allowed."""
|
||||
mock_sgl = MagicMock()
|
||||
mock_sgl.Runtime.return_value = MagicMock()
|
||||
|
||||
with mock_patch.dict("sys.modules", {"sglang": mock_sgl}):
|
||||
from soup_cli.utils.sglang import create_sglang_runtime
|
||||
|
||||
runtime, name = create_sglang_runtime(
|
||||
model_path="meta-llama/Llama-3.1-8B",
|
||||
)
|
||||
assert name == "meta-llama/Llama-3.1-8B"
|
||||
|
|
|
|||
Loading…
Reference in New Issue