MPT-22436 Use freshly created product for e2e media tests#352
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (2)
📜 Recent review details⏰ 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). (2)
📝 WalkthroughWalkthroughTwo test fixture signatures are updated in the sync and async media test files. The ChangesMedia fixture rewiring
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
jentyk
force-pushed
the
claude/stupefied-morse-be6c21
branch
from
87ba139 to
7bf4306
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| @pytest.fixture | ||
| def created_product(mpt_vendor, product_data, logo_fd): | ||
| product = mpt_vendor.catalog.products.create(product_data, file=logo_fd) | ||
|
|
||
| yield product | ||
|
|
||
| try: | ||
| mpt_vendor.catalog.products.delete(product.id) | ||
| except MPTAPIError as error: | ||
| print(f"TEARDOWN - Unable to delete product {product.id}: {error.title}") # noqa: WPS421 | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| async def async_created_product(async_mpt_vendor, product_data, logo_fd): | ||
| product = await async_mpt_vendor.catalog.products.create(product_data, file=logo_fd) | ||
|
|
||
| yield product | ||
|
|
||
| try: | ||
| await async_mpt_vendor.catalog.products.delete(product.id) | ||
| except MPTAPIError as error: | ||
| print(f"TEARDOWN - Unable to delete product {product.id}: {error.title}") # noqa: WPS421 | ||
|
|
There was a problem hiding this comment.
Check the helpers: async_create_fixture_resource_and_delete and create_fixture_resource_and_delete
There was a problem hiding this comment.
Done — both created_product and async_created_product now use the shared create_fixture_resource_and_delete / async_create_fixture_resource_and_delete context-manager helpers from tests/e2e/helper.py, which removes the hand-rolled create + try/except-delete teardown.
One side effect: those helpers call service.create(resource_data) without a file=, so the product is no longer created with the logo_fd icon. The public OpenAPI spec marks neither product nor icon as required on POST /public/v1/catalog/products, so creating without an icon is valid — and these fixtures only need a product to exist so media/etc. can be attached.
🤖 Generated by AI
There was a problem hiding this comment.
Update after running the e2e suite — I had to back this out. Creating a product through the helper drops the icon, and the live API rejects that:
400 One or more validation errors occurred.
{ "Icon": ["A value for the 'Icon' parameter or property was not provided."] }
(The OpenAPI spec doesn't mark Icon as required, which is what misled me earlier.) So created_product / async_created_product stay on the existing hand-rolled fixtures that upload logo_fd — the same ones #351 just merged — and this PR now only repoints the media service fixtures at them. All 28 product + media e2e tests pass this way.
Using the shared helper here would mean teaching it to forward a file= kwarg — happy to do that in a follow-up if you'd prefer.
🤖 Generated by AI
jentyk
force-pushed
the
claude/stupefied-morse-be6c21
branch
from
7bf4306 to
a1e2896
Compare
|
@albertsola ready for re-review 🙏 Both 🤖 Generated by AI |
jentyk
force-pushed
the
claude/stupefied-morse-be6c21
branch
from
a1e2896 to
b125076
Compare
…-22436] The media service fixtures built their service against the hardcoded product id from e2e_config (catalog.product.id), which had reached its maximum number of media items. MediaService.create then returned 400 Bad Request, failing setup for the sync and async media tests. Point the sync and async media service fixtures at the shared created_product / async_created_product fixtures so each run starts from a freshly created, empty product instead of the shared seeded one. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
jentyk
force-pushed
the
claude/stupefied-morse-be6c21
branch
from
b125076 to
dadcf25
Compare
|
2 participants
🤖 AI-generated PR — Please review carefully.
What
Fixes the e2e media tests (sync and async) that failed at setup with
400 Bad Request — Cannot create additional media as the maximum number of media items has been reached.The media service fixtures built their service against the hardcoded product id from
e2e_config(catalog.product.id, e.g.PRD-7255-3950). That product had reached its maximum number of media items, so everyMediaService.createreturned 400 and broke the media tests during fixture setup.Changes
Point the media service fixtures at the shared
created_product/async_created_productfixtures (already present in the product-levelconftest.py) instead of the hardcodedproduct_id, so each run starts from a freshly created, empty product:vendor_media_service→created_product.idasync_media_serviceandasync_vendor_media_service→async_created_product.id(pytest caches the fixture per test, so both share the same fresh product)The
created_productfixtures keep uploading the icon (file=logo_fd) — product creation requires it (see review thread), so the sharedcreate_fixture_resource_and_deletehelper can't be used here as-is.Testing
tests/e2e/catalog/product/media+tests/e2e/catalog/productproduct tests): 28 passed.ruff check,ruff format --check, andflake8(incl. wemake / flake8-aaa) pass on the changed files.Jira: https://softwareone.atlassian.net/browse/MPT-22436
🤖 Generated with Claude Code
Closes MPT-22436
Changes
created_productandasync_created_productfixtures to sharedconftest.pyto enable reuse across test modulescreate_fixture_resource_and_deleteandasync_create_fixture_resource_and_deletehelperscreated_product.idandasync_created_product.id) instead of hardcoded product ID from configurationMPTAPIErrorimports from product test files