Refactored test for get policy service #2189
Conversation
|
Refactored the test for get_policy_service with below test cases:
Created |
ZeelSatasiya
changed the title
There was a problem hiding this comment.
Thanks for all of this @ZeelSatasiya. Just had a few nitpicks and a few questions. Also: I'd actually rename your testing file to "tests/unit/services/test_policy_service.py", like you had, as we want every test file to be "test_" + filename.
tests/fixtures/policy_fixtures.py
Outdated
|
|
||
|
|
||
| # Sample policy data | ||
| sample_policy_data = { |
There was a problem hiding this comment.
nit: Rename this to something like "valid_policy_data" (assuming it is)
tests/fixtures/policy_fixtures.py
Outdated
| "label": None, | ||
| "api_version": "1.180.1", | ||
| "policy_json": json.dumps( | ||
| {"gov.irs.income.bracket.rates.2": {"2024-01-01.2024-12-31": 0.2433}} |
There was a problem hiding this comment.
nit: Define this policy object separately
Doing so is a bit cleaner, and when we move toward a better schematization standard, it will be a bit easier to manage that way
tests/fixtures/policy_fixtures.py
Outdated
| "policy_json": json.dumps( | ||
| {"gov.irs.income.bracket.rates.2": {"2024-01-01.2024-12-31": 0.2433}} | ||
| ), | ||
| "policy_hash": "NgJhpeuRVnIAwgYWuJsd2fI/N88rIE6Kcj8q4TPD/i4=", |
There was a problem hiding this comment.
| "policy_hash": "NgJhpeuRVnIAwgYWuJsd2fI/N88rIE6Kcj8q4TPD/i4=", | |
| "policy_hash": valid_hash_value, |
nit: This helps keep the data you're testing cleaner
tests/fixtures/policy_fixtures.py
Outdated
| @pytest.fixture | ||
| def mock_database(): | ||
| """Mock the database module.""" | ||
| with patch("policyengine_api.services.policy_service.database") as mock_db: | ||
| yield mock_db |
There was a problem hiding this comment.
| @pytest.fixture | |
| def mock_database(): | |
| """Mock the database module.""" | |
| with patch("policyengine_api.services.policy_service.database") as mock_db: | |
| yield mock_db |
blocking: Remove this and use the test_db fixture in its place
|
|
||
|
|
||
| @pytest.fixture | ||
| def existing_policy_record(test_db): |
There was a problem hiding this comment.
accolade: Nice setup of this fixture
| valid_policy_json = json.loads(existing_policy_record["policy_json"]) | ||
|
|
||
| # THEN the result should contain the expected policy data | ||
| assert result["policy_json"] == valid_policy_json |
There was a problem hiding this comment.
suggestion: This is only testing the JSON of the policy, but nothing else; compare the entire return output (whether that be a formatted dict, just the JSON, just an ID) to the part you're fetching out of the table
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2189 +/- ##
==========================================
- Coverage 75.48% 74.84% -0.65%
==========================================
Files 78 81 +3
Lines 2921 3045 +124
Branches 309 328 +19
==========================================
+ Hits 2205 2279 +74
- Misses 651 701 +50
Partials 65 65 ☔ View full report in Codecov by Sentry. |
No description provided.