MPT-14896 E2E for catalog/price_lists #138
Conversation
WalkthroughAdds end-to-end tests, fixtures, and helpers for catalog price lists: new sync and async test modules, conftest fixtures to create/manage price lists, and test helper functions for create/delete teardown, filtering/iterate assertions, and update assertions. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
albertsola
force-pushed
the
e2e/catalog/MPT-14896-price-lists-and-price-lists-items
branch
from
f6a3967 to
74cd8ee
Compare
albertsola
requested review from
alephsur,
d3rky,
jentyk,
robcsegal,
ruben-sebrango and
svazquezco
as code owners
| @@ -0,0 +1,68 @@ | |||
| from contextlib import asynccontextmanager, contextmanager | |||
There was a problem hiding this comment.
@svazquezco I believe you were suggesting something like these.
If these is what you mean with generic tests, let me know and I will refactor the rest of the tests to use this.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/e2e/catalog/price_lists/test_sync_price_lists.py (1)
13-72: Sync price list e2e coverage looks solidThe fixture-backed tests exercise create, multiple get paths, iteration, filtering, update, delete, and error cases (404/400) in a clear way; the patterns align nicely with the shared helpers.
One optional improvement: in
test_delete_price_list, you could add a follow-upgetexpecting a 404 to assert deletion behavior explicitly, but current coverage is already useful.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
tests/e2e/catalog/price_lists/conftest.py(1 hunks)tests/e2e/catalog/price_lists/test_async_price_lists.py(1 hunks)tests/e2e/catalog/price_lists/test_sync_price_lists.py(1 hunks)tests/e2e/helper.py(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-27T00:05:54.691Z
Learnt from: robcsegal
Repo: softwareone-platform/mpt-api-python-client PR: 131
File: tests/e2e/accounts/accounts_users/test_async_accounts_users.py:0-0
Timestamp: 2025-11-27T00:05:54.691Z
Learning: In pytest-asyncio, async fixtures are automatically resolved before being passed to test functions (both sync and async). Test functions receive the yielded value from async fixtures, not coroutines, so fixture parameters should never be awaited. A sync test function can use async fixtures without any special handling.
Applied to files:
tests/e2e/helper.py
🧬 Code graph analysis (2)
tests/e2e/catalog/price_lists/test_sync_price_lists.py (4)
tests/e2e/helper.py (3)
assert_service_filter_with_iterate(42-50)assert_update_resource(53-59)create_fixture_resource_and_delete(20-28)mpt_api_client/exceptions.py (1)
MPTAPIError(20-42)tests/e2e/catalog/price_lists/conftest.py (1)
price_list_data(5-12)tests/e2e/conftest.py (2)
product_id(96-97)short_uuid(101-102)
tests/e2e/helper.py (3)
mpt_api_client/rql/query_builder.py (1)
RQLQuery(115-524)mpt_api_client/exceptions.py (1)
MPTAPIError(20-42)mpt_api_client/models/model.py (1)
id(119-121)
🪛 GitHub Actions: PR build and merge
tests/e2e/catalog/price_lists/test_sync_price_lists.py
[error] 2-2: ImportError during test collection: No module named 'e2e'
tests/e2e/catalog/price_lists/test_async_price_lists.py
[error] 2-2: ImportError during test collection: No module named 'e2e'
🪛 Ruff (0.14.6)
tests/e2e/helper.py
16-16: Unused noqa directive (unknown: WPS421)
Remove unused noqa directive
(RUF100)
28-28: Unused noqa directive (unknown: WPS421)
Remove unused noqa directive
(RUF100)
🔇 Additional comments (8)
tests/e2e/helper.py (4)
7-17: Async create/delete helper looks correctResource lifecycle is handled cleanly: create once, yield to the test, and always attempt delete in teardown while swallowing API errors so tests don’t fail on cleanup.
19-28: Sync create/delete helper mirrors async version appropriatelySync helper mirrors the async behavior and keeps teardown concerns localized; good reuse for multiple tests.
31-40: Filter+iterate assertion helpers are soundUsing
RQLQuery(id=filter_by_id)and asserting a single result with matching id gives a concise, reusable check for filter behavior in both async and sync paths.Also applies to: 42-51
53-60: Update assertion helpers provide useful coverageUpdate helpers correctly verify id stability and that the updated field reflects in
to_dict(), which is exactly what the e2e tests need.Also applies to: 62-68
tests/e2e/catalog/price_lists/conftest.py (1)
4-22: Price list fixtures are well-structuredPayload, service, and id fixtures are clean and reusable, and they match how the sync/async tests use them.
tests/e2e/catalog/price_lists/test_async_price_lists.py (2)
13-74: Async price list tests use fixtures and helpers correctlyAsync fixtures, the async contextmanager for resource lifecycle, and the helper coroutines for filter/update are wired correctly, and the tests cover the same CRUD and error scenarios as the sync suite.
test_create_price_listbeing a sync function that consumes an async fixture’s value withoutawaitis fine under pytest-asyncio, which resolves async fixtures before injection. Based on learnings, this is the intended pattern.
2-6: Import pattern is correct and follows established codebase convention—no changes needed.The
from e2e.helper importpattern is the correct import style for this codebase and is already used consistently in bothtest_async_price_lists.pyandtest_sync_price_lists.py.The repository's
pyproject.tomlexplicitly configurespythonpath = "."withtestpaths = "tests", which makes thee2eimport path valid. Bothtests/__init__.pyandtests/e2e/__init__.pyexist as package markers. When pytest runs from the repository root,e2e.helpercorrectly resolves totests/e2e/helper.py. The GitHub CI workflows (cron-main-e2e.yml, pr-build-merge.yml) run tests successfully with this import pattern, and no CI failures related to this import are evident.tests/e2e/catalog/price_lists/test_sync_price_lists.py (1)
2-6: I need to verify the import path issue mentioned in the review comment. Let me investigate the actual module structure and how other tests import from the helper module.
<function_calls>
#!/bin/bash1. Find helper.py location
echo "=== Locating helper.py ==="
find . -name "helper.py" -type f | head -202. Check if tests is a package
echo ""
echo "=== Checking for init.py files ==="
find tests -name "init.py" -type f 2>/dev/null | head -203. List structure of tests/e2e
echo ""
echo "=== Structure of tests/e2e directory ==="
ls -la tests/e2e/ 2>/dev/null || echo "Directory not found"
</function_calls><function_calls>
#!/bin/bashCheck how other e2e tests import from helper
echo "=== Checking imports of helper in e2e tests ==="
rg -n "from.*helper import" tests/e2e/ -A 2 -B 2echo ""
echo "=== Alternative: checking all imports in e2e test files ==="
rg -n "^from|^import" tests/e2e/catalog/price_lists/ | head -20
</function_calls><function_calls>
#!/bin/bashCheck pytest configuration
echo "=== Checking pytest configuration files ==="
ls -la | grep -E "(pytest|setup|pyproject|conftest)"echo ""
echo "=== Contents of pyproject.toml or pytest.ini ==="
cat pyproject.toml 2>/dev/null | grep -A 10 "[tool.pytest" || echo "No pytest config in pyproject.toml"
cat pytest.ini 2>/dev/null || echo "No pytest.ini found"
cat setup.cfg 2>/dev/null | grep -A 10 "[tool:pytest" || echo "No pytest config in setup.cfg"echo ""
echo "=== Checking conftest.py files ==="
find . -name "conftest.py" | head -10
</function_calls><function_calls>
#!/bin/bashCheck the actual test file and surrounding files
echo "=== Contents of test file directory ==="
ls -la tests/e2e/catalog/price_lists/ 2>/dev/null | head -20echo ""
echo "=== First 20 lines of the test file to see imports ==="
head -20 tests/e2e/catalog/price_lists/test_sync_price_lists.py 2>/dev/null || echo "File not found"
</function_calls><function_calls>
#!/bin/bashSearch for ALL imports of helper to understand the pattern
echo "=== All imports referencing 'helper' in tests directory ==="
rg -n "helper" tests/e2e --type py | grep -E "(^from|^import)" | head -30
</function_calls>
| try: | ||
| await service.delete(resource.id) | ||
| except MPTAPIError as error: | ||
| print(f"TEARDOWN - Unable to delete resource {resource}: {error.title}") # noqa: WPS421 |
There was a problem hiding this comment.
Remove unused/unknown noqa codes on teardown prints
Ruff reports RUF100 because WPS421 isn’t a known rule here, so these # noqa: WPS421 comments are now lint errors. Since print isn’t otherwise flagged, you can safely drop the noqa markers.
Suggested change:
- except MPTAPIError as error:
- print(f"TEARDOWN - Unable to delete resource {resource}: {error.title}") # noqa: WPS421
+ except MPTAPIError as error:
+ print(f"TEARDOWN - Unable to delete resource {resource}: {error.title}")
@@
- except MPTAPIError as error:
- print(f"TEARDOWN - Unable to delete resource {resource}: {error.title}") # noqa: WPS421
+ except MPTAPIError as error:
+ print(f"TEARDOWN - Unable to delete resource {resource}: {error.title}")Also applies to: 25-28
🧰 Tools
🪛 Ruff (0.14.6)
16-16: Unused noqa directive (unknown: WPS421)
Remove unused noqa directive
(RUF100)
🤖 Prompt for AI Agents
In tests/e2e/helper.py around lines 13-16 (and similarly lines 25-28), remove
the invalid/unknown inline noqa markers on the teardown print statements: the
prints are fine and the `# noqa: WPS421` causes RUF100 because WPS421 is not
recognized; simply delete the `# noqa: WPS421` suffixes so the print calls
remain and no lint suppression is present.
albertsola
force-pushed
the
e2e/catalog/MPT-14896-price-lists-and-price-lists-items
branch
from
74cd8ee to
b75c04e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
tests/e2e/helper.py (2)
13-16: Remove unusednoqadirective.The
# noqa: WPS421comment is flagged by Ruff as unknown, causing a lint error. Per the repository's learnings, while WPS rules are valid for wemake-python-styleguide, Ruff doesn't recognize them. Remove the noqa directive since the print statement is acceptable in teardown code.Apply this diff:
except MPTAPIError as error: - print(f"TEARDOWN - Unable to delete resource {resource}: {error.title}") # noqa: WPS421 + print(f"TEARDOWN - Unable to delete resource {resource}: {error.title}")
25-28: Remove unusednoqadirective.Same as line 16, the
# noqa: WPS421comment should be removed.Apply this diff:
except MPTAPIError as error: - print(f"TEARDOWN - Unable to delete resource {resource}: {error.title}") # noqa: WPS421 + print(f"TEARDOWN - Unable to delete resource {resource}: {error.title}")
🧹 Nitpick comments (2)
tests/e2e/catalog/price_lists/test_async_price_lists.py (2)
44-48: Consider removing unnecessary comment.The
# actcomment on line 47 doesn't add meaningful information since the function call is self-explanatory. This is a common pattern in unit tests following Arrange-Act-Assert, but in this context the comment is redundant.Apply this diff:
async def test_filter_price_lists(async_price_lists_service, async_created_price_list): await assert_async_service_filter_with_iterate( async_price_lists_service, async_created_price_list.id, ["-product"] - ) # act + )
50-57: Consider removing unnecessary comment.Similarly, the
# actcomment on line 56 is redundant.Apply this diff:
async def test_update_price_list(async_price_lists_service, async_created_price_list, short_uuid): await assert_async_update_resource( async_price_lists_service, async_created_price_list.id, "notes", f"Updated notes {short_uuid}", - ) # act + )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
tests/e2e/catalog/price_lists/conftest.py(1 hunks)tests/e2e/catalog/price_lists/test_async_price_lists.py(1 hunks)tests/e2e/catalog/price_lists/test_sync_price_lists.py(1 hunks)tests/e2e/helper.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/e2e/catalog/price_lists/conftest.py
- tests/e2e/catalog/price_lists/test_sync_price_lists.py
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-01T10:48:52.572Z
Learnt from: jentyk
Repo: softwareone-platform/mpt-api-python-client PR: 136
File: tests/e2e/notifications/subscribers/conftest.py:14-25
Timestamp: 2025-12-01T10:48:52.572Z
Learning: The repository softwareone-platform/mpt-api-python-client uses wemake-python-styleguide linter, so noqa directives for WPS rules should not be flagged as unused.
Applied to files:
tests/e2e/helper.py
📚 Learning: 2025-11-27T00:05:54.691Z
Learnt from: robcsegal
Repo: softwareone-platform/mpt-api-python-client PR: 131
File: tests/e2e/accounts/accounts_users/test_async_accounts_users.py:0-0
Timestamp: 2025-11-27T00:05:54.691Z
Learning: In pytest-asyncio, async fixtures are automatically resolved before being passed to test functions (both sync and async). Test functions receive the yielded value from async fixtures, not coroutines, so fixture parameters should never be awaited. A sync test function can use async fixtures without any special handling.
Applied to files:
tests/e2e/helper.py
🪛 GitHub Actions: PR build and merge
tests/e2e/catalog/price_lists/test_async_price_lists.py
[error] 2-2: ModuleNotFoundError: No module named 'e2e'. Import during test collection failed. Command: 'pytest -v -p no:randomly --no-cov --junitxml=e2e-report.xml tests/e2e'.
🪛 Ruff (0.14.7)
tests/e2e/helper.py
16-16: Unused noqa directive (unknown: WPS421)
Remove unused noqa directive
(RUF100)
28-28: Unused noqa directive (unknown: WPS421)
Remove unused noqa directive
(RUF100)
| from e2e.helper import ( | ||
| assert_async_service_filter_with_iterate, | ||
| assert_async_update_resource, | ||
| async_create_fixture_resource_and_delete, | ||
| ) |
There was a problem hiding this comment.
Fix import error blocking the build.
The import from e2e.helper import ... fails because the module path is incorrect. Since this file is located at tests/e2e/catalog/price_lists/test_async_price_lists.py and the helper is at tests/e2e/helper.py, use a relative import to navigate up the directory structure.
Apply this diff:
import pytest
-from e2e.helper import (
+from ...helper import (
assert_async_service_filter_with_iterate,
assert_async_update_resource,
async_create_fixture_resource_and_delete,
)📝 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.
| from e2e.helper import ( | |
| assert_async_service_filter_with_iterate, | |
| assert_async_update_resource, | |
| async_create_fixture_resource_and_delete, | |
| ) | |
| from ...helper import ( | |
| assert_async_service_filter_with_iterate, | |
| assert_async_update_resource, | |
| async_create_fixture_resource_and_delete, | |
| ) |
🧰 Tools
🪛 GitHub Actions: PR build and merge
[error] 2-2: ModuleNotFoundError: No module named 'e2e'. Import during test collection failed. Command: 'pytest -v -p no:randomly --no-cov --junitxml=e2e-report.xml tests/e2e'.
🤖 Prompt for AI Agents
In tests/e2e/catalog/price_lists/test_async_price_lists.py around lines 2-6, the
absolute import "from e2e.helper import ..." is incorrect for this package
layout; replace it with a package-relative import that climbs from
tests.e2e.catalog.price_lists up to tests.e2e and imports the same symbols, e.g.
change to "from ...helper import assert_async_service_filter_with_iterate,
assert_async_update_resource, async_create_fixture_resource_and_delete".
b75c04e to
00e1538
Compare
albertsola
force-pushed
the
e2e/catalog/MPT-14896-price-lists-and-price-lists-items
branch
from
00e1538 to
e4cb857
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
tests/e2e/helper.py (1)
13-17: Remove unusednoqadirectives.This issue was already flagged in a previous review. The
# noqa: WPS421markers on lines 16 and 28 are unknown to Ruff and should be removed.
🧹 Nitpick comments (1)
tests/e2e/catalog/price_lists/test_sync_price_lists.py (1)
24-28: Consider consolidating redundant get tests.This test is very similar to
test_get_price_list(lines 18-22). Both validate the get operation with essentially the same logic. Unlessprice_list_idandcreated_price_listfixtures represent meaningfully different test scenarios, you might consider consolidating these tests.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
tests/e2e/catalog/price_lists/conftest.py(1 hunks)tests/e2e/catalog/price_lists/test_async_price_lists.py(1 hunks)tests/e2e/catalog/price_lists/test_sync_price_lists.py(1 hunks)tests/e2e/helper.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/e2e/catalog/price_lists/conftest.py
- tests/e2e/catalog/price_lists/test_async_price_lists.py
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-12-01T10:48:52.572Z
Learnt from: jentyk
Repo: softwareone-platform/mpt-api-python-client PR: 136
File: tests/e2e/notifications/subscribers/conftest.py:14-25
Timestamp: 2025-12-01T10:48:52.572Z
Learning: The repository softwareone-platform/mpt-api-python-client uses wemake-python-styleguide linter, so noqa directives for WPS rules should not be flagged as unused.
Applied to files:
tests/e2e/helper.py
📚 Learning: 2025-11-27T00:05:54.691Z
Learnt from: robcsegal
Repo: softwareone-platform/mpt-api-python-client PR: 131
File: tests/e2e/accounts/accounts_users/test_async_accounts_users.py:0-0
Timestamp: 2025-11-27T00:05:54.691Z
Learning: In pytest-asyncio, async fixtures are automatically resolved before being passed to test functions (both sync and async). Test functions receive the yielded value from async fixtures, not coroutines, so fixture parameters should never be awaited. A sync test function can use async fixtures without any special handling.
Applied to files:
tests/e2e/helper.py
🧬 Code graph analysis (1)
tests/e2e/helper.py (2)
mpt_api_client/exceptions.py (1)
MPTAPIError(20-42)mpt_api_client/models/model.py (1)
id(119-121)
🪛 Ruff (0.14.7)
tests/e2e/helper.py
16-16: Unused noqa directive (unknown: WPS421)
Remove unused noqa directive
(RUF100)
28-28: Unused noqa directive (unknown: WPS421)
Remove unused noqa directive
(RUF100)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (12)
tests/e2e/helper.py (4)
31-40: LGTM!The async filter and iterate assertion helper is well-structured and provides clear validation of filtering behavior with optional field selection.
42-51: LGTM!The synchronous counterpart maintains consistency with the async version and correctly validates filtering behavior.
53-60: LGTM!The update assertion helper correctly validates both the resource identity and the field update.
62-68: LGTM!The async update assertion maintains consistency with the sync version and correctly validates update operations.
tests/e2e/catalog/price_lists/test_sync_price_lists.py (8)
12-16: LGTM!The creation test properly validates the product association.
18-22: LGTM!The get operation is properly validated.
30-36: LGTM!The iteration test correctly validates that the created resource can be found in the full result set.
44-48: LGTM!The update test properly validates field modification using the helper function.
50-52: LGTM!The delete test correctly validates successful deletion (no exception raised).
54-59: LGTM!The not-found error test properly validates 404 handling.
61-65: LGTM!The invalid data test properly validates 400 error handling for missing required fields.
38-42: No action needed. The select syntax["-product"]is valid RQL syntax for field exclusion. This pattern is consistently used throughout the test suite and explicitly validated in unit tests (e.g.,test_queryable_mixin_select), which confirm that minus-prefixed fields are correctly passed to the API query string as exclusions.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
tests/e2e/catalog/price_lists/test_async_price_lists.py (1)
63-74: Consider making negative assertions less dependent on full error messagesBoth not-found and invalid-data tests match on the full
"404 Not Found"/"400 One or more validation errors occurred"message, which can be brittle if wording changes. You might instead match only the status code in the regex (e.g.r"404") or capture the exception and assert on structured attributes likestatus/titleto decouple from exact phrasing.tests/e2e/helper.py (1)
7-28: Teardown helpers trade off strict cleanup for test robustnessThe async and sync
*_create_fixture_resource_and_deletehelpers centralize resource lifecycle and avoid hard failures on teardown by catchingMPTAPIErrorand logging. That’s reasonable for keeping e2e runs stable, but it also means failed deletes (including 5xx) won’t fail the test and can leave stray resources behind. If leaking resources becomes a problem, consider narrowing what you swallow (e.g., only ignore 404s) or re-raising on unexpected statuses while still logging.tests/e2e/catalog/price_lists/test_sync_price_lists.py (2)
12-52: Sync CRUD and iterate tests mirror async coverage effectivelyThe sync tests cover create/get/get-by-id/iterate/filter/update/delete using the same helpers as the async suite (
assert_service_filter_with_iterate,assert_update_resource), which keeps behavior parity between sync and async clients. Intest_iterate_price_lists, you could simplify toassert any(...)instead of storingresult, but that’s purely stylistic.
54-65: Optional: Relax or restructure negative assertions for stabilityAs in the async module, the not-found and invalid-create tests assert on the full error message text via regex. To make these tests more resilient to wording changes, consider matching only the status code (e.g.
r"404",r"400") or capturing the exception and asserting on structured fields likestatus/titleinstead of the exact combined string.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
tests/e2e/catalog/price_lists/conftest.py(1 hunks)tests/e2e/catalog/price_lists/test_async_price_lists.py(1 hunks)tests/e2e/catalog/price_lists/test_sync_price_lists.py(1 hunks)tests/e2e/helper.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/e2e/catalog/price_lists/conftest.py
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-27T00:05:54.691Z
Learnt from: robcsegal
Repo: softwareone-platform/mpt-api-python-client PR: 131
File: tests/e2e/accounts/accounts_users/test_async_accounts_users.py:0-0
Timestamp: 2025-11-27T00:05:54.691Z
Learning: In pytest-asyncio, async fixtures are automatically resolved before being passed to test functions (both sync and async). Test functions receive the yielded value from async fixtures, not coroutines, so fixture parameters should never be awaited. A sync test function can use async fixtures without any special handling.
Applied to files:
tests/e2e/helper.py
📚 Learning: 2025-12-01T10:48:52.572Z
Learnt from: jentyk
Repo: softwareone-platform/mpt-api-python-client PR: 136
File: tests/e2e/notifications/subscribers/conftest.py:14-25
Timestamp: 2025-12-01T10:48:52.572Z
Learning: The repository softwareone-platform/mpt-api-python-client uses wemake-python-styleguide linter, so noqa directives for WPS rules should not be flagged as unused.
Applied to files:
tests/e2e/helper.py
🧬 Code graph analysis (2)
tests/e2e/catalog/price_lists/test_sync_price_lists.py (5)
mpt_api_client/exceptions.py (1)
MPTAPIError(20-42)tests/e2e/helper.py (2)
assert_service_filter_with_iterate(42-50)assert_update_resource(53-59)tests/e2e/catalog/price_lists/conftest.py (3)
created_price_list(23-25)price_lists_service(18-19)price_list_id(29-30)tests/e2e/conftest.py (2)
product_id(96-97)short_uuid(101-102)tests/seed/catalog/test_product.py (1)
product(18-19)
tests/e2e/helper.py (3)
mpt_api_client/rql/query_builder.py (1)
RQLQuery(115-524)mpt_api_client/exceptions.py (1)
MPTAPIError(20-42)mpt_api_client/models/model.py (1)
id(119-121)
🪛 Ruff (0.14.7)
tests/e2e/helper.py
16-16: Unused noqa directive (unknown: WPS421)
Remove unused noqa directive
(RUF100)
28-28: Unused noqa directive (unknown: WPS421)
Remove unused noqa directive
(RUF100)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (5)
tests/e2e/catalog/price_lists/test_async_price_lists.py (3)
1-8: Import path and helper usage look correctUsing
from tests.e2e.helper import ...aligns with the repo layout and resolves the previousModuleNotFoundErrorwhile reusing the shared assertion helpers cleanly.
13-27: Async fixtures and context manager teardown pattern are appropriate
async_price_lists_serviceandasync_created_price_listcorrectly wrap the async client and useasync_create_fixture_resource_and_deletefor lifecycle management; using an async fixture in a sync test (test_create_price_list) is valid and idiomatic with pytest-asyncio. Based on learnings, async fixtures should be consumed without explicitawaitin the test signature.
32-60: Async CRUD, filter, and update coverage is solidThe async tests for get/filter/update/delete exercise the service API end-to-end, and delegating filtering and update assertions to shared helpers (
assert_async_service_filter_with_iterate,assert_async_update_resource) keeps the tests focused on behavior rather than mechanics.tests/e2e/helper.py (2)
31-50: Filter helpers correctly assert a unique match by id
assert_async_service_filter_with_iterateandassert_service_filter_with_iteratebuild anRQLQuery(id=filter_by_id), optionally applyselect, iterate, and assert exactly one result with the expected id. This nicely encapsulates the common pattern and keeps individual tests concise.
53-68: Update helpers provide clear assertions on updated fieldsBoth sync and async update helpers construct a minimal payload, call
update, and then assert on the id and the updated field viato_dict(). This is a good, reusable pattern for validating partial updates in e2e tests.
|
albertsola
deleted the
e2e/catalog/MPT-14896-price-lists-and-price-lists-items
branch
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.