Add subcategory tests #10
Conversation
WalkthroughAdds a shared PyTest fixture for producing authenticated Authorization headers, refactors token-error tests in category tests to parameterized cases (removes local fixture), and introduces a new comprehensive Subcategory test suite covering CRUD, token errors, and related-data endpoints. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant T as Test
participant F as create_authenticated_headers
participant A as Auth Service
participant API as App API
participant DB as Database
T->>F: request headers (email?, password?)
F->>A: POST /register (email,password)
A-->>F: 201 Created
F->>A: POST /login (email,password)
A-->>F: 200 {access_token}
F-->>T: Authorization: Bearer <token>
note right of T: Use header for CRUD & token-error tests
T->>API: POST /subcategories (auth header, payload)
API->>DB: INSERT
DB-->>API: OK
API-->>T: 201 Created
T->>API: PUT/DELETE/GET /subcategories/:id (auth header)
API->>DB: UPDATE/DELETE/SELECT
DB-->>API: OK or NotFound
API-->>T: 200/201/404 as applicable
alt token invalid/expired/missing
API-->>T: token_expired / invalid_token / authorization_required
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (13)
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() == 1Optional: 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
📜 Review details
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.
📒 Files selected for processing (3)
tests/conftest.py(1 hunks)tests/test_category.py(0 hunks)tests/test_subcategory.py(1 hunks)
💤 Files with no reviewable changes (1)
- tests/test_category.py
🧰 Additional context used
🧬 Code graph analysis (2)
tests/conftest.py (2)
tests/utils.py (1)
get_auth_header(13-14)tests/test_category.py (3)
create_authenticated_headers(17-24)_get_headers(18-22)create_category(27-35)
tests/test_subcategory.py (1)
tests/test_category.py (10)
TestCategory(7-241)test_create_category(51-58)test_update_category(92-107)test_update_category_missing_token(184-198)test_update_category_invalid_token(165-182)test_delete_category(109-119)test_delete_category_invalid_token(216-228)test_update_category_expired_token(144-163)test_create_category_invalid_token(130-137)test_delete_category_expired_token(200-214)
🔇 Additional comments (7)
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.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
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() == 0Optional 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
📜 Review details
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.
📒 Files selected for processing (3)
tests/conftest.py(2 hunks)tests/test_category.py(1 hunks)tests/test_subcategory.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/conftest.py
- tests/test_subcategory.py
🧰 Additional context used
🧬 Code graph analysis (1)
tests/test_category.py (2)
tests/utils.py (3)
get_expired_token_headers(17-22)get_invalid_token_headers(25-26)verify_token_error_response(6-10)tests/conftest.py (1)
create_authenticated_headers(44-51)
Summary by CodeRabbit