MPT-22435 derive product-document e2e fixtures from a created product by jentyk · Pull Request #351 · softwareone-platform/mpt-api-python-client

jentyk · 2026-06-19T09:10:08Z

What

The catalog product-document e2e tests now create their own product per test instead of reusing the hardcoded seeded product (catalog.product.id / PRD-7255-3950).

Hoist created_product / async_created_product into tests/e2e/catalog/product/conftest.py so the documents/ subpackage can use them.
vendor_document_service / async_vendor_document_service now build off created_product.id / async_created_product.id.
Rework the get/download tests to create their own document (dropping the seeded document_id); the download assertion now checks the uploaded file name (empty.pdf).
Consolidate the duplicated async document-service fixture (removing the previously unused conftest fixture).

Why

The seeded product is shared across runs. Documents accumulated on it over time until it reached the server-side per-product maximum, after which document creation failed at fixture setup:

mpt_api_client.exceptions.MPTAPIError: 400 Bad Request -
Cannot create additional documents because the maximum number of documents has been reached.
POST /public/v1/catalog/products/PRD-7255-3950/documents

Creating a fresh product per test starts each test from zero documents, eliminating the limit failure and removing the dependency on shared seeded state. Same pattern as #339 (order asset fixtures derived from a created order).

Fixes MPT-22435.

Reviewer notes

The server preserves the uploaded filename, so the download tests assert the exact uploaded name (empty.pdf) rather than the old seeded pdf - empty.pdf.
components / fixVersions are required on the linked Jira bug; set to Extension Python SDK / v6.

Verification

Scoped e2e run against the live API: 32 passed (tests/e2e/catalog/product/documents + the two product test modules).
make check (ruff format/check, flake8, mypy, uv lock) clean.
Unit suite: 2310 passed.

🤖 Generated with Claude Code

Closes MPT-22435

Release Notes

Refactored catalog product-document e2e tests to create a fresh product (and associated documents) per test instead of reusing a hardcoded seeded product, preventing document accumulation and max-document fixture setup failures
Hoisted created_product and async_created_product fixtures to tests/e2e/catalog/product/conftest.py so document subpackage tests can reuse dynamically created products
Updated vendor_document_service and async_vendor_document_service fixtures to derive service context from the dynamically created product IDs
Reworked document get/download tests to create their own documents (removing dependence on a seeded document_id) and updated download assertions to expect empty.pdf
Consolidated async document-service fixture logic by centralizing it in the shared product conftest
Refactored sync/async document lifecycle tests to use shared e2e helpers from tests/e2e/helper.py, removing hand-rolled lifecycle/teardown and standardizing assertions
Extended shared helper fixture creation functions with an optional upload_file parameter to support file-backed document creation
Cleaned up local product test fixtures by removing redundant fixture implementations now provided by shared conftest

…PT-22435) The catalog product-document e2e tests created documents against a hardcoded seeded product (catalog.product.id / PRD-7255-3950). That shared product accumulated documents across runs until it reached the server-side per-product maximum, after which document creation failed at fixture setup with "400 ... maximum number of documents has been reached", turning the suite red. Point the document-service fixtures at a freshly created product per test: - Hoist created_product / async_created_product into the product conftest so the documents subpackage can use them. - vendor_document_service / async_vendor_document_service now build off created_product.id / async_created_product.id. - Rework the get/download tests to create their own document (dropping the seeded document_id) and assert the uploaded file name. - Consolidate the duplicated async document-service fixture. Verified: scoped e2e run green (32 passed), make check clean, unit suite 2310 passed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

github-actions · 2026-06-19T09:10:23Z

	Warnings
⚠️	This PR contains 2 commits. Please squash them into a single commit to keep the git history clean and easy to follow. Multiple commits are acceptable only in the following cases: One commit is a technical refactoring, and another introduces business logic changes. You are doing a complex multi-step refactoring (although in this case we still recommend splitting it into separate PRs).

✅ Found Jira issue key in the title: MPT-22435

Generated by 🚫 dangerJS against 3fee2a1

coderabbitai · 2026-06-19T09:10:24Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro

Run ID: 2376eddb-b371-40f4-a524-e0696eeee54a

📥 Commits

Reviewing files that changed from the base of the PR and between 2b04870 and 3fee2a1.

📒 Files selected for processing (3)

tests/e2e/catalog/product/documents/test_async_document.py
tests/e2e/catalog/product/documents/test_sync_document.py
tests/e2e/helper.py

🔗 Linked repositories identified

CodeRabbit considers these linked repositories for cross-repo context during reviews:

softwareone-platform/mpt-extension-skills (manual)

🚧 Files skipped from review as they are similar to previous changes (1)

tests/e2e/catalog/product/documents/test_async_document.py

📜 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). (1)

GitHub Check: build

🧰 Additional context used

📓 Path-based instructions (2)

**/*

⚙️ CodeRabbit configuration file

**/*: For each subsequent commit in this PR, explicitly verify if previous review comments have been resolved

Files:

tests/e2e/helper.py
tests/e2e/catalog/product/documents/test_sync_document.py

**

⚙️ CodeRabbit configuration file

**: # AGENTS.md

Working protocol for any task in this repository:

Identify the task type and select only the local repository files that are relevant to that task.

Read only those relevant local files before making changes.

If any selected local file references shared standards or shared operational guidance that are relevant to the same task, read those shared documents too before proceeding.

Treat repository-local documents as repository-specific additions, restrictions, or overrides to shared guidance.

If a repository-local rule conflicts with a shared rule, the local repository rule takes precedence.

Python API client for the SoftwareONE Marketplace Platform (MPT) API. Provides synchronous
(MPTClient) and asynchronous (AsyncMPTClient) clients built on httpx, with typed
resource services, mixin-based HTTP operations, and an RQL query builder.

Documentation Reading Order

When applicable, read the repository documentation in this order:

README.md — repository overview, quick start, and documentation map

docs/usage.md — installation, configuration, Python usage examples, and supported Docker-based commands

docs/architecture.md — layered architecture, directory structure, and key abstractions

docs/local-development.md — Docker-only setup and execution model

docs/testing.md — repository-specific testing strategy and command mapping

docs/contributing.md — repository-specific workflow and links to shared standards

docs/documentation.md — repository-specific documentation rules

docs/unit_tests.md — unit test structure and guidance

docs/e2e_tests.md — end-to-end test setup and execution

Then inspect the code paths relevant to the task:

mpt_api_client/mpt_client.py — public sync and async client entry points

mpt_api_client/http/ — HTTP clients, services, query state, and reusable mixins

mpt_api_client/resources/ — domain resource groups such as catalog, commerce, billing, and integrati...

Files:

tests/e2e/helper.py
tests/e2e/catalog/product/documents/test_sync_document.py

🧠 Learnings (7)

📚 Learning: 2025-12-12T15:02:20.732Z

Learnt from: robcsegal
Repo: softwareone-platform/mpt-api-python-client PR: 160
File: tests/e2e/commerce/agreement/attachment/test_async_agreement_attachment.py:55-58
Timestamp: 2025-12-12T15:02:20.732Z
Learning: In pytest with pytest-asyncio, if a test function uses async fixtures but contains no await, declare the test function as def (synchronous) instead of async def. Pytest-asyncio will resolve the async fixtures automatically; this avoids linter complaints about unnecessary async functions. This pattern applies to any test file under the tests/ directory that uses such fixtures.

Applied to files:

tests/e2e/helper.py
tests/e2e/catalog/product/documents/test_sync_document.py

📚 Learning: 2026-04-02T09:35:03.825Z

Learnt from: jentyk
Repo: softwareone-platform/mpt-api-python-client PR: 269
File: tests/e2e/helpdesk/chats/links/test_sync_links.py:18-18
Timestamp: 2026-04-02T09:35:03.825Z
Learning: In this repository’s test suite, flake8-aaa/flake8-aaa codes use short two-digit suffixes (e.g., `# noqa: AAA01`), not three-digit variants like `AAA001`. If you see `# noqa: AAA01` in a test file (e.g., when the Act step is performed via a pytest fixture rather than inline code), treat it as valid and intentional—do not flag it as an invalid/no-longer-needed noqa, and do not suggest removing it even if Ruff reports `RUF102`, since `AAA` is configured under `tool.ruff.lint.external` and these noqa directives are expected to be preserved.

Applied to files:

tests/e2e/helper.py
tests/e2e/catalog/product/documents/test_sync_document.py

📚 Learning: 2026-01-08T08:34:05.465Z

Learnt from: albertsola
Repo: softwareone-platform/mpt-api-python-client PR: 183
File: tests/e2e/catalog/price_lists/conftest.py:29-30
Timestamp: 2026-01-08T08:34:05.465Z
Learning: In end-to-end tests (e.g., tests/e2e/...), reuse existing API resources for read-only operations and safe mutations to speed up test execution. Reserve isolated fixtures (e.g., created_price_list) for destructive tests that require per-test creation and cleanup. Ensure tests document when a fixture creates/destroys data and clearly indicate which operations are destructive, so tests stay fast and properly isolated.

Applied to files:

tests/e2e/helper.py
tests/e2e/catalog/product/documents/test_sync_document.py

📚 Learning: 2026-01-08T23:38:19.565Z

Learnt from: robcsegal
Repo: softwareone-platform/mpt-api-python-client PR: 186
File: tests/e2e/billing/ledger/charge/test_sync_ledger_charge.py:33-39
Timestamp: 2026-01-08T23:38:19.565Z
Learning: In Python e2e tests under tests/e2e, hardcoded external IDs (e.g., INV12345) are intentional because they come from seeded test data. Ensure the test data seeds consistently include these IDs; if seeds change, update tests accordingly. Prefer using a named constant for such IDs (e.g., INV_EXTERNAL_ID) and document the dependency on seed data to avoid brittle tests.

Applied to files:

tests/e2e/helper.py
tests/e2e/catalog/product/documents/test_sync_document.py

📚 Learning: 2026-02-02T13:05:41.144Z

Learnt from: albertsola
Repo: softwareone-platform/mpt-api-python-client PR: 201
File: tests/unit/resources/accounts/mixins/test_activatable_mixin.py:132-139
Timestamp: 2026-02-02T13:05:41.144Z
Learning: In the mpt-api-python-client repository, tests are configured to use pytest asyncio mode auto, which auto-detects async test functions and runs them without requiring pytest.mark.asyncio. Reviewers should rely on this behavior for all Python test files under tests/, and avoid adding unnecessary asyncio markers in async tests. Ensure test files in tests/ adhere to this convention unless a specific test requires an explicit marker.

Applied to files:

tests/e2e/helper.py
tests/e2e/catalog/product/documents/test_sync_document.py

📚 Learning: 2026-04-16T13:00:41.320Z

Learnt from: jentyk
Repo: softwareone-platform/mpt-api-python-client PR: 303
File: tests/e2e/helpdesk/chats/participants/conftest.py:25-31
Timestamp: 2026-04-16T13:00:41.320Z
Learning: In mpt-api-python-client, do not treat list-wrapped arguments to CreateMixin.create() / AsyncCreateMixin.create() as an error. ResourceData is intentionally typed as Resource | list[Resource] (see mpt_api_client/models/model.py), so create() should accept either a single resource dict or a list of resource dicts (e.g., create([chat_participant_data])) to perform a batch create and return a ModelCollection. Therefore, reviewers should only flag list-wrapped create() arguments when there is evidence they violate the expected API contract beyond this documented batch-create behavior.

Applied to files:

tests/e2e/helper.py
tests/e2e/catalog/product/documents/test_sync_document.py

📚 Learning: 2026-01-08T08:34:19.306Z

Learnt from: albertsola
Repo: softwareone-platform/mpt-api-python-client PR: 183
File: tests/e2e/catalog/price_lists/items/conftest.py:26-27
Timestamp: 2026-01-08T08:34:19.306Z
Learning: In tests/e2e/catalog/price_lists/items/conftest.py, the price_list_item fixture is seeded to always provide an item. Do not add StopIteration handling in this test fixture since absence of items is not expected; treat StopIteration as an upstream test setup failure rather than a recoverable condition.

Applied to files:

tests/e2e/catalog/product/documents/test_sync_document.py

🔇 Additional comments (5)

tests/e2e/helper.py (1)

36-45: LGTM!

Also applies to: 49-58

tests/e2e/catalog/product/documents/test_sync_document.py (4)

13-19: LGTM!

22-26: LGTM!

39-45: LGTM!

69-72: LGTM!

📝 Walkthrough

Walkthrough

created_product and async_created_product fixtures are extracted from individual product test files into a shared tests/e2e/catalog/product/conftest.py. Test helpers in helper.py are enhanced to accept optional file uploads. The document conftest replaces document_id with document_data and wires service fixtures to use created-product ids. All sync and async document tests are updated to use the shared service fixtures and created_document_from_file instead of standalone ids, with download filename assertions corrected to "empty.pdf".

Changes

Product fixture centralization and helper enhancement

Layer / File(s)	Summary
Shared product fixtures and enhanced helper utilities `tests/e2e/catalog/product/conftest.py`, `tests/e2e/catalog/product/test_sync_product.py`, `tests/e2e/catalog/product/test_async_product.py`, `tests/e2e/helper.py`	`created_product` and `async_created_product` fixtures are added to the shared conftest with create/yield/delete teardown and `MPTAPIError` handling. Duplicate local fixture definitions are removed from both product test files. `async_create_fixture_resource_and_delete` and `create_fixture_resource_and_delete` helper functions are enhanced to accept an optional `upload_file` parameter and conditionally pass it to `service.create(..., file=upload_file)`.
Document conftest: wire to created product fixtures `tests/e2e/catalog/product/documents/conftest.py`	`document_id` fixture is replaced by `document_data` returning a full payload dictionary. `vendor_document_service` and `async_vendor_document_service` fixtures now accept `created_product` and `async_created_product` parameters and derive product id from those fixtures using `.id` property.
Sync document tests refactored to use shared fixtures `tests/e2e/catalog/product/documents/test_sync_document.py`	Document creation fixtures now use `create_fixture_resource_and_delete` helper. `test_update_document` delegates to `assert_update_resource` helper. `test_get_document` and `test_download_document` use `created_document_from_file.id` instead of `document_id`; expected filename updated to `"empty.pdf"`. `test_filter_documents` uses `assert_service_filter_with_iterate` helper. `test_review_and_publish_document` calls review via `vendor_document_service` and derives `ops_service` from `created_product.id`.
Async document tests refactored to use shared fixtures `tests/e2e/catalog/product/documents/test_async_document.py`	Document creation fixtures now use `async_create_fixture_resource_and_delete` helper. All async document CRUD tests rewired from `async_document_service` to `async_vendor_document_service`, using `created_document_from_file_async.id` for get/download operations; expected filename updated to `"empty.pdf"`. `test_filter_documents_async` uses `assert_async_service_filter_with_iterate` helper. `test_review_and_publish_document_async` calls review via `async_vendor_document_service` and derives `ops_service` from `async_created_product.id`. Not-found and delete tests updated to use `async_vendor_document_service`.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

softwareone-platform/mpt-api-python-client#126: Aligns catalog product/document test fixtures and shared helpers to the new file-upload create(..., file=...)/multipart semantics introduced by icon-to-file mixin changes.
softwareone-platform/mpt-api-python-client#138: Introduces create_fixture_resource_and_delete and async_create_fixture_resource_and_delete helpers in tests/e2e/helper.py, which this PR extends to support optional upload_file parameter.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Documentation Up To Date	✅ Passed	This PR is a test-only refactoring that changes e2e test fixture strategy from shared seeded products to fresh per-test products, fixing server-side document limits. While it modifies test strategy...

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

albertsola

We have a set of helpers to simplify all that code

albertsola · 2026-06-19T09:41:40Z

@@ -43,37 +38,47 @@ def test_create_from_link_async(created_document_from_link_async, pdf_url, docum
    assert created_document_from_link_async.description == document_data["description"]



we have helpers that does this for you:

check async_create_fixture_resource_and_delete and create_fixture_resource_and_delete

Done in 3fee2a1 — both fixtures (sync + async) now use create_fixture_resource_and_delete / async_create_fixture_resource_and_delete. I extended those helpers with an optional upload_file param so the file-backed document fixture can use them too (backward compatible: it is only forwarded to create() when provided, so existing non-file callers are unchanged). 🤖 Generated by AI

albertsola · 2026-06-19T09:42:49Z

+async def test_iterate_documents_async(
+    async_vendor_document_service, created_document_from_file_async
+):
+    documents = [doc async for doc in async_vendor_document_service.iterate()]

    result = any(doc.id == created_document_from_file_async.id for doc in documents)

    assert result is True


check helper: assert_service_filter_with_iterate and assert_async_service_filter_with_iterate

Done in 3fee2a1 — test_filter_documents / test_filter_documents_async now use assert_service_filter_with_iterate / assert_async_service_filter_with_iterate. 🤖 Generated by AI

albertsola · 2026-06-19T09:43:26Z

+async def test_update_document_async(
+    async_vendor_document_service, created_document_from_file_async
+):
    update_data = {"name": "Updated e2e test document - please delete"}

-    result = await async_document_service.update(created_document_from_file_async.id, update_data)
+    result = await async_vendor_document_service.update(
+        created_document_from_file_async.id, update_data
+    )

    assert result.name == update_data["name"]


check helper: assert_update_resource and assert_async_update_resource

Done in 3fee2a1 — test_update_document / test_update_document_async now use assert_update_resource / assert_async_update_resource. 🤖 Generated by AI

Address review feedback on #351: replace hand-rolled fixture/teardown and assertion boilerplate with the shared tests/e2e/helper.py helpers. - created_document_from_file/_url (sync + async) now use create_fixture_resource_and_delete / async variant. - Extend those helpers with an optional upload_file so file-backed document creation can use them (backward compatible: forwarded only when provided). - test_update_document(_async) -> assert_update_resource / async variant. - test_filter_documents(_async) -> assert_service_filter_with_iterate / async. Verified: scoped e2e green (20 passed), make check clean, unit suite 2310 passed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

sonarqubecloud · 2026-06-19T10:02:27Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

jentyk requested a review from a team as a code owner June 19, 2026 09:10

jentyk requested review from d3rky and svazquezco June 19, 2026 09:10

coderabbitai Bot approved these changes Jun 19, 2026

View reviewed changes

albertsola approved these changes Jun 19, 2026

View reviewed changes

jentyk requested a review from albertsola June 19, 2026 10:01

jentyk merged commit a1bcd9d into main Jun 19, 2026
6 checks passed

jentyk deleted the claude/nifty-sammet-44b0e2 branch June 19, 2026 10:09

coderabbitai Bot mentioned this pull request Jun 19, 2026

MPT-22436 Use freshly created product for e2e media tests #352

Merged

Navigation Menu

MPT-22435 derive product-document e2e fixtures from a created product - #351

MPT-22435 derive product-document e2e fixtures from a created product#351
jentyk merged 2 commits into
mainfrom
claude/nifty-sammet-44b0e2

jentyk commented Jun 19, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

github-actions Bot commented Jun 19, 2026 •

edited by atlassian Bot

Loading

Uh oh!

coderabbitai Bot commented Jun 19, 2026 •

edited

Loading

Documentation Reading Order

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Uh oh!

albertsola left a comment

Uh oh!

albertsola Jun 19, 2026

Uh oh!

jentyk Jun 19, 2026

Uh oh!

albertsola Jun 19, 2026

Uh oh!

jentyk Jun 19, 2026

Uh oh!

albertsola Jun 19, 2026

Uh oh!

jentyk Jun 19, 2026

Uh oh!

sonarqubecloud Bot commented Jun 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -43,37 +38,47 @@ def test_create_from_link_async(created_document_from_link_async, pdf_url, docum
		assert created_document_from_link_async.description == document_data["description"]

Conversation

jentyk commented Jun 19, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Why

Reviewer notes

Verification

Release Notes

Uh oh!

github-actions Bot commented Jun 19, 2026 • edited by atlassian Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Jun 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Documentation Reading Order

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Uh oh!

albertsola left a comment

Choose a reason for hiding this comment

Uh oh!

albertsola Jun 19, 2026

Choose a reason for hiding this comment

Uh oh!

jentyk Jun 19, 2026

Choose a reason for hiding this comment

Uh oh!

albertsola Jun 19, 2026

Choose a reason for hiding this comment

Uh oh!

jentyk Jun 19, 2026

Choose a reason for hiding this comment

Uh oh!

albertsola Jun 19, 2026

Choose a reason for hiding this comment

Uh oh!

jentyk Jun 19, 2026

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud Bot commented Jun 19, 2026

Quality Gate passed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jentyk commented Jun 19, 2026 •

edited by coderabbitai Bot

Loading

github-actions Bot commented Jun 19, 2026 •

edited by atlassian Bot

Loading

coderabbitai Bot commented Jun 19, 2026 •

edited

Loading