Skip to content

Refactor setups_tag tests to use direct method calls for non-happy-path tests #298

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
PGijsbers merged 4 commits into from
Mar 31, 2026
+53 −24
Merged

Refactor setups_tag tests to use direct method calls for non-happy-path tests #298

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
386da4c
Split setup tag tests: API vs direct
Mar 27, 2026
ee61275
fix(setups_tag tests): assert tag list and tidy tag strings
Mar 27, 2026
0758b5f
Validate full json response
PGijsbers Mar 31, 2026
0edec8d
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Mar 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 53 additions & 24 deletions tests/routers/openml/setups_tag_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import re
from http import HTTPStatus

import httpx
import pytest
from sqlalchemy import text
from sqlalchemy.ext.asyncio import AsyncConnection

from tests.users import ApiKey
from core.errors import SetupNotFoundError, TagAlreadyExistsError
from routers.openml.setups import tag_setup
from tests.users import SOME_USER, ApiKey


async def test_setup_tag_missing_auth(py_api: httpx.AsyncClient) -> None:
Expand All @@ -16,41 +17,69 @@ async def test_setup_tag_missing_auth(py_api: httpx.AsyncClient) -> None:
assert response.json()["detail"] == "Authentication failed"


async def test_setup_tag_unknown_setup(py_api: httpx.AsyncClient) -> None:
@pytest.mark.mut
async def test_setup_tag_api_success(
py_api: httpx.AsyncClient, expdb_test: AsyncConnection
) -> None:
tag = "setup_tag_via_http"
response = await py_api.post(
f"/setup/tag?api_key={ApiKey.SOME_USER}",
json={"setup_id": 999999, "tag": "test_tag"},
json={"setup_id": 1, "tag": tag},
)
assert response.status_code == HTTPStatus.NOT_FOUND
assert re.match(r"Setup \d+ not found.", response.json()["detail"])

assert response.status_code == HTTPStatus.OK
expected = {"setup_tag": {"id": "1", "tag": ["setup_tag_via_http"]}}
assert expected == response.json()

rows = await expdb_test.execute(
text("SELECT * FROM setup_tag WHERE id = 1 AND tag = :tag"),
parameters={"tag": tag},
)
assert len(rows.all()) == 1


# ── Direct call tests: tag_setup ──


async def test_setup_tag_unknown_setup(expdb_test: AsyncConnection) -> None:
with pytest.raises(SetupNotFoundError, match=r"Setup \d+ not found."):
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Tighten the exception regex in Line 45.

not found. uses . as a wildcard. Escape the trailing period (or assert the exact message) to avoid false positives.

Proposed fix 
-    with pytest.raises(SetupNotFoundError, match=r"Setup \d+ not found."):
+    with pytest.raises(SetupNotFoundError, match=r"Setup \d+ not found\."):
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
with pytest.raises(SetupNotFoundError, match=r"Setup \d+ not found."):
with pytest.raises(SetupNotFoundError, match=r"Setup \d+ not found\."):
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@tests/routers/openml/setups_tag_test.py` at line 45, The pytest assertion
using pytest.raises for SetupNotFoundError currently uses the regex r"Setup \d+
not found." where the trailing dot is unescaped; update the match to either
escape the period (r"Setup \d+ not found\.") or assert the exact message (e.g.,
match=r"^Setup \d+ not found\.$") to prevent the dot acting as a wildcard and
ensure the error message is matched precisely in the with pytest.raises(...)
line.

await tag_setup(
setup_id=999999,
tag="test_tag",
user=SOME_USER,
expdb_db=expdb_test,
)


@pytest.mark.mut
async def test_setup_tag_already_exists(
py_api: httpx.AsyncClient, expdb_test: AsyncConnection
) -> None:
async def test_setup_tag_already_exists(expdb_test: AsyncConnection) -> None:
tag = "setup_tag_conflict"
await expdb_test.execute(
text("INSERT INTO setup_tag (id, tag, uploader) VALUES (1, 'existing_tag_123', 2);")
text("INSERT INTO setup_tag (id, tag, uploader) VALUES (1, :tag, 2);"),
parameters={"tag": tag},
)
response = await py_api.post(
f"/setup/tag?api_key={ApiKey.SOME_USER}",
json={"setup_id": 1, "tag": "existing_tag_123"},
)
assert response.status_code == HTTPStatus.CONFLICT
assert response.json()["detail"] == "Setup 1 already has tag 'existing_tag_123'."
with pytest.raises(TagAlreadyExistsError, match=rf"Setup 1 already has tag '{tag}'\."):
await tag_setup(
setup_id=1,
tag=tag,
user=SOME_USER,
expdb_db=expdb_test,
)


@pytest.mark.mut
async def test_setup_tag_success(py_api: httpx.AsyncClient, expdb_test: AsyncConnection) -> None:
response = await py_api.post(
f"/setup/tag?api_key={ApiKey.SOME_USER}",
json={"setup_id": 1, "tag": "my_new_success_tag"},
async def test_setup_tag_direct_success(expdb_test: AsyncConnection) -> None:
tag = "setup_tag_via_direct"
result = await tag_setup(
setup_id=1,
tag=tag,
user=SOME_USER,
expdb_db=expdb_test,
)

assert response.status_code == HTTPStatus.OK
assert "my_new_success_tag" in response.json()["setup_tag"]["tag"]

assert result["setup_tag"]["tag"][-1] == tag
rows = await expdb_test.execute(
text("SELECT * FROM setup_tag WHERE id = 1 AND tag = 'my_new_success_tag'")
text("SELECT * FROM setup_tag WHERE id = 1 AND tag = :tag"),
parameters={"tag": tag},
)
assert len(rows.all()) == 1
Loading