tests/conftest.py (3)

41-50: Solid shared auth header fixture; add basic assertions and avoid double-registration.

This is a nice centralization. Two small tweaks will make failures more diagnosable and shave a bit of overhead:

• Check the login response and presence of access_token explicitly (clearer failures).
• Try login first; only register if login fails. This prevents re-registering the same user when a test calls this multiple times.

Also, the inline import can be safely moved to the module top to fail fast and avoid repeating the import.

Apply:

@@
-from tests import utils
-        return utils.get_auth_header(tokens["access_token"])
+        return utils.get_auth_header(tokens["access_token"])

And refactor the fixture:

@@
+from tests import utils
@@
 @pytest.fixture
 def create_authenticated_headers(register_user, login_user):
-    def _get_headers(email="[email protected]", password="testpassword"):
-        register_user(email, password)
-        resp = login_user(email, password)
-        tokens = resp.get_json()
-        from tests import utils
-        return utils.get_auth_header(tokens["access_token"])
+    def _get_headers(email="[email protected]", password="testpassword"):
+        # Try login first; register only if needed to avoid duplicate registrations
+        resp = login_user(email, password)
+        if resp.status_code != 200:
+            register_user(email, password)
+            resp = login_user(email, password)
+        tokens = resp.get_json() or {}
+        assert resp.status_code == 200, f"login_user failed: {resp.status_code} {tokens}"
+        assert "access_token" in tokens, f"access_token missing in login response: {tokens}"
+        return utils.get_auth_header(tokens["access_token"])
 
     return _get_headers

---

12-20: Tear-down completeness: remove DB session after drop_all.

After dropping all tables, call db.session.remove() to dispose scoped sessions and avoid any lingering connections between tests.

     with app.app_context():
-        db.drop_all()
+        db.drop_all()
+        db.session.remove()

---

5-7: Replace env-var hack with test config/app factory for determinism and isolation.

Setting env vars at import time works but is brittle. Prefer a create_app pattern with a dedicated testing config (deterministic JWT secret, file-based SQLite, engine options) and an app fixture that initializes/tears down the DB per test.

If you want, I can open a follow-up PR to introduce create_app and migrate tests.

Example sketch (outside this diff):

# conftest.py
import pytest
from app import create_app, db

@pytest.fixture
def app():
    app = create_app(
        TESTING=True,
        SQLALCHEMY_DATABASE_URI="sqlite://",  # temp file in memory-safe mode
        JWT_SECRET_KEY="test-secret",
        SQLALCHEMY_ENGINE_OPTIONS={"connect_args": {"check_same_thread": False}},
    )
    with app.app_context():
        db.create_all()
        yield app
        db.drop_all()
        db.session.remove()

@pytest.fixture
def client(app):
    return app.test_client()

tests/test_subcategory.py (10)

16-29: Great helper; consider reusing a single header within tests that create multiple subcategories.

The helper is clean and flexible. In tests where you call create_subcategory more than once, pass a single headers object to avoid repeated register/login calls (faster and less log noise).

Example for get-all test below:

-    def test_get_all_subcategories(self, create_subcategory):
-        create_subcategory("A")
-        create_subcategory("B")
+    def test_get_all_subcategories(self, create_subcategory, create_authenticated_headers):
+        headers = create_authenticated_headers()
+        create_subcategory("A", headers=headers)
+        create_subcategory("B", headers=headers)

---

54-61: Avoid pinning to 500 for duplicates; allow 409 (Conflict) as acceptable.

Returning 500 for uniqueness violations couples tests to an implementation bug/choice. Consider accepting 409 (or 400) to keep tests future-proof.

-        assert response.status_code == 500
+        assert response.status_code in (409, 400, 500)
         assert self._count_subcategories() == 1

Optional: also assert on an error field to ensure this is a duplicate-name failure.

---

75-87: Reuse headers to avoid duplicate user registrations.

As noted earlier, reusing headers avoids repeated auth calls.

-    def test_get_all_subcategories(self, create_subcategory):
-        create_subcategory("A")
-        create_subcategory("B")
+    def test_get_all_subcategories(self, create_subcategory, create_authenticated_headers):
+        headers = create_authenticated_headers()
+        create_subcategory("A", headers=headers)
+        create_subcategory("B", headers=headers)

---

88-103: Update path validated well; consider 200 over 201 for updates.

Semantics look correct, including DB checks. If/when the API shifts toward standard REST semantics, 200 is more typical for successful updates (201 is for resource creation).

-        assert update_resp.status_code == 201
+        assert update_resp.status_code in (200, 201)

---

116-137: Token error tests: good coverage; can be parameterized to reduce duplication.

These three cases repeat the same structure across create/update/delete. Parametrizing keeps things DRY and faster.

Example for create:

@pytest.mark.parametrize(
    "headers_factory, expected_key",
    [
        (lambda self: utils.get_expired_token_headers(self.client.application.app_context()), "token_expired"),
        (lambda self: utils.get_invalid_token_headers(), "invalid_token"),
        (lambda self: None, "authorization_required"),
    ],
)
def test_create_subcategory_token_errors(self, headers_factory):
    headers = headers_factory(self) if headers_factory else None
    resp = self.client.post("/subcategory/create", json={"name": "TokenCase"}, headers=headers)
    utils.verify_token_error_response(resp, expected_key)
    self._verify_subcategory_in_db("TokenCase", should_exist=False)

Repeat similarly for update/delete.

---

139-171: Update token-error paths: same parametrization opportunity.

Same suggestion applies here; consolidating reduces test maintenance.

---

187-221: Delete token-error paths: comprehensive; consider parametrization.

Coverage is complete; recommend the same DRY refactor.

---

62-63: Follow-up: add creation-with-relations tests when ready.

When category/product fixtures are available, add tests that:

• Create categories/products first.
• Create subcategory with relations in payload.
• Assert both the API response and the relation tables in DB.

Happy to supply concrete tests once the data factories are in place.

---

64-74: Consider adding 404/edge-case tests for robustness.

A few cases that would strengthen coverage:

• GET /subcategory/ for a non-existent id -> 404.
• PUT /subcategory//update for a non-existent id -> 404 and no DB writes.
• DELETE /subcategory/ for a non-existent id -> 404 idempotency semantics (or 200 with no-op, depending on API).

I can draft these with minimal duplication using helper methods.

Also applies to: 112-115, 222-233, 234-244

---

16-29: Micro-optimization: pass headers explicitly in multi-create tests.

Small perf win: generating tokens only once per test. Suggestions already shown above.

Also applies to: 75-87

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

tests/test_subcategory.py (7)

10-15: Setup fixture looks good.

Autouse setup ensures a clean DB state at test start; simple and effective.

---

45-53: Creation happy-path covered well.

Status code, shape, and DB verification all look good.

---

64-74: Get-by-id path is precise and asserts identity.

Covers status, id, and name; nice.

---

104-115: Delete path assertions are solid.

Status, subsequent 404, and DB check are all covered.

---

173-186: Missing-token path is correct and asserts no unintended writes.

Looks good.

---

222-233: Related categories endpoint: good baseline check for empty relations.

Asserts both envelope and empty list.

---

234-244: Related products endpoint: likewise solid.

Mirrors categories; good.

tests/test_category.py (3)

111-118: Parametrization looks good; add readable IDs and one extra DB-safety assert.

• IDs make the three cases show up clearly in pytest output (and -k selection).
• The extra count assert guards against accidental creation under a different name.

Apply this diff:

 @pytest.mark.parametrize(
     "get_headers, expected_code",
     [
         (lambda self: utils.get_expired_token_headers(self.client.application.app_context()), "token_expired"),
         (lambda self: utils.get_invalid_token_headers(), "invalid_token"),
         (lambda self: None, "authorization_required")
-        ]
-    )
+        ],
+        ids=["expired_token", "invalid_token", "authorization_required"],
+)
@@
         )
         utils.verify_token_error_response(response, expected_code)
         self._verify_category_in_db("CreateTokenError", should_exist=False)
+        assert self._count_categories() == 0

Optional dedup to avoid repeating the same tuples three times across tests; define a shared constant near the top of the file:

TOKEN_ERROR_CASES = [
    (lambda self: utils.get_expired_token_headers(self.client.application.app_context()), "token_expired"),
    (lambda self: utils.get_invalid_token_headers(), "invalid_token"),
    (lambda self: None, "authorization_required"),
]

Then reference it in each parametrize as @pytest.mark.parametrize("get_headers, expected_code", TOKEN_ERROR_CASES, ids=[...]).

Also applies to: 124-126

---

127-134: Reuse readable IDs for parametrization here as well.

Keeps test reports consistent and grep-able across create/update/delete token-error suites.

Apply this diff:

 @pytest.mark.parametrize(
     "get_headers, expected_code",
     [
         (lambda self: utils.get_expired_token_headers(self.client.application.app_context()), "token_expired"),
         (lambda self: utils.get_invalid_token_headers(), "invalid_token"),
         (lambda self: None, "authorization_required")
-        ]
-    )
+        ],
+        ids=["expired_token", "invalid_token", "authorization_required"],
+)

---

152-159: Mirror parametrization IDs and add a quick GET-by-id check post-failure.

• IDs improve readability.
• Verifying the resource still exists via GET complements the DB-name check.

Apply this diff:

 @pytest.mark.parametrize(
     "get_headers, expected_code",
     [
         (lambda self: utils.get_expired_token_headers(self.client.application.app_context()), "token_expired"),
         (lambda self: utils.get_invalid_token_headers(), "invalid_token"),
         (lambda self: None, "authorization_required")
-        ]
-    )
+        ],
+        ids=["expired_token", "invalid_token", "authorization_required"],
+)
@@
     )
 
     utils.verify_token_error_response(delete_resp, expected_code)
     self._verify_category_in_db("DeleteTokenError")
+    # Sanity: record should still be retrievable
+    get_resp = self.client.get(f"/category/{cat_id}")
+    assert get_resp.status_code == 200
+    assert get_resp.get_json()["name"] == "DeleteTokenError"

Also applies to: 169-171

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

• MCP integration is disabled by default for public repositories
• Jira integration is disabled by default for public repositories
• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.