MPT-22111 Add MPT pytest fixtures shared library#49
Conversation
📝 WalkthroughWalkthroughThis PR adds a new ChangesFixtures package rollout
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
fixtures/mpt_extension_contrib/fixtures/factories.py (1)
30-30: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInvalid
noqarule code flagged by Ruff.Ruff's RUF102 flags
WPS110as an unrecognized rule code since this repo uses Ruff, not flake8-wps. The comment is dead weight and Ruff can auto-remove it on--fix, silently dropping the intended annotation.🔧 Proposed fix
- ordering: Any = Use(list) # noqa: WPS110 - field name mirrors the SDK model + ordering: Any = Use(list) # field name mirrors the SDK modelAlternatively, add
WPStolint.externalin the rootpyproject.tomlif this suppression is meant to be preserved for a future flake8-wps run.🤖 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 `@fixtures/mpt_extension_contrib/fixtures/factories.py` at line 30, The `ordering` field in `factories.py` has a dead `# noqa: WPS110` annotation that Ruff treats as an invalid rule code, so remove that suppression or replace it with a Ruff-valid ignore if one is still needed. Update the `ordering: Any = Use(list)` declaration so it no longer depends on flake8-wps codes, and if preserving the ignore is intentional for future flake8-wps use, adjust the root lint configuration instead of leaving the invalid comment in `factories.py`.Source: Linters/SAST tools
🤖 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 `@fixtures/.copier-answers.yml`:
- Line 2: The committed _src_path value is hardcoded to a machine-specific
absolute path, so update the Copier answers entry to use a repo-relative
template path instead. Change the _src_path setting in
fixtures/.copier-answers.yml to point to the template location relative to the
repository root, keeping the file portable across checkouts and matching the
expected Copier configuration.
In `@fixtures/mpt_extension_contrib/fixtures/scenarios.py`:
- Around line 38-49: The build() method in scenarios.py is overwriting an
explicit parameters override whenever ordering or fulfillment is present in the
merged fields. Update ScenarioBuilder.build() so that a caller-provided
parameters value takes precedence, and only synthesize parameter_bag(...) when
parameters was not supplied; keep the ordering/fulfillment extraction logic in
build() consistent with the docstring and preserve overrides passed through
_registry and **overrides.
---
Nitpick comments:
In `@fixtures/mpt_extension_contrib/fixtures/factories.py`:
- Line 30: The `ordering` field in `factories.py` has a dead `# noqa: WPS110`
annotation that Ruff treats as an invalid rule code, so remove that suppression
or replace it with a Ruff-valid ignore if one is still needed. Update the
`ordering: Any = Use(list)` declaration so it no longer depends on flake8-wps
codes, and if preserving the ignore is intentional for future flake8-wps use,
adjust the root lint configuration instead of leaving the invalid comment in
`factories.py`.
🪄 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: 83d09553-5370-4169-b6ae-7b56353d8375
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (28)
AGENTS.mdREADME.mdfixtures/.copier-answers.ymlfixtures/AGENTS.mdfixtures/LICENSEfixtures/README.mdfixtures/docs/architecture.mdfixtures/docs/contributing.mdfixtures/docs/releases.mdfixtures/docs/testing.mdfixtures/docs/usage.mdfixtures/mpt_extension_contrib/fixtures/__init__.pyfixtures/mpt_extension_contrib/fixtures/errors.pyfixtures/mpt_extension_contrib/fixtures/factories.pyfixtures/mpt_extension_contrib/fixtures/parameters.pyfixtures/mpt_extension_contrib/fixtures/plugin.pyfixtures/mpt_extension_contrib/fixtures/py.typedfixtures/mpt_extension_contrib/fixtures/scenarios.pyfixtures/pyproject.tomlfixtures/tests/conftest.pyfixtures/tests/test_demo.pyfixtures/tests/test_errors.pyfixtures/tests/test_factories.pyfixtures/tests/test_parameters.pyfixtures/tests/test_plugin.pyfixtures/tests/test_scenarios.pymake/common.mkpyproject.toml
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
softwareone-platform/mpt-extension-skills(manual)
📜 Review details
🧰 Additional context used
📓 Path-based instructions (9)
**
⚙️ CodeRabbit configuration file
**: # AGENTS.mdWorking protocol for any task in this repository:
- Identify the package or repository-level concern affected by the task.
- Read only the relevant local files before making changes.
- Read the matching shared standard or operational guidance from
softwareone-platform/mpt-extension-skills
when the task involves shared engineering conventions (resolved locally from
${MPT_EXTENSION_SKILLS_HOME:-$HOME/.mpt-extension-skills}/current).- Treat repository-local documents as additions, restrictions, or overrides to
shared guidance.- If a repository-local rule conflicts with a shared or global rule, the
repository-local rule takes precedence.Repository model
mpt-extension-contribis a uv workspace monorepo
of independently released Python libraries for SoftwareONE MPT extensions. It does
not publish an umbrellampt-extension-contribdistribution.
- The root
pyproject.tomldefines the uv workspace and the shared
tool configuration (ruff, flake8, mypy, pytest, coverage). It is not an
installable distribution.- A single
uv.lockat the repository root locks the whole workspace.- Each root-level directory is one distribution. All distributions share the
PEP 420 namespace packagempt_extension_contrib.Mechanical naming rule for a module
<module>:Repository directory: <module> (kebab-case) Distribution name: mpt-extension-contrib-<module> Python import: mpt_extension_contrib.<module_with_underscores>Packages
shared/: internal reusable code exposed asmpt_extension_contrib.shared
(distributionmpt-extension-contrib-shared).custom-notifications/: public package exposed asmpt_extension_contrib.custom_notifications(distributionmpt-extension-contrib-custom-notifications)....
Files:
fixtures/tests/conftest.pyfixtures/docs/contributing.mdfixtures/LICENSEfixtures/AGENTS.mdfixtures/tests/test_parameters.pyAGENTS.mdfixtures/tests/test_plugin.pyfixtures/docs/testing.mdREADME.mdfixtures/docs/releases.mdfixtures/mpt_extension_contrib/fixtures/errors.pyfixtures/README.mdfixtures/mpt_extension_contrib/fixtures/__init__.pymake/common.mkfixtures/tests/test_demo.pyfixtures/mpt_extension_contrib/fixtures/plugin.pyfixtures/docs/architecture.mdfixtures/tests/test_scenarios.pyfixtures/tests/test_factories.pyfixtures/mpt_extension_contrib/fixtures/parameters.pypyproject.tomlfixtures/mpt_extension_contrib/fixtures/factories.pyfixtures/mpt_extension_contrib/fixtures/scenarios.pyfixtures/tests/test_errors.pyfixtures/pyproject.tomlfixtures/docs/usage.md
**/*
⚙️ CodeRabbit configuration file
**/*: For each subsequent commit in this PR, explicitly verify if previous review comments have been resolved
Files:
fixtures/tests/conftest.pyfixtures/docs/contributing.mdfixtures/LICENSEfixtures/AGENTS.mdfixtures/tests/test_parameters.pyAGENTS.mdfixtures/tests/test_plugin.pyfixtures/docs/testing.mdREADME.mdfixtures/docs/releases.mdfixtures/mpt_extension_contrib/fixtures/errors.pyfixtures/README.mdfixtures/mpt_extension_contrib/fixtures/__init__.pymake/common.mkfixtures/tests/test_demo.pyfixtures/mpt_extension_contrib/fixtures/plugin.pyfixtures/docs/architecture.mdfixtures/tests/test_scenarios.pyfixtures/tests/test_factories.pyfixtures/mpt_extension_contrib/fixtures/parameters.pypyproject.tomlfixtures/mpt_extension_contrib/fixtures/factories.pyfixtures/mpt_extension_contrib/fixtures/scenarios.pyfixtures/tests/test_errors.pyfixtures/pyproject.tomlfixtures/docs/usage.md
AGENTS.md
📄 CodeRabbit inference engine (Custom checks)
Update
AGENTS.mdwhen a change introduces or modifies top-level structure or agent navigation.
Files:
AGENTS.md
README.md
📄 CodeRabbit inference engine (Custom checks)
README.md: UpdateREADME.mdordocs/usage.mdwhen a change affects user-facing or public behavior and commands.
UpdateREADME.mdordocs/usage.mdwhen a change adds or changes CLI commands, followingstandards/cli.md.Read
README.mdfirst for repository purpose, structure, the two-view model, the naming rule, and themakeworkflow before making changes.
Files:
README.md
{shared,fixtures,custom-notifications,due-date}/mpt_extension_contrib/**
📄 CodeRabbit inference engine (AGENTS.md)
Keep each root-level package independently releasable and confined to its own concrete module directory under
mpt_extension_contrib.
Files:
fixtures/mpt_extension_contrib/fixtures/errors.pyfixtures/mpt_extension_contrib/fixtures/__init__.pyfixtures/mpt_extension_contrib/fixtures/plugin.pyfixtures/mpt_extension_contrib/fixtures/parameters.pyfixtures/mpt_extension_contrib/fixtures/factories.pyfixtures/mpt_extension_contrib/fixtures/scenarios.py
fixtures/**/fixtures/{__init__,plugin}.py
📄 CodeRabbit inference engine (fixtures/AGENTS.md)
Resolve the fixtures package implementation lazily in
__init__.pyandplugin.pyso startup registration stays measurable by coverage.
Files:
fixtures/mpt_extension_contrib/fixtures/__init__.pyfixtures/mpt_extension_contrib/fixtures/plugin.py
make/**
📄 CodeRabbit inference engine (AGENTS.md)
Prefer the canonical commands in
make/over ad hocuvor Docker invocations.
Files:
make/common.mk
⚙️ CodeRabbit configuration file
make/**: Review changes inmake/againstdocs/contributing.mdand the linked repository'sstandards/makefiles.md.
Use the shared standard as the source of truth for Makefile architecture, file layout, and command-group organization.
Files:
make/common.mk
pyproject.toml
📄 CodeRabbit inference engine (AGENTS.md)
Keep the root
pyproject.tomlas the workspace-level configuration only; do not treat it as an installable distribution, and use it for shared tool configuration such as ruff, flake8, mypy, pytest, and coverage.
Files:
pyproject.toml
{shared,fixtures,custom-notifications,due-date}/pyproject.toml
📄 CodeRabbit inference engine (AGENTS.md)
Add package-specific runtime and development dependencies to the matching package
pyproject.toml, not to the root workspace configuration.
Files:
fixtures/pyproject.toml
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: softwareone-platform/mpt-extension-python-contrib
Timestamp: 2026-07-01T08:46:15.073Z
Learning: Identify the package or repository-level concern affected by the task before making changes.
Learnt from: CR
Repo: softwareone-platform/mpt-extension-python-contrib
Timestamp: 2026-07-01T08:46:15.073Z
Learning: Read only the relevant local files before making changes.
Learnt from: CR
Repo: softwareone-platform/mpt-extension-python-contrib
Timestamp: 2026-07-01T08:46:15.073Z
Learning: When a task involves shared engineering conventions, read the matching guidance from `softwareone-platform/mpt-extension-skills` from the locally resolved `current` directory.
Learnt from: CR
Repo: softwareone-platform/mpt-extension-python-contrib
Timestamp: 2026-07-01T08:46:15.073Z
Learning: Treat repository-local documents as additions, restrictions, or overrides to shared guidance.
Learnt from: CR
Repo: softwareone-platform/mpt-extension-python-contrib
Timestamp: 2026-07-01T08:46:15.073Z
Learning: If a repository-local rule conflicts with a shared or global rule, the repository-local rule takes precedence.
Learnt from: CR
Repo: softwareone-platform/mpt-extension-python-contrib
Timestamp: 2026-07-01T08:46:26.613Z
Learning: Before changing this module, read `README.md`, `docs/usage.md`, `docs/architecture.md`, `docs/contributing.md`, `docs/testing.md`, `docs/releases.md`, and `../AGENTS.md` in that order.
Learnt from: CR
Repo: softwareone-platform/mpt-extension-python-contrib
Timestamp: 2026-07-01T08:46:26.613Z
Learning: Use `make check-all pkg=fixtures` from the repository root while iterating on changes to this module.
Learnt from: CR
Repo: softwareone-platform/mpt-extension-python-contrib
Timestamp: 2026-07-01T08:46:26.613Z
Learning: Before changing tests, read `docs/testing.md`; before releasing the module, read `docs/releases.md`; before changing the module, read `docs/contributing.md`.
Learnt from: CR
Repo: softwareone-platform/mpt-extension-python-contrib
Timestamp: 2026-07-01T08:46:26.613Z
Learning: The public API is re-exported from `mpt_extension_contrib.fixtures` via a lazy PEP 562 `__getattr__`; implementation lives in the `factories`, `parameters`, `scenarios`, and `errors` submodules.
📚 Learning: 2026-06-30T11:25:32.303Z
Learnt from: d3rky
Repo: softwareone-platform/mpt-extension-python-contrib PR: 45
File: custom-notifications/tests/conftest.py:35-40
Timestamp: 2026-06-30T11:25:32.303Z
Learning: In softwareone-platform/mpt-extension-python-contrib, Ruff rule S106 is already ignored for all `tests/**` via `[tool.ruff.lint.per-file-ignores]` in the root `pyproject.toml`. When editing test code (e.g., `custom-notifications/tests/conftest.py`), avoid adding inline `# noqa: S106` suppressions if the file is covered by that central `tests/**` ignore—Ruff will already skip S106 for these paths per repository configuration.
Applied to files:
fixtures/tests/conftest.pyfixtures/tests/test_parameters.pyfixtures/tests/test_plugin.pyfixtures/tests/test_demo.pyfixtures/tests/test_scenarios.pyfixtures/tests/test_factories.pyfixtures/tests/test_errors.py
📚 Learning: 2026-06-16T09:48:32.998Z
Learnt from: d3rky
Repo: softwareone-platform/mpt-extension-python-contrib PR: 9
File: make/common.mk:11-29
Timestamp: 2026-06-16T09:48:32.998Z
Learning: In SoftwareONE Platform repos that use the shared Makefile convention, do not add explicit `.PHONY` declarations inside `make/*.mk` included make snippet files. The root `Makefile` centrally auto-declares phony targets by scanning `MAKEFILE_LIST` (e.g., via the awk-based `.PHONY: $(shell awk ...)` line). Since targets from included `make/*.mk` files are already present in `MAKEFILE_LIST`, per-file `.PHONY` blocks are redundant and would diverge from the org standard. Only suggest explicit `.PHONY` in `make/*.mk` if this repo does not use the shared root Makefile convention / phony auto-scan.
Applied to files:
make/common.mk
🪛 GitHub Check: SonarCloud Code Analysis
fixtures/mpt_extension_contrib/fixtures/factories.py
[warning] 62-62: Complete the task associated to this "TODO" comment.
🪛 LanguageTool
fixtures/AGENTS.md
[style] ~10-~10: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...re.md) for the public API boundary. 4. docs/contributing.md ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~11-~11: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ng.md) before changing this module. 5. docs/testing.md before cha...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~12-~12: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: .../testing.md) before changing tests. 6. docs/releases.md before r...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
fixtures/docs/architecture.md
[grammar] ~40-~40: Use a hyphen to join words.
Context: ... detail. So the factories build SDK typed models, and .to_dict() is the ...
(QB_NEW_EN_HYPHEN)
fixtures/docs/usage.md
[grammar] ~195-~195: Ensure spelling is correct
Context: ...enario Scenarios.fixture() returns a parametrized fixture; any test that requests it runs...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 Ruff (0.15.20)
fixtures/mpt_extension_contrib/fixtures/factories.py
[warning] 30-30: Invalid rule code in # noqa: WPS110
Add non-Ruff rule codes to the lint.external configuration option
(RUF102)
🔇 Additional comments (28)
fixtures/pyproject.toml (1)
1-34: LGTM!fixtures/LICENSE (1)
1-202: LGTM!fixtures/AGENTS.md (1)
1-29: LGTM!fixtures/mpt_extension_contrib/fixtures/__init__.py (1)
14-64: LGTM! TheTYPE_CHECKINGimports,__all__, and_EXPORTSmapping are all kept consistent, and the lazy__getattr__correctly resolves each name to its owning submodule (factories,parameters,scenarios,errors), matching the design described in the retrieved learning about lazy PEP 562 exports.fixtures/mpt_extension_contrib/fixtures/factories.py (2)
62-66: LGTM! The TODO is well-justified (explains why the enum shouldn't live in this test library) — no action needed despite the SonarCloud flag.
1-29: LGTM!Also applies to: 34-54, 68-87
fixtures/tests/test_factories.py (1)
1-48: LGTM!fixtures/mpt_extension_contrib/fixtures/parameters.py (1)
1-38: LGTM!fixtures/tests/test_parameters.py (1)
1-22: LGTM!fixtures/mpt_extension_contrib/fixtures/errors.py (1)
1-33: LGTM!fixtures/tests/test_errors.py (1)
1-22: LGTM!fixtures/README.md (1)
1-20: LGTM!fixtures/docs/architecture.md (1)
1-113: LGTM!fixtures/docs/contributing.md (1)
1-9: LGTM!make/common.mk (1)
5-5: LGTM!pyproject.toml (1)
2-7: LGTM!Also applies to: 133-136, 138-146, 164-167, 192-197, 219-224
fixtures/docs/releases.md (1)
1-8: LGTM!fixtures/docs/testing.md (1)
1-14: LGTM!fixtures/docs/usage.md (1)
1-259: LGTM!AGENTS.md (1)
41-41: LGTM!README.md (1)
15-15: LGTM!fixtures/mpt_extension_contrib/fixtures/scenarios.py (2)
51-70: LGTM!
22-22: 🎯 Functional CorrectnessNo action needed:
fixtures/pyproject.tomlalready requires Python>=3.12, soclass Scenarios[ModelT: BaseModel]:is supported.> Likely an incorrect or invalid review comment.fixtures/tests/test_scenarios.py (1)
1-59: LGTM!fixtures/mpt_extension_contrib/fixtures/plugin.py (1)
1-65: LGTM!Lazy
importlib-based fixtures match the documented rationale (coverage measurability for code loaded before instrumentation starts) and the plugin'spytest11registration goal.fixtures/tests/conftest.py (1)
1-1: LGTM!fixtures/tests/test_plugin.py (1)
1-28: LGTM!Using
runpytest_subprocess()to verify the fixtures are auto-registered via the installedpytest11entry point (rather than an in-process run that could pick up ambient conftest state) is the right approach.fixtures/tests/test_demo.py (1)
1-69: LGTM!
| @@ -0,0 +1,4 @@ | |||
| # Managed by Copier — do not edit by hand. Run `copier update` to refresh. | |||
| _src_path: /workspace/scripts/templates/module | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
copier _src_path should be relative to project root
💡 Result:
In Copier, the _src_path key stored in the.copier-answers.yml file records the source of the template used to generate or update a project [1][2][3]. It is important to clarify the distinction between _src_path and settings that require paths relative to the project root: 1. The _src_path in.copier-answers.yml: This value represents the template source (which may be a local path or a remote VCS URL) [4][5][1]. It is not strictly forced to be relative to the project root; however, community discussions have highlighted that storing it as a path relative to the current working directory (as Copier sometimes does) can make it difficult to run updates from different locations [6]. 2. Configuration Settings: Certain Copier configuration settings, such as answers_file, must be defined relative to the project root [7][8]. According to the official documentation, the answers_file path must be relative to the project root because it must be accessible at render time and must exist within the version-controlled repository to support project updates [7]. If you are encountering issues where _src_path is not being resolved correctly during updates, it is often because Copier relies on this value to locate the template, and if it was saved as a relative path from a specific directory, it may not be valid if the update command is executed from a different directory [6]. The _src_path is intended to be a persistent reference to the template, and while it is stored in the answers file, it is primarily managed by Copier rather than manually configured by the user [4][1].
Citations:
- 1: https://copier.readthedocs.io/en/stable/creating/
- 2: https://copier.readthedocs.io/en/v9.3.0/reference/cli/
- 3: https://copier.readthedocs.io/en/latest/creating/
- 4: https://copier.readthedocs.io/en/v9.3.0/reference/main/
- 5: https://copier.readthedocs.io/en/v9.7.0/reference/main/
- 6: Shouldn't
_src_pathbe stored as absolute path? copier-org/copier#335 - 7: https://copier.readthedocs.io/en/stable/configuring/
- 8: Using answers file in a subdirectory is difficult copier-org/copier#2183
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== file =="
cat -n fixtures/.copier-answers.yml
echo
echo "== search for copier mentions =="
rg -n "_src_path|copier-answers|copier" -S . || true
echo
echo "== repo files around fixture/docs =="
fd -HI -t f 'copier' . || trueRepository: softwareone-platform/mpt-extension-python-contrib
Length of output: 2270
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== template =="
cat -n scripts/templates/module/copier.yml
echo
echo "== answers files =="
for f in custom-notifications/.copier-answers.yml due-date/.copier-answers.yml fixtures/.copier-answers.yml; do
echo "--- $f ---"
cat -n "$f"
doneRepository: softwareone-platform/mpt-extension-python-contrib
Length of output: 1608
🌐 Web query:
Copier _src_path answers file absolute local path update behavior
💡 Result:
In Copier, the _src_path key in the.copier-answers.yml file tracks the location of the template used to generate or last update a project [1][2]. Its behavior regarding paths is as follows: 1. Storage Behavior: Copier typically stores _src_path as it was provided during the initial copy operation [3]. If a relative path is provided, it is stored as a relative path; if an absolute path is provided, it is stored as an absolute path [3]. Note that relative paths are often resolved relative to the current working directory from which the command was executed, which can lead to issues if subsequent updates are run from a different directory [3]. 2. Update Behavior: When you run copier update, Copier relies on the _src_path stored in the answers file to locate the template [1][4]. If the path is relative, Copier will attempt to resolve it based on the directory where the update command is being executed [3]. Because this behavior can be brittle, users are cautioned against manually editing the.copier-answers.yml file [5][6]. Doing so can confuse Copier’s update logic, as the tool uses the stored _src_path and _commit keys to determine how to apply changes while preserving local modifications [5][7]. 3. Recommendations: To ensure reliable updates, it is generally best practice to avoid manual manipulation of _src_path [5]. If the template location changes or the relative path resolution fails, you may need to use copier recopy, which reapplies the template while keeping your existing answers, effectively bypassing the smart diff algorithm that relies on the historical template path and commit [5][8]. In summary, Copier does not automatically normalize _src_path to an absolute local path. It persists the path as originally specified, and the reliability of future updates depends on that path remaining valid relative to where the copier update command is invoked [3][1].
Citations:
- 1: https://copier.readthedocs.io/en/v9.3.0/reference/cli/
- 2: https://copier.readthedocs.io/en/stable/creating/
- 3: Shouldn't
_src_pathbe stored as absolute path? copier-org/copier#335 - 4: https://copier.readthedocs.io/en/v9.3.0/reference/main/
- 5: https://copier.readthedocs.io/en/latest/updating/
- 6: https://copier.readthedocs.io/en/stable/updating/
- 7: https://copier.readthedocs.io/en/v7.2.0/updating/
- 8: https://copier.readthedocs.io/en/latest/generating/
Replace the committed _src_path with a repo-relative template path
fixtures/.copier-answers.yml hardcodes /workspace/scripts/templates/module, which ties future Copier updates to one checkout location. Use a repo-relative source path so the answers file stays portable across machines.
🤖 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 `@fixtures/.copier-answers.yml` at line 2, The committed _src_path value is
hardcoded to a machine-specific absolute path, so update the Copier answers
entry to use a repo-relative template path instead. Change the _src_path setting
in fixtures/.copier-answers.yml to point to the template location relative to
the repository root, keeping the file portable across checkouts and matching the
expected Copier configuration.
2f8112e to
8717c0e
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@fixtures/docs/usage.md`:
- Around line 129-135: The AwsOrderFactory.migration preset currently forwards
parameters= alongside **overrides, so a caller-provided parameters value will
cause a duplicate-key TypeError. Update migration in the AwsOrderFactory example
to merge its default parameter_bag value with any caller overrides before
calling purchase, keeping the same override behavior as build() while preserving
the preset’s defaults.
In `@fixtures/mpt_extension_contrib/fixtures/factories.py`:
- Line 30: The inline suppression on the ordering field in factories.py is not
Ruff-compatible, so Ruff will flag it as an invalid suppression. Update the lint
setup to recognize WPS110 as an external code if that is intended, or replace
the comment with a Ruff-accepted suppression near the ordering declaration in
the fixture factory. Keep the fix aligned with the field definition in the
factory model so the lint exception matches the actual rule being silenced.
🪄 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: 01932b58-d3ec-4f27-aeca-ee4954227ac7
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (29)
AGENTS.mdREADME.mdfixtures/.copier-answers.ymlfixtures/AGENTS.mdfixtures/LICENSEfixtures/README.mdfixtures/docs/architecture.mdfixtures/docs/contributing.mdfixtures/docs/releases.mdfixtures/docs/technical-design-review.mdfixtures/docs/testing.mdfixtures/docs/usage.mdfixtures/mpt_extension_contrib/fixtures/__init__.pyfixtures/mpt_extension_contrib/fixtures/errors.pyfixtures/mpt_extension_contrib/fixtures/factories.pyfixtures/mpt_extension_contrib/fixtures/parameters.pyfixtures/mpt_extension_contrib/fixtures/plugin.pyfixtures/mpt_extension_contrib/fixtures/py.typedfixtures/mpt_extension_contrib/fixtures/scenarios.pyfixtures/pyproject.tomlfixtures/tests/conftest.pyfixtures/tests/test_demo.pyfixtures/tests/test_errors.pyfixtures/tests/test_factories.pyfixtures/tests/test_parameters.pyfixtures/tests/test_plugin.pyfixtures/tests/test_scenarios.pymake/common.mkpyproject.toml
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
softwareone-platform/mpt-extension-skills(manual)
✅ Files skipped from review due to trivial changes (8)
- fixtures/docs/technical-design-review.md
- AGENTS.md
- fixtures/docs/testing.md
- fixtures/docs/contributing.md
- fixtures/README.md
- fixtures/LICENSE
- fixtures/docs/releases.md
- fixtures/.copier-answers.yml
🚧 Files skipped from review as they are similar to previous changes (16)
- make/common.mk
- fixtures/pyproject.toml
- fixtures/tests/test_parameters.py
- fixtures/tests/test_errors.py
- fixtures/tests/conftest.py
- fixtures/mpt_extension_contrib/fixtures/plugin.py
- fixtures/mpt_extension_contrib/fixtures/init.py
- fixtures/tests/test_demo.py
- fixtures/mpt_extension_contrib/fixtures/errors.py
- README.md
- fixtures/tests/test_factories.py
- fixtures/tests/test_scenarios.py
- fixtures/mpt_extension_contrib/fixtures/scenarios.py
- fixtures/mpt_extension_contrib/fixtures/parameters.py
- fixtures/tests/test_plugin.py
- pyproject.toml
📜 Review details
🧰 Additional context used
📓 Path-based instructions (3)
{shared,fixtures,custom-notifications,due-date}/**
📄 CodeRabbit inference engine (AGENTS.md)
Each root-level package (
shared/,fixtures/,custom-notifications/,due-date/) must remain independently releasable and represent a single distribution in the workspace model.
Files:
fixtures/AGENTS.mdfixtures/docs/architecture.mdfixtures/docs/usage.mdfixtures/mpt_extension_contrib/fixtures/factories.py
**
⚙️ CodeRabbit configuration file
**: # AGENTS.mdWorking protocol for any task in this repository:
- Identify the package or repository-level concern affected by the task.
- Read only the relevant local files before making changes.
- Read the matching shared standard or operational guidance from
softwareone-platform/mpt-extension-skills
when the task involves shared engineering conventions (resolved locally from
${MPT_EXTENSION_SKILLS_HOME:-$HOME/.mpt-extension-skills}/current).- Treat repository-local documents as additions, restrictions, or overrides to
shared guidance.- If a repository-local rule conflicts with a shared or global rule, the
repository-local rule takes precedence.Repository model
mpt-extension-contribis a uv workspace monorepo
of independently released Python libraries for SoftwareONE MPT extensions. It does
not publish an umbrellampt-extension-contribdistribution.
- The root
pyproject.tomldefines the uv workspace and the shared
tool configuration (ruff, flake8, mypy, pytest, coverage). It is not an
installable distribution.- A single
uv.lockat the repository root locks the whole workspace.- Each root-level directory is one distribution. All distributions share the
PEP 420 namespace packagempt_extension_contrib.Mechanical naming rule for a module
<module>:Repository directory: <module> (kebab-case) Distribution name: mpt-extension-contrib-<module> Python import: mpt_extension_contrib.<module_with_underscores>Packages
shared/: internal reusable code exposed asmpt_extension_contrib.shared
(distributionmpt-extension-contrib-shared).custom-notifications/: public package exposed asmpt_extension_contrib.custom_notifications(distributionmpt-extension-contrib-custom-notifications)....
Files:
fixtures/AGENTS.mdfixtures/docs/architecture.mdfixtures/docs/usage.mdfixtures/mpt_extension_contrib/fixtures/factories.py
**/*
⚙️ CodeRabbit configuration file
**/*: For each subsequent commit in this PR, explicitly verify if previous review comments have been resolved
Files:
fixtures/AGENTS.mdfixtures/docs/architecture.mdfixtures/docs/usage.mdfixtures/mpt_extension_contrib/fixtures/factories.py
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
Repo: softwareone-platform/mpt-extension-python-contrib
Timestamp: 2026-07-01T09:00:34.979Z
Learning: Identify the package or repository-level concern affected by the task before making changes.
Learnt from: CR
Repo: softwareone-platform/mpt-extension-python-contrib
Timestamp: 2026-07-01T09:00:34.979Z
Learning: Read only the relevant local files before making changes.
Learnt from: CR
Repo: softwareone-platform/mpt-extension-python-contrib
Timestamp: 2026-07-01T09:00:34.979Z
Learning: When a task involves shared engineering conventions, read the matching shared standard or operational guidance from `softwareone-platform/mpt-extension-skills` (resolved locally from `${MPT_EXTENSION_SKILLS_HOME:-$HOME/.mpt-extension-skills}/current`).
Learnt from: CR
Repo: softwareone-platform/mpt-extension-python-contrib
Timestamp: 2026-07-01T09:00:34.979Z
Learning: Treat repository-local documents as additions, restrictions, or overrides to shared guidance.
Learnt from: CR
Repo: softwareone-platform/mpt-extension-python-contrib
Timestamp: 2026-07-01T09:00:34.979Z
Learning: If a repository-local rule conflicts with a shared or global rule, the repository-local rule takes precedence.
Learnt from: CR
Repo: softwareone-platform/mpt-extension-python-contrib
Timestamp: 2026-07-01T09:00:41.584Z
Learning: Read README.md, docs/usage.md, docs/architecture.md, docs/technical-design-review.md, docs/contributing.md, docs/testing.md, docs/releases.md, and ../AGENTS.md in that order before working on this module.
Learnt from: CR
Repo: softwareone-platform/mpt-extension-python-contrib
Timestamp: 2026-07-01T09:00:41.584Z
Learning: Run `make check-all pkg=fixtures` from the repository root while iterating.
🪛 GitHub Check: SonarCloud Code Analysis
fixtures/mpt_extension_contrib/fixtures/factories.py
[warning] 62-62: Complete the task associated to this "TODO" comment.
🪛 LanguageTool
fixtures/AGENTS.md
[style] ~10-~10: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...re.md) for the public API boundary. 4. [docs/technical-design-review.md](docs/techni...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~11-~11: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...rements, and how they map to usage. 5. docs/contributing.md ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~12-~12: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ng.md) before changing this module. 6. docs/testing.md before cha...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~13-~13: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: .../testing.md) before changing tests. 7. docs/releases.md before r...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
fixtures/docs/architecture.md
[grammar] ~40-~40: Use a hyphen to join words.
Context: ... detail. So the factories build SDK typed models, and .to_dict() is the ...
(QB_NEW_EN_HYPHEN)
fixtures/docs/usage.md
[grammar] ~195-~195: Ensure spelling is correct
Context: ...enario Scenarios.fixture() returns a parametrized fixture; any test that requests it runs...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 Ruff (0.15.20)
fixtures/mpt_extension_contrib/fixtures/factories.py
[warning] 30-30: Invalid rule code in # noqa: WPS110
Add non-Ruff rule codes to the lint.external configuration option
(RUF102)
🔇 Additional comments (2)
fixtures/AGENTS.md (1)
1-29: LGTM!fixtures/docs/architecture.md (1)
1-113: LGTM!
| class AwsOrderFactory(OrderFactory): | ||
| @classmethod | ||
| def migration(cls, **overrides): | ||
| return cls.purchase( | ||
| parameters=parameter_bag(ordering={"accountType": "Migrate", "mpaId": "123456"}), | ||
| **overrides, | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the preset helper override-safe.
This example forwards parameters= and **overrides together, so any caller that passes parameters will hit a duplicate-key TypeError. Merge parameters before delegating if the preset is meant to keep the same override surface as build().
♻️ Proposed fix
def migration(cls, **overrides):
- return cls.purchase(
- parameters=parameter_bag(ordering={"accountType": "Migrate", "mpaId": "123456"}),
- **overrides,
- )
+ parameters = overrides.pop(
+ "parameters",
+ parameter_bag(ordering={"accountType": "Migrate", "mpaId": "123456"}),
+ )
+ return cls.purchase(parameters=parameters, **overrides)📝 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.
| class AwsOrderFactory(OrderFactory): | |
| @classmethod | |
| def migration(cls, **overrides): | |
| return cls.purchase( | |
| parameters=parameter_bag(ordering={"accountType": "Migrate", "mpaId": "123456"}), | |
| **overrides, | |
| ) | |
| class AwsOrderFactory(OrderFactory): | |
| `@classmethod` | |
| def migration(cls, **overrides): | |
| parameters = overrides.pop( | |
| "parameters", | |
| parameter_bag(ordering={"accountType": "Migrate", "mpaId": "123456"}), | |
| ) | |
| return cls.purchase(parameters=parameters, **overrides) |
🤖 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 `@fixtures/docs/usage.md` around lines 129 - 135, The AwsOrderFactory.migration
preset currently forwards parameters= alongside **overrides, so a
caller-provided parameters value will cause a duplicate-key TypeError. Update
migration in the AwsOrderFactory example to merge its default parameter_bag
value with any caller overrides before calling purchase, keeping the same
override behavior as build() while preserving the preset’s defaults.
| __set_as_default_factory_for_type__ = True | ||
| __check_model__ = False | ||
|
|
||
| ordering: Any = Use(list) # noqa: WPS110 - field name mirrors the SDK model |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Fix the invalid inline lint suppression.
# noqa: WPS110 is not a Ruff-recognized suppression, so Ruff will emit RUF102 here unless WPS110 is registered as an external code in the workspace lint config. Please update the repo-level lint config or replace this with a Ruff-compatible suppression.
🧰 Tools
🪛 Ruff (0.15.20)
[warning] 30-30: Invalid rule code in # noqa: WPS110
Add non-Ruff rule codes to the lint.external configuration option
(RUF102)
🤖 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 `@fixtures/mpt_extension_contrib/fixtures/factories.py` at line 30, The inline
suppression on the ordering field in factories.py is not Ruff-compatible, so
Ruff will flag it as an invalid suppression. Update the lint setup to recognize
WPS110 as an external code if that is intended, or replace the comment with a
Ruff-accepted suppression near the ordering declaration in the fixture factory.
Keep the fix aligned with the field definition in the factory model so the lint
exception matches the actual rule being silenced.
Source: Linters/SAST tools
8717c0e to
6243145
Compare
Introduce mpt-extension-contrib-fixtures: polyfactory factories for typed SDK models (Order/Agreement/Subscription) with empty parameters, the parameter_bag helper, the Scenarios registry for parametrized tests, and mpt_error_factory. Ships as a pytest plugin via the pytest11 entry point, with a lazy plugin/package init so plugin-startup imports stay measurable by coverage. Documents the SDK-vs-API-client representation boundary and the roadmap, and adds an Order/Agreement demo test. MPT-22111 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
6243145 to
9d1813a
Compare
|
There was a problem hiding this comment.
🧹 Nitpick comments (2)
fixtures/mpt_extension_contrib/fixtures/scenarios.py (1)
35-35: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueShallow registry copy shares mutable spec values across builds.
dict(registry)only shallow-copies the outer mapping; nested spec dicts (and any mutable values within, e.g. a sharedparametersobject) are shared by reference across allbuild()calls for that scenario name. If a caller mutates a returned model'sparametersin place, it could leak into subsequent builds of the same scenario. Low risk given current usage (specs are typically literal dicts), but worth a defensive copy if scenarios are reused across tests with in-place mutation.Also applies to: 44-44
🤖 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 `@fixtures/mpt_extension_contrib/fixtures/scenarios.py` at line 35, The scenario registry copy in the scenario builder is only shallow, so nested spec values can be shared across build() calls. Update the registry handling in the scenario class that assigns self._registry from registry to make a defensive deep copy of the stored specs and any mutable nested values, so later in-place mutations from one build do not affect subsequent builds for the same scenario.fixtures/tests/test_scenarios.py (1)
1-59: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd a regression test for explicit
parametersoverride.Given the prior bug where
parametersoverrides were silently discarded, consider adding a test likeORDERS.build("with_params", parameters=parameter_bag(ordering={"accountType": "Existing"}))and asserting the override value wins, to guard against regression now that the fix is in place.✅ Suggested additional test
def test_build_override_replaces_parameters() -> None: override = parameter_bag(ordering={"accountType": "Existing"}) result = ORDERS.build("with_params", parameters=override) assert result.parameters.get_ordering_value("accountType") == "Existing"🤖 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 `@fixtures/tests/test_scenarios.py` around lines 1 - 59, Add a regression test in the Scenarios test suite to verify that an explicit parameters override is preserved by ORDERS.build; use ORDERS.build("with_params", parameters=parameter_bag(...)) and assert the override value wins instead of the scenario default. Place it alongside the existing test_build_* cases and reference the ORDERS fixture setup plus parameter_bag to cover the previously dropped-override path.
🤖 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 `@fixtures/mpt_extension_contrib/fixtures/scenarios.py`:
- Line 35: The scenario registry copy in the scenario builder is only shallow,
so nested spec values can be shared across build() calls. Update the registry
handling in the scenario class that assigns self._registry from registry to make
a defensive deep copy of the stored specs and any mutable nested values, so
later in-place mutations from one build do not affect subsequent builds for the
same scenario.
In `@fixtures/tests/test_scenarios.py`:
- Around line 1-59: Add a regression test in the Scenarios test suite to verify
that an explicit parameters override is preserved by ORDERS.build; use
ORDERS.build("with_params", parameters=parameter_bag(...)) and assert the
override value wins instead of the scenario default. Place it alongside the
existing test_build_* cases and reference the ORDERS fixture setup plus
parameter_bag to cover the previously dropped-override path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 94d3d435-04ec-4141-9879-38d030002c2d
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (29)
AGENTS.mdREADME.mdfixtures/.copier-answers.ymlfixtures/AGENTS.mdfixtures/LICENSEfixtures/README.mdfixtures/docs/architecture.mdfixtures/docs/contributing.mdfixtures/docs/releases.mdfixtures/docs/technical-design-review.mdfixtures/docs/testing.mdfixtures/docs/usage.mdfixtures/mpt_extension_contrib/fixtures/__init__.pyfixtures/mpt_extension_contrib/fixtures/errors.pyfixtures/mpt_extension_contrib/fixtures/factories.pyfixtures/mpt_extension_contrib/fixtures/parameters.pyfixtures/mpt_extension_contrib/fixtures/plugin.pyfixtures/mpt_extension_contrib/fixtures/py.typedfixtures/mpt_extension_contrib/fixtures/scenarios.pyfixtures/pyproject.tomlfixtures/tests/conftest.pyfixtures/tests/test_demo.pyfixtures/tests/test_errors.pyfixtures/tests/test_factories.pyfixtures/tests/test_parameters.pyfixtures/tests/test_plugin.pyfixtures/tests/test_scenarios.pymake/common.mkpyproject.toml
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
softwareone-platform/mpt-extension-skills(manual)
✅ Files skipped from review due to trivial changes (11)
- fixtures/docs/releases.md
- fixtures/tests/conftest.py
- fixtures/.copier-answers.yml
- fixtures/docs/testing.md
- fixtures/LICENSE
- fixtures/README.md
- AGENTS.md
- fixtures/pyproject.toml
- fixtures/docs/contributing.md
- README.md
- fixtures/docs/technical-design-review.md
🚧 Files skipped from review as they are similar to previous changes (11)
- fixtures/tests/test_plugin.py
- fixtures/tests/test_parameters.py
- fixtures/tests/test_demo.py
- make/common.mk
- fixtures/mpt_extension_contrib/fixtures/errors.py
- fixtures/mpt_extension_contrib/fixtures/parameters.py
- fixtures/tests/test_factories.py
- fixtures/mpt_extension_contrib/fixtures/init.py
- pyproject.toml
- fixtures/tests/test_errors.py
- fixtures/mpt_extension_contrib/fixtures/plugin.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
fixtures/**/fixtures/**/*.py
📄 CodeRabbit inference engine (fixtures/AGENTS.md)
Implementation for the fixtures package should live in the
factories,parameters,scenarios, anderrorssubmodules, not in the public package surface.
Files:
fixtures/mpt_extension_contrib/fixtures/scenarios.pyfixtures/mpt_extension_contrib/fixtures/factories.py
**
⚙️ CodeRabbit configuration file
**: # AGENTS.mdWorking protocol for any task in this repository:
- Identify the package or repository-level concern affected by the task.
- Read only the relevant local files before making changes.
- Read the matching shared standard or operational guidance from
softwareone-platform/mpt-extension-skills
when the task involves shared engineering conventions (resolved locally from
${MPT_EXTENSION_SKILLS_HOME:-$HOME/.mpt-extension-skills}/current).- Treat repository-local documents as additions, restrictions, or overrides to
shared guidance.- If a repository-local rule conflicts with a shared or global rule, the
repository-local rule takes precedence.Repository model
mpt-extension-contribis a uv workspace monorepo
of independently released Python libraries for SoftwareONE MPT extensions. It does
not publish an umbrellampt-extension-contribdistribution.
- The root
pyproject.tomldefines the uv workspace and the shared
tool configuration (ruff, flake8, mypy, pytest, coverage). It is not an
installable distribution.- A single
uv.lockat the repository root locks the whole workspace.- Each root-level directory is one distribution. All distributions share the
PEP 420 namespace packagempt_extension_contrib.Mechanical naming rule for a module
<module>:Repository directory: <module> (kebab-case) Distribution name: mpt-extension-contrib-<module> Python import: mpt_extension_contrib.<module_with_underscores>Packages
shared/: internal reusable code exposed asmpt_extension_contrib.shared
(distributionmpt-extension-contrib-shared).custom-notifications/: public package exposed asmpt_extension_contrib.custom_notifications(distributionmpt-extension-contrib-custom-notifications)....
Files:
fixtures/mpt_extension_contrib/fixtures/scenarios.pyfixtures/tests/test_scenarios.pyfixtures/AGENTS.mdfixtures/docs/architecture.mdfixtures/docs/usage.mdfixtures/mpt_extension_contrib/fixtures/factories.py
**/*
⚙️ CodeRabbit configuration file
**/*: For each subsequent commit in this PR, explicitly verify if previous review comments have been resolved
Files:
fixtures/mpt_extension_contrib/fixtures/scenarios.pyfixtures/tests/test_scenarios.pyfixtures/AGENTS.mdfixtures/docs/architecture.mdfixtures/docs/usage.mdfixtures/mpt_extension_contrib/fixtures/factories.py
fixtures/**/tests/**/*.py
📄 CodeRabbit inference engine (fixtures/AGENTS.md)
Test plugin behavior with the
pytesterfixture, unit-test each helper directly, and add tests undertests/for every behavior change.
Files:
fixtures/tests/test_scenarios.py
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: softwareone-platform/mpt-extension-python-contrib
Timestamp: 2026-07-01T09:29:23.593Z
Learning: Identify the package or repository-level concern affected by the task before making changes.
Learnt from: CR
Repo: softwareone-platform/mpt-extension-python-contrib
Timestamp: 2026-07-01T09:29:23.593Z
Learning: Read only the relevant local files before making changes.
Learnt from: CR
Repo: softwareone-platform/mpt-extension-python-contrib
Timestamp: 2026-07-01T09:29:23.593Z
Learning: When a task involves shared engineering conventions, read the matching guidance from `softwareone-platform/mpt-extension-skills` resolved from `${MPT_EXTENSION_SKILLS_HOME:-$HOME/.mpt-extension-skills}/current`.
Learnt from: CR
Repo: softwareone-platform/mpt-extension-python-contrib
Timestamp: 2026-07-01T09:29:23.593Z
Learning: Treat repository-local documents as additions, restrictions, or overrides to shared guidance.
Learnt from: CR
Repo: softwareone-platform/mpt-extension-python-contrib
Timestamp: 2026-07-01T09:29:23.593Z
Learning: If a repository-local rule conflicts with a shared or global rule, the repository-local rule takes precedence.
Learnt from: CR
Repo: softwareone-platform/mpt-extension-python-contrib
Timestamp: 2026-07-01T09:29:23.593Z
Learning: `mpt-extension-contrib` is a uv workspace monorepo of independently released Python libraries and does not publish an umbrella `mpt-extension-contrib` distribution.
Learnt from: CR
Repo: softwareone-platform/mpt-extension-python-contrib
Timestamp: 2026-07-01T09:29:23.593Z
Learning: Each root-level directory is one independently released distribution sharing the `mpt_extension_contrib` PEP 420 namespace package.
Learnt from: CR
Repo: softwareone-platform/mpt-extension-python-contrib
Timestamp: 2026-07-01T09:29:23.593Z
Learning: Use the mechanical naming rule for modules: repository directory in kebab-case, distribution name `mpt-extension-contrib-<module>`, and Python import `mpt_extension_contrib.<module_with_underscores>`.
Learnt from: CR
Repo: softwareone-platform/mpt-extension-python-contrib
Timestamp: 2026-07-01T09:29:23.593Z
Learning: Keep each root-level package independently releasable.
Learnt from: CR
Repo: softwareone-platform/mpt-extension-python-contrib
Timestamp: 2026-07-01T09:29:23.593Z
Learning: Do not invent release, compatibility, or testing guarantees that are not represented in repository code or documentation.
Learnt from: CR
Repo: softwareone-platform/mpt-extension-python-contrib
Timestamp: 2026-07-01T09:30:15.414Z
Learning: Read `README.md`, `docs/usage.md`, `docs/architecture.md`, `docs/technical-design-review.md`, `docs/contributing.md`, `docs/testing.md`, `docs/releases.md`, and `../AGENTS.md` in that order before changing this module or its tests.
Learnt from: CR
Repo: softwareone-platform/mpt-extension-python-contrib
Timestamp: 2026-07-01T09:30:15.414Z
Learning: Run `make check-all pkg=fixtures` from the repository root while iterating.
📚 Learning: 2026-06-30T11:25:32.303Z
Learnt from: d3rky
Repo: softwareone-platform/mpt-extension-python-contrib PR: 45
File: custom-notifications/tests/conftest.py:35-40
Timestamp: 2026-06-30T11:25:32.303Z
Learning: In softwareone-platform/mpt-extension-python-contrib, Ruff rule S106 is already ignored for all `tests/**` via `[tool.ruff.lint.per-file-ignores]` in the root `pyproject.toml`. When editing test code (e.g., `custom-notifications/tests/conftest.py`), avoid adding inline `# noqa: S106` suppressions if the file is covered by that central `tests/**` ignore—Ruff will already skip S106 for these paths per repository configuration.
Applied to files:
fixtures/tests/test_scenarios.py
🪛 GitHub Check: SonarCloud Code Analysis
fixtures/mpt_extension_contrib/fixtures/factories.py
[warning] 62-62: Complete the task associated to this "TODO" comment.
🪛 LanguageTool
fixtures/AGENTS.md
[style] ~10-~10: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...re.md) for the public API boundary. 4. [docs/technical-design-review.md](docs/techni...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~11-~11: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...rements, and how they map to usage. 5. docs/contributing.md ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~12-~12: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ng.md) before changing this module. 6. docs/testing.md before cha...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~13-~13: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: .../testing.md) before changing tests. 7. docs/releases.md before r...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
fixtures/docs/architecture.md
[grammar] ~40-~40: Use a hyphen to join words.
Context: ... detail. So the factories build SDK typed models. For now, tests **mock at...
(QB_NEW_EN_HYPHEN)
fixtures/docs/usage.md
[grammar] ~201-~201: Ensure spelling is correct
Context: ...enario Scenarios.fixture() returns a parametrized fixture; any test that requests it runs...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 Ruff (0.15.20)
fixtures/mpt_extension_contrib/fixtures/factories.py
[warning] 30-30: Invalid rule code in # noqa: WPS110
Add non-Ruff rule codes to the lint.external configuration option
(RUF102)
🔀 Multi-repo context softwareone-platform/mpt-extension-skills
Relevant context in [::softwareone-platform/mpt-extension-skills::]:
standards/unittests.md:277-285— recommends splitting shared fixtures into a dedicatedfixtures/package and registering it viapytest_plugins, which matches the new installable fixtures package.standards/unittests.md:343-380— recommends factory fixtures for reusable test object builders, including anorder_factoryexample that aligns with the newOrderFactory-style API.
No repo-local consumers of the new package/API were found, so I didn’t identify a breaking cross-repo dependency.
🔇 Additional comments (6)
fixtures/mpt_extension_contrib/fixtures/scenarios.py (2)
37-44: Past review issue resolved.The previously flagged bug — where
build()silently discarded an explicitparametersoverride wheneverordering/fulfillmentwere present — is fixed. Theordering/fulfillmentpop-and-recompute logic has been removed entirely;parametersis now just a regular spec field passed straight through, as confirmed by the docstring andtest_scenarios.py'swith_paramsscenario.
21-21: 🎯 Functional CorrectnessNo action needed:
fixturesalready targets Python 3.12
fixtures/pyproject.tomlsetsrequires-python = ">=3.12", and the rootpyproject.tomlis workspace-only tool config, so the PEP 695 class syntax is valid here.> Likely an incorrect or invalid review comment.fixtures/docs/usage.md (1)
129-135: Keep the preset override merge safe.This still forwards
parameters=alongside**overrides, so a caller-suppliedparametersvalue will raiseTypeError. As per path instructions, I verified the earlier review note is still unresolved.Source: Path instructions
fixtures/mpt_extension_contrib/fixtures/factories.py (1)
30-31: Revisit the Ruff suppression onordering.
# noqa: WPS110is still present here, and Ruff flagsWPS110as an invalid suppression unless it is registered as an external code. As per path instructions, I verified the earlier review note is still unresolved.Sources: Path instructions, Linters/SAST tools
fixtures/AGENTS.md (1)
1-30: LGTM!fixtures/docs/architecture.md (1)
1-109: LGTM!
| ## Roadmap | ||
|
|
||
| Shipped: `OrderFactory` / `AgreementFactory` / `SubscriptionFactory`, | ||
| `parameter_bag`, `Scenarios`, `mpt_error_factory`, the pytest plugin. | ||
|
|
||
| Planned (MPT-22111 — platform/MPT-API scope only): | ||
|
|
||
| - Realistic MPT ids per object (`ORD-####-####-####`, `AGR-…`, `LCE-…`, …) via | ||
| per-factory id providers, propagated into nested models. | ||
| - More factories where an SDK model exists: Asset, Account/Buyer/Seller/Licensee, | ||
| Product/Item, Template, Authorization, Price, `ParameterValueFactory` (rich | ||
| parameters with type/phase/constraints), Extension/Installation. | ||
| - Event fixtures (`Event` / `TaskEvent` / `EventMetadata`). | ||
| - API-response helpers for wire mocking: `api_object`, `api_collection` | ||
| (`{"data": [...], "$meta": {...}}`), `api_error`, plus an optional `respx` | ||
| fixture. | ||
| - Depends on SDK follow-ups: models for Listing / Request / PriceList, and | ||
| canonical `OrderType` / `OrderStatus` enums (the SDK still types these as | ||
| `str`). |
There was a problem hiding this comment.
I think this is out of scope for the architecture documentation
| ## Pytest plugin conventions | ||
|
|
||
| This package follows the canonical installable-plugin layout | ||
| ([pytest docs](https://docs.pytest.org/en/stable/how-to/writing_plugins.html), | ||
| [`cookiecutter-pytest-plugin`](https://github.com/pytest-dev/cookiecutter-pytest-plugin)): | ||
|
|
||
| - **Entry point.** `[project.entry-points.pytest11]` maps a name to | ||
| `mpt_extension_contrib.fixtures.plugin`; pytest auto-discovers the plugin once | ||
| the package is installed — no `conftest.py` wiring for consumers. | ||
| - **`plugin.py`.** Holds the fixtures (and would hold hooks such as | ||
| `pytest_configure` / `pytest_addoption` and custom marker registration, if | ||
| any were needed). | ||
| - **Package root re-exports the public API** so `from mpt_extension_contrib.fixtures | ||
| import OrderFactory` works. | ||
| - **Classifier.** `Framework :: Pytest` is declared so the package is | ||
| discoverable as a pytest plugin on PyPI. | ||
| - **Testing.** The plugin is verified the canonical way — with the `pytester` | ||
| fixture running an inline pytest session | ||
| ([test_plugin.py](../tests/test_plugin.py)) — in addition to direct unit tests | ||
| of each helper. | ||
| - **Assertion rewriting.** Not used here (no shipped assertion helpers); if a | ||
| helper module ever grows `assert`s for consumers, register it with | ||
| `pytest.register_assert_rewrite(...)`. |
There was a problem hiding this comment.
Umm... I'm not sure if all of this information should be part of this document or if it should be moved to the usage documentation
| @@ -0,0 +1,118 @@ | |||
| # Technical Design Review — MPT pytest fixtures (MPT-22111) | |||
There was a problem hiding this comment.
Do we want to keep TDRs in the repository? If so, I suggest using a specific folder docs/tdrs/, since we can include multiples TDRs.
Also, should we remove MPT-22111 from the document? This is public documentation, while the Jira issue is not publicly accessible. We have the internal traceability in the PR and commit history
| """MPT pytest fixtures and polyfactory model factories. | ||
|
|
||
| Public API for building MPT-shaped test objects. The factories produce | ||
| structurally valid SDK models with empty parameters; product-specific parameter | ||
| catalogs and scenarios are supplied by the consumer. The pytest plugin | ||
| (registered via the ``pytest11`` entry point) also exposes these as ready-to-use | ||
| fixtures, with no ``conftest.py`` wiring required. | ||
|
|
||
| Names are resolved lazily (PEP 562): this package is imported during pytest | ||
| startup when the plugin registers, so eagerly importing the implementation here | ||
| would run its code before coverage instrumentation starts. | ||
| """ |
| would run its code before coverage instrumentation starts. | ||
| """ | ||
|
|
||
| from __future__ import annotations |
There was a problem hiding this comment.
Umm...I believe this import is not needed here because the types imported under TYPE_CHECKING are not used in any type annotations
|
|
||
|
|
||
| def test_plugin_registers_fixtures(pytester: pytest.Pytester) -> None: | ||
| pytester.makepyfile( |
| - [Usage](docs/usage.md) | ||
| - [Architecture](docs/architecture.md) | ||
| - [Technical Design Review](docs/technical-design-review.md) | ||
| - [Contributing](docs/contributing.md) | ||
| - [Testing](docs/testing.md) | ||
| - [Releases](docs/releases.md) |
There was a problem hiding this comment.
Relative links work on Github but will be broken on pypi. We should add them to the readme property in pyproject.toml files or use absolute urls
| ] | ||
| omit = [ | ||
| "__init__.py", | ||
| "*/plugin.py", |
There was a problem hiding this comment.
is this a mistake or why do we want to exclude this file from the coverage.report?
| ) | ||
|
|
||
|
|
||
| def test_build_a_purchase_order() -> None: |
There was a problem hiding this comment.
Please remove type annotations from the test folder and review all the tests
| def build(self, name: str, **overrides: Any) -> ModelT: | ||
| """Build the named scenario, applying any extra field overrides. | ||
|
|
||
| The spec and ``overrides`` are passed to the factory as field values, so | ||
| a scenario can set any model field (``status``, ``type``, ``lines``, | ||
| ``subscriptions``, ``template``, ``parameters``, ...). | ||
| """ | ||
| return self._factory.build(**{**self._registry[name], **overrides}) |
There was a problem hiding this comment.
Should we create a deep copy of the self._registry on each build?
2 participants
🤖 AI-generated PR — Please review carefully.
First module for MPT-22111: a shared pytest-fixtures library for MPT-shaped test objects, so extensions stop copy-pasting MPT object factories. Opening it now to show the team the shape and gather feedback before expanding.
What's here
mpt-extension-contrib-fixtures(mpt_extension_contrib.fixtures).ModelFactorysubclasses building typed SDK models with empty parameters:OrderFactory(+purchase/change/terminate/configurationpresets),AgreementFactory,SubscriptionFactory.parameter_bag(ordering=…, fulfillment=…)— product parameter catalogs stay in the consumer.Scenarios(factory, {...})— one registry object for any entity:.build(name),.cases(),.fixture().mpt_error_factory(...)— MPT API error payloads.pytest11entry point (fixtures available on install, noconftestwiring).Design notes for review
mpt_extension_sdk.models..to_dict()bridges to the API JSON shape, so dict-based suites can adopt incrementally and API responses can be mocked at the wire.mpt_api_clientreturns dynamic models; SDK services always wrap them into pydantic viafrom_payload. The library standardises on the SDK side — seedocs/architecture.md.sys.monitoring); the scoped lint ignores and*/plugin.pycoverage omit are documented indocs/architecture.md.Scope / roadmap (intentionally not in this PR)
ORD-…/AGR-…/LCE-…), more factories (Asset, Account/Buyer/Seller/Licensee, Product/Item, Template, Authorization, Price,ParameterValueFactory), Event fixtures, and API-response/respxmock helpers.OrderType/OrderStatusenums.Validation
make check-all pkg=fixturesis green — 29 tests, 100% coverage, ruff / flake8 (wemake) / mypy clean.🤖 Generated with Claude Code
Closes MPT-22111
fixturescontrib modulempt_extension_contrib.fixtures, exposed as an installablepytest11plugin (mpt_extension_contrib_fixtures) so fixtures are available withoutconftest.py.OrderFactorywithpurchase,change,terminate, andconfigurationpresets, plusAgreementFactoryandSubscriptionFactory(defaulting to structurally valid models with empty parameters/catalogs).parameter_bag(...)for parameter injection from consumer catalogs,Scenarios(...)registry for named scenario-driven test parametrization (cases()/fixture()), andmpt_error_factory(...)for standardized MPT API error payload dicts..to_dict()documented as the API-shaped JSON bridge for payload generation/mocking.__getattr__) and a coverage-friendly pytest plugin using lazy imports at fixture invocation time.fixturespackage in checks/tests and adjusted coverage/lint/type-check settings to account for plugin startup/import timing.