MPT-21557 handle orphaned parameter during parameter creation in sync and async tests#337
Merged
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 (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📜 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)
📝 WalkthroughWalkthroughBoth ChangesParameter Fixture Resilience
🎯 2 (Simple) | ⏱️ ~8 minutes Caution Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional.
❌ Failed checks (2 errors)
✅ Passed checks (2 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
tests/e2e/program/program/parameter/test_async_parameter.py (1)
17-17: 💤 Low valueConsider extracting the external ID to a module-level constant.
The external ID
"e2eCreatedProgramParameter"appears in both the error message check (line 17) and the RQLQuery filter (line 22). Extracting it to a constant would improve maintainability if this value needs to change.♻️ Suggested refactor
+PARAMETER_EXTERNAL_ID = "e2eCreatedProgramParameter" + + `@pytest.fixture` async def created_parameter(async_mpt_vendor, program_id, parameter_data): service = async_mpt_vendor.program.programs.parameters(program_id) try: parameter = await service.create(parameter_data) except MPTAPIError as error: if ( error.status_code == 400 - and "Parameter with given external identifier 'e2eCreatedProgramParameter' already " + and f"Parameter with given external identifier '{PARAMETER_EXTERNAL_ID}' already " "exists for given program." in error.detail ): parameter = await service.filter( - RQLQuery(externalId="e2eCreatedProgramParameter") + RQLQuery(externalId=PARAMETER_EXTERNAL_ID) ).fetch_one()Also applies to: 22-22
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/e2e/program/program/parameter/test_async_parameter.py` at line 17, Extract the external identifier "e2eCreatedProgramParameter" into a module-level constant (e.g., EXTERNAL_ID) and replace the literal occurrences used in the error message assertion and the RQLQuery filter with that constant; update the test in tests/e2e/program/program/parameter/test_async_parameter.py so the assertion that checks for "Parameter with given external identifier 'e2eCreatedProgramParameter' already " and the RQLQuery(...) filter both reference EXTERNAL_ID instead of the hard-coded string.tests/e2e/program/program/parameter/test_sync_parameter.py (2)
17-17: 💤 Low valueConsider extracting the external ID to a module-level constant.
The external ID
"e2eCreatedProgramParameter"is duplicated in the error message check (line 17) and the RQLQuery filter (line 22). Extracting it to a constant would improve maintainability.♻️ Suggested refactor
+PARAMETER_EXTERNAL_ID = "e2eCreatedProgramParameter" + + `@pytest.fixture` def created_parameter(mpt_vendor, program_id, parameter_data): service = mpt_vendor.program.programs.parameters(program_id) try: parameter = service.create(parameter_data) except MPTAPIError as error: if ( error.status_code == 400 - and "Parameter with given external identifier 'e2eCreatedProgramParameter' already " + and f"Parameter with given external identifier '{PARAMETER_EXTERNAL_ID}' already " "exists for given program." in error.detail ): parameter = service.filter( - RQLQuery(externalId="e2eCreatedProgramParameter") + RQLQuery(externalId=PARAMETER_EXTERNAL_ID) ).fetch_one()Also applies to: 22-22
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/e2e/program/program/parameter/test_sync_parameter.py` at line 17, Extract the duplicated external ID string "e2eCreatedProgramParameter" to a module-level constant (e.g., E2E_CREATED_PROGRAM_PARAMETER_EXTERNAL_ID) and replace the two occurrences: the error message assertion that contains "Parameter with given external identifier 'e2eCreatedProgramParameter' already " and the RQLQuery filter that uses the same literal; update any imports or references in the test_sync_parameter.py module so both places reference the new constant.
12-25: ⚖️ Poor tradeoffConsider extracting duplicate error handling to a shared conftest helper.
The error recovery logic (lines 12-25) is duplicated between
test_async_parameter.pyandtest_sync_parameter.py. If this pattern is needed elsewhere or if the logic changes, extracting it to a shared helper inconftest.pywould reduce maintenance burden.💡 Potential approach
Create sync and async helper functions in
tests/e2e/program/program/parameter/conftest.py:PARAMETER_EXTERNAL_ID = "e2eCreatedProgramParameter" def _handle_duplicate_parameter_error(error: MPTAPIError, service): """Fetch existing parameter if creation fails due to duplicate external ID.""" if ( error.status_code == 400 and f"Parameter with given external identifier '{PARAMETER_EXTERNAL_ID}' already " "exists for given program." in error.detail ): return service.filter(RQLQuery(externalId=PARAMETER_EXTERNAL_ID)).fetch_one() raise async def _handle_duplicate_parameter_error_async(error: MPTAPIError, service): """Async version of _handle_duplicate_parameter_error.""" if ( error.status_code == 400 and f"Parameter with given external identifier '{PARAMETER_EXTERNAL_ID}' already " "exists for given program." in error.detail ): return await service.filter(RQLQuery(externalId=PARAMETER_EXTERNAL_ID)).fetch_one() raiseThen simplify the fixtures:
try: parameter = service.create(parameter_data) except MPTAPIError as error: parameter = _handle_duplicate_parameter_error(error, service)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/e2e/program/program/parameter/test_sync_parameter.py` around lines 12 - 25, Extract the duplicated duplicate-parameter recovery logic into shared helpers in tests/e2e/program/program/parameter/conftest.py: add a constant PARAMETER_EXTERNAL_ID and two helpers _handle_duplicate_parameter_error(error, service) and async _handle_duplicate_parameter_error_async(error, service) which check MPTAPIError.status_code and error.detail, and on match return service.filter(RQLQuery(externalId=PARAMETER_EXTERNAL_ID)).fetch_one() (or await ...fetch_one() for the async variant); then replace the try/except in test_sync_parameter.py (the service.create call and its except MPTAPIError block) with a call to the new helper to fetch the existing parameter on duplicate instead of duplicating the logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/e2e/program/program/parameter/test_async_parameter.py`:
- Line 17: Extract the external identifier "e2eCreatedProgramParameter" into a
module-level constant (e.g., EXTERNAL_ID) and replace the literal occurrences
used in the error message assertion and the RQLQuery filter with that constant;
update the test in tests/e2e/program/program/parameter/test_async_parameter.py
so the assertion that checks for "Parameter with given external identifier
'e2eCreatedProgramParameter' already " and the RQLQuery(...) filter both
reference EXTERNAL_ID instead of the hard-coded string.
In `@tests/e2e/program/program/parameter/test_sync_parameter.py`:
- Line 17: Extract the duplicated external ID string
"e2eCreatedProgramParameter" to a module-level constant (e.g.,
E2E_CREATED_PROGRAM_PARAMETER_EXTERNAL_ID) and replace the two occurrences: the
error message assertion that contains "Parameter with given external identifier
'e2eCreatedProgramParameter' already " and the RQLQuery filter that uses the
same literal; update any imports or references in the test_sync_parameter.py
module so both places reference the new constant.
- Around line 12-25: Extract the duplicated duplicate-parameter recovery logic
into shared helpers in tests/e2e/program/program/parameter/conftest.py: add a
constant PARAMETER_EXTERNAL_ID and two helpers
_handle_duplicate_parameter_error(error, service) and async
_handle_duplicate_parameter_error_async(error, service) which check
MPTAPIError.status_code and error.detail, and on match return
service.filter(RQLQuery(externalId=PARAMETER_EXTERNAL_ID)).fetch_one() (or await
...fetch_one() for the async variant); then replace the try/except in
test_sync_parameter.py (the service.create call and its except MPTAPIError
block) with a call to the new helper to fetch the existing parameter on
duplicate instead of duplicating the logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 743716be-eba6-4a6f-919d-4743df36fbbc
📒 Files selected for processing (2)
tests/e2e/program/program/parameter/test_async_parameter.pytests/e2e/program/program/parameter/test_sync_parameter.py
📜 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/program/program/parameter/test_sync_parameter.pytests/e2e/program/program/parameter/test_async_parameter.py
**
⚙️ CodeRabbit configuration file
**: # AGENTS.mdWorking 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 mapdocs/usage.md— installation, configuration, Python usage examples, and supported Docker-based commandsdocs/architecture.md— layered architecture, directory structure, and key abstractionsdocs/local-development.md— Docker-only setup and execution modeldocs/testing.md— repository-specific testing strategy and command mappingdocs/contributing.md— repository-specific workflow and links to shared standardsdocs/documentation.md— repository-specific documentation rulesThen inspect the code paths relevant to the task:
mpt_api_client/mpt_client.py— public sync and async client entry pointsmpt_api_client/http/— HTTP clients, services, query state, and reusable mixinsmpt_api_client/resources/— domain resource groups such as catalog, commerce, billing, and integrationmpt_api_client/models/— response model layer and collection wrappersmpt_api_client/rql/— fluent RQL query ...
Files:
tests/e2e/program/program/parameter/test_sync_parameter.pytests/e2e/program/program/parameter/test_async_parameter.py
🧠 Learnings (11)
📓 Common learnings
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:13.551Z
Learning: In the mpt-api-python-client E2E tests, the team intentionally reuses existing API resources (via e2e_config) for read-only operations and safe mutations (e.g., updating the "notes" field) to improve test execution speed. Only destructive/dangerous tests use isolated fixtures (created_price_list or async_created_price_list) that are created and cleaned up per test.
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.701Z
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.
Learnt from: CR
Repo: softwareone-platform/mpt-api-python-client PR: 0
File: docs/unit_tests.md:0-0
Timestamp: 2026-06-02T14:06:45.662Z
Learning: Applies to docs/tests/unit/conftest.py : Shared test fixtures (http_client, async_http_client, DummyModel) should be defined in `tests/unit/conftest.py`
Learnt from: albertsola
Repo: softwareone-platform/mpt-api-python-client PR: 277
File: tests/e2e/extensibility/extensions/conftest.py:37-40
Timestamp: 2026-04-06T09:03:39.708Z
Learning: In the softwareone-platform/mpt-api-python-client repository, E2E conftest teardown fixtures intentionally catch all `MPTAPIError` exceptions (not just 404) and print a diagnostic message instead of re-raising. This is by design: in some scenarios the resource cannot be deleted, and the broad catch keeps fixtures reusable without failing the test suite during teardown. Do not suggest narrowing the except clause (e.g., checking `error.status == 404`) in E2E conftest teardown blocks.
📚 Learning: 2026-04-06T09:03:39.708Z
Learnt from: albertsola
Repo: softwareone-platform/mpt-api-python-client PR: 277
File: tests/e2e/extensibility/extensions/conftest.py:37-40
Timestamp: 2026-04-06T09:03:39.708Z
Learning: In the softwareone-platform/mpt-api-python-client repository, E2E conftest teardown fixtures intentionally catch all `MPTAPIError` exceptions (not just 404) and print a diagnostic message instead of re-raising. This is by design: in some scenarios the resource cannot be deleted, and the broad catch keeps fixtures reusable without failing the test suite during teardown. Do not suggest narrowing the except clause (e.g., checking `error.status == 404`) in E2E conftest teardown blocks.
Applied to files:
tests/e2e/program/program/parameter/test_sync_parameter.pytests/e2e/program/program/parameter/test_async_parameter.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/program/program/parameter/test_sync_parameter.pytests/e2e/program/program/parameter/test_async_parameter.py
📚 Learning: 2026-04-16T17:40:16.671Z
Learnt from: robcsegal
Repo: softwareone-platform/mpt-api-python-client PR: 305
File: tests/e2e/program/program/media/test_async_media.py:26-33
Timestamp: 2026-04-16T17:40:16.671Z
Learning: In the mpt-api-python-client repository, when creating program media of type "Video" via URL (e.g., a YouTube URL), the API still requires a file upload (e.g., a thumbnail/logo file). The `created_media_from_url` fixture correctly passes both `media_data["url"]` and `file=logo_fd` to `.create()`. Do not flag the `file` argument as unnecessary in URL-based media creation fixtures under `tests/e2e/program/program/media/`.
Applied to files:
tests/e2e/program/program/parameter/test_sync_parameter.py
📚 Learning: 2026-04-16T11:49:56.583Z
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-16T11:49:56.583Z
Learning: In the mpt-api-python-client repo, the helpdesk chat participants E2E fixtures (`created_chat_participant`, `async_created_chat_participant` in `tests/e2e/helpdesk/chats/participants/conftest.py`) do not need teardown. The parent `created_chat` fixture handles cascade-deletion of the chat and all its relations (including participants) during teardown. Do not flag missing participant cleanup as an issue.
Applied to files:
tests/e2e/program/program/parameter/test_sync_parameter.pytests/e2e/program/program/parameter/test_async_parameter.py
📚 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/program/program/parameter/test_sync_parameter.pytests/e2e/program/program/parameter/test_async_parameter.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/program/program/parameter/test_sync_parameter.pytests/e2e/program/program/parameter/test_async_parameter.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/program/program/parameter/test_sync_parameter.pytests/e2e/program/program/parameter/test_async_parameter.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/program/program/parameter/test_sync_parameter.pytests/e2e/program/program/parameter/test_async_parameter.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/program/program/parameter/test_sync_parameter.pytests/e2e/program/program/parameter/test_async_parameter.py
📚 Learning: 2025-11-27T00:05:54.701Z
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.701Z
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/program/program/parameter/test_async_parameter.py
🔇 Additional comments (2)
tests/e2e/program/program/parameter/test_async_parameter.py (1)
12-25: LGTM!tests/e2e/program/program/parameter/test_sync_parameter.py (1)
12-25: LGTM!
robcsegal
approved these changes
Jun 8, 2026
Member
Author
|
@coderabbitai review |
jentyk
force-pushed
the
bugfix/MPT-21557
branch
2 times, most recently
from
9f87e17 to
61c73c8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/e2e/program/program/parameter/test_async_parameter.py`:
- Around line 13-24: The existing except block catches MPTAPIError for the case
where create() fails due to duplicate externalId but then calls
service.filter(...).fetch_one() which actually raises ValueError for 0 or >1
results; change the error handling so you inspect the create() exception safely
(guarding error.detail for None before doing "in"), and when falling back call
fetch_one() inside a try/except that catches ValueError (or let it bubble with
an augmented message including EXTERNAL_ID/RQLQuery), i.e. keep the check for
MPTAPIError on service.create and its detail, but wrap
service.filter(RQLQuery(externalId=EXTERNAL_ID)).fetch_one() in a try/except
ValueError to handle not-found/multiple-match cases (or re-raise a clearer error
mentioning EXTERNAL_ID).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: e08f5add-703a-4922-a371-eb144ab5020d
📒 Files selected for processing (3)
tests/e2e/program/program/parameter/conftest.pytests/e2e/program/program/parameter/test_async_parameter.pytests/e2e/program/program/parameter/test_sync_parameter.py
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/e2e/program/program/parameter/test_sync_parameter.py
📜 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/program/program/parameter/conftest.pytests/e2e/program/program/parameter/test_async_parameter.py
**
⚙️ CodeRabbit configuration file
**: # AGENTS.mdWorking 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 mapdocs/usage.md— installation, configuration, Python usage examples, and supported Docker-based commandsdocs/architecture.md— layered architecture, directory structure, and key abstractionsdocs/local-development.md— Docker-only setup and execution modeldocs/testing.md— repository-specific testing strategy and command mappingdocs/contributing.md— repository-specific workflow and links to shared standardsdocs/documentation.md— repository-specific documentation rulesThen inspect the code paths relevant to the task:
mpt_api_client/mpt_client.py— public sync and async client entry pointsmpt_api_client/http/— HTTP clients, services, query state, and reusable mixinsmpt_api_client/resources/— domain resource groups such as catalog, commerce, billing, and integrationmpt_api_client/models/— response model layer and collection wrappersmpt_api_client/rql/— fluent RQL query ...
Files:
tests/e2e/program/program/parameter/conftest.pytests/e2e/program/program/parameter/test_async_parameter.py
🧠 Learnings (8)
📚 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/program/program/parameter/conftest.pytests/e2e/program/program/parameter/test_async_parameter.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/program/program/parameter/conftest.pytests/e2e/program/program/parameter/test_async_parameter.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/program/program/parameter/conftest.pytests/e2e/program/program/parameter/test_async_parameter.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/program/program/parameter/conftest.pytests/e2e/program/program/parameter/test_async_parameter.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/program/program/parameter/conftest.pytests/e2e/program/program/parameter/test_async_parameter.py
📚 Learning: 2026-04-02T16:26:46.138Z
Learnt from: albertsola
Repo: softwareone-platform/mpt-api-python-client PR: 275
File: tests/e2e/exchange/currencies/conftest.py:43-43
Timestamp: 2026-04-02T16:26:46.138Z
Learning: In this repo’s Python E2E conftest files, line-specific flake8/wemake suppressions like `# noqa: WPS421` placed inline on individual statements (e.g., on specific `print()` calls in teardown blocks) are intentional and acceptable. When reviewing, avoid suggesting converting these to a file-wide ignore (e.g., adding per-file ignores under `[tool.flake8]` in `pyproject.toml`), since that would over-suppress WPS421 for the entire file. Also treat Ruff `RUF102` about an unknown rule code as expected here when suppressing `WPS421` from wemake-python-styleguide.
Applied to files:
tests/e2e/program/program/parameter/conftest.py
📚 Learning: 2026-04-06T09:03:34.466Z
Learnt from: albertsola
Repo: softwareone-platform/mpt-api-python-client PR: 277
File: tests/e2e/extensibility/extensions/conftest.py:37-40
Timestamp: 2026-04-06T09:03:34.466Z
Learning: In this repo’s E2E test conftest teardown fixtures, it is intentional to catch `MPTAPIError` broadly (not just 404) and to print a diagnostic message instead of re-raising. This avoids failing the test suite during teardown when a resource cannot be deleted in some scenarios. Do not recommend narrowing the `except` clause in these teardown blocks (e.g., checking `error.status == 404`), unless the fixture is clearly not intended as an E2E teardown diagnostic.
Applied to files:
tests/e2e/program/program/parameter/conftest.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/program/program/parameter/conftest.pytests/e2e/program/program/parameter/test_async_parameter.py
🔇 Additional comments (2)
tests/e2e/program/program/parameter/conftest.py (1)
3-3: LGTM!Also applies to: 19-19
tests/e2e/program/program/parameter/test_async_parameter.py (1)
5-5: LGTM!
…c and async tests
|
Member
Author
|
@coderabbitai review |
1 similar comment
Member
Author
|
@coderabbitai review |
Member
Author
|
@coderabbitai approve |
Member
Author
|
@coderabbitai resolve |
✅ Action performedComments resolved and changes approved. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes MPT-21557
Release Notes