feat(project): add ProjectSpec hub for markers and PluginManager - #10
Open
RonnyPfannschmidt wants to merge 1 commit into
Open
feat(project): add ProjectSpec hub for markers and PluginManager#10RonnyPfannschmidt wants to merge 1 commit into
RonnyPfannschmidt wants to merge 1 commit into
Conversation
Reviewer's GuideIntroduce a ProjectSpec configuration hub that centralizes project name, hook markers, and plugin manager creation, and update existing markers and PluginManager to accept either a string project name or a ProjectSpec instance while keeping string-based usage fully backward compatible. Sequence diagram for using ProjectSpec to configure markers and PluginManagersequenceDiagram
actor User
participant ProjectSpec
participant HookspecMarker
participant PluginManager
User->>ProjectSpec: __init__(project_name)
ProjectSpec->>ProjectSpec: create hookspec and hookimpl
User->>ProjectSpec: hookspec
ProjectSpec->>HookspecMarker: __init__(ProjectSpec)
HookspecMarker-->>User: decorator
User->>ProjectSpec: create_plugin_manager()
ProjectSpec->>PluginManager: __init__(ProjectSpec)
PluginManager-->>User: PluginManager instance
User->>PluginManager: add_hookspecs(decorated_function)
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
This was referenced Jul 24, 2026
refactor(caller): Protocol HookCaller, split callers, CompletionHook multicall
pytest-dev/pluggy#708
Draft
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The tests and some internal logic rely on accessing
_project_specdirectly on markers and the plugin manager; consider exposing a publicproject_specattribute/property instead so callers don’t depend on private implementation details. - The conversion from
str | ProjectSpecto aProjectSpecinstance is duplicated inHookspecMarker,HookimplMarker, andPluginManager; extracting a small helper (e.g.ensure_project_spec(...)) would reduce repetition and keep future changes to that behavior centralized.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The tests and some internal logic rely on accessing `_project_spec` directly on markers and the plugin manager; consider exposing a public `project_spec` attribute/property instead so callers don’t depend on private implementation details.
- The conversion from `str | ProjectSpec` to a `ProjectSpec` instance is duplicated in `HookspecMarker`, `HookimplMarker`, and `PluginManager`; extracting a small helper (e.g. `ensure_project_spec(...)`) would reduce repetition and keep future changes to that behavior centralized.
## Individual Comments
### Comment 1
<location path="testing/test_project_spec.py" line_range="139-150" />
<code_context>
+ assert project.get_hookimpl_config(undecorated) is None
+
+
+def test_marker_classes_accept_project_spec() -> None:
+ project = ProjectSpec("testproject")
+
+ hookspec_from_project = HookspecMarker(project)
+ hookimpl_from_project = HookimplMarker(project)
+
+ assert hookspec_from_project.project_name == "testproject"
+ assert hookimpl_from_project.project_name == "testproject"
+ assert hookspec_from_project._project_spec is project
+ assert hookimpl_from_project._project_spec is project
+
+
</code_context>
<issue_to_address>
**suggestion (testing):** Add tests for interoperability between string-based markers and ProjectSpec-based PluginManager (and vice versa)
The current tests only cover all-ProjectSpec and all-string flows. Please also add mixed-type cases, e.g.:
- `HookspecMarker("testproject")` / `HookimplMarker("testproject")` with `PluginManager(ProjectSpec("testproject"))`.
- `HookspecMarker(ProjectSpec("testproject"))` / `HookimplMarker(ProjectSpec("testproject"))` with `PluginManager("testproject")`.
These will exercise the `ProjectSpec` wrapping logic and help catch regressions when mixing marker and manager input types.
```suggestion
def test_marker_classes_accept_project_spec() -> None:
project = ProjectSpec("testproject")
hookspec_from_project = HookspecMarker(project)
hookimpl_from_project = HookimplMarker(project)
assert hookspec_from_project.project_name == "testproject"
assert hookimpl_from_project.project_name == "testproject"
assert hookspec_from_project._project_spec is project
assert hookimpl_from_project._project_spec is project
def test_string_markers_with_project_spec_plugin_manager() -> None:
pm = PluginManager(ProjectSpec("testproject"))
hookspec = HookspecMarker("testproject")
hookimpl = HookimplMarker("testproject")
class Spec:
@hookspec
def hello(self) -> str:
"""A simple hook spec."""
class Impl:
@hookimpl
def hello(self) -> str:
return "world"
pm.add_hookspecs(Spec)
pm.register(Impl())
assert pm.project_name == "testproject"
assert pm._project_spec is not None
assert pm._project_spec.project_name == "testproject"
assert pm.hook.hello() == ["world"]
def test_project_spec_markers_with_string_plugin_manager() -> None:
pm = PluginManager("testproject")
hookspec = HookspecMarker(ProjectSpec("testproject"))
hookimpl = HookimplMarker(ProjectSpec("testproject"))
class Spec:
@hookspec
def hello(self) -> str:
"""A simple hook spec."""
class Impl:
@hookimpl
def hello(self) -> str:
return "world"
pm.add_hookspecs(Spec)
pm.register(Impl())
assert pm.project_name == "testproject"
assert pm._project_spec is not None
assert pm._project_spec.project_name == "testproject"
assert pm.hook.hello() == ["world"]
```
</issue_to_address>
### Comment 2
<location path="testing/test_project_spec.py" line_range="37-48" />
<code_context>
+ assert isinstance(pm2, PluginManager)
+
+
+def test_project_spec_custom_plugin_manager_class() -> None:
+ class CustomPluginManager(PluginManager):
+ def __init__(self, project_name: str | ProjectSpec) -> None:
+ super().__init__(project_name)
+ self.custom_attr = "custom_value"
+
+ project = ProjectSpec("testproject", plugin_manager_cls=CustomPluginManager)
+ pm = project.create_plugin_manager()
+
+ assert isinstance(pm, CustomPluginManager)
+ assert pm.project_name == "testproject"
+ assert pm.custom_attr == "custom_value"
+
+
</code_context>
<issue_to_address>
**suggestion (testing):** Strengthen tests to assert that custom PluginManager instances are independent across multiple calls
The `ProjectSpec` docstring guarantees that `create_plugin_manager()` returns fresh, independent instances. For the custom `PluginManager` case, this test currently only validates type and attributes for a single instance. Please also verify independence by, for example:
- Calling `create_plugin_manager()` twice with `CustomPluginManager` and asserting `pm1 is not pm2`.
- Optionally registering different plugins on each and asserting they remain isolated.
This would mirror `test_project_spec_multiple_plugin_managers_independent()` and ensure the independence contract holds when `plugin_manager_cls` is customized.
```suggestion
def test_project_spec_custom_plugin_manager_class() -> None:
class CustomPluginManager(PluginManager):
def __init__(self, project_name: str | ProjectSpec) -> None:
super().__init__(project_name)
self.custom_attr = "custom_value"
project = ProjectSpec("testproject", plugin_manager_cls=CustomPluginManager)
pm1 = project.create_plugin_manager()
pm2 = project.create_plugin_manager()
# type and attribute guarantees
assert isinstance(pm1, CustomPluginManager)
assert isinstance(pm2, CustomPluginManager)
assert pm1.project_name == "testproject"
assert pm2.project_name == "testproject"
assert pm1.custom_attr == "custom_value"
assert pm2.custom_attr == "custom_value"
# independence guarantees: fresh instances on each call
assert pm1 is not pm2
# isolation guarantees: plugins registered on each manager remain independent
plugin1 = object()
plugin2 = object()
pm1.register(plugin1)
pm2.register(plugin2)
plugins1 = set(pm1.get_plugins())
plugins2 = set(pm2.get_plugins())
assert plugin1 in plugins1
assert plugin1 not in plugins2
assert plugin2 in plugins2
assert plugin2 not in plugins1
```
</issue_to_address>
### Comment 3
<location path="changelog/708.feature.rst" line_range="1" />
<code_context>
+New :class:`pluggy.ProjectSpec` hub bundles a project's
+:class:`~pluggy.HookspecMarker`, :class:`~pluggy.HookimplMarker` and
+:meth:`~pluggy.ProjectSpec.create_plugin_manager` under one project name.
</code_context>
<issue_to_address>
**suggestion (typo):** Consider adding an article before "New" for smoother grammar (e.g., "A new" or "The new").
The current phrasing (“New :class:`pluggy.ProjectSpec` hub bundles…”) is grammatically abrupt. Starting with “A new …” or “The new …” would read more naturally while preserving the meaning.
```suggestion
A new :class:`pluggy.ProjectSpec` hub bundles a project's
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+139
to
+150
| def test_marker_classes_accept_project_spec() -> None: | ||
| project = ProjectSpec("testproject") | ||
|
|
||
| hookspec_from_project = HookspecMarker(project) | ||
| hookimpl_from_project = HookimplMarker(project) | ||
|
|
||
| assert hookspec_from_project.project_name == "testproject" | ||
| assert hookimpl_from_project.project_name == "testproject" | ||
| assert hookspec_from_project._project_spec is project | ||
| assert hookimpl_from_project._project_spec is project | ||
|
|
||
|
|
There was a problem hiding this comment.
suggestion (testing): Add tests for interoperability between string-based markers and ProjectSpec-based PluginManager (and vice versa)
The current tests only cover all-ProjectSpec and all-string flows. Please also add mixed-type cases, e.g.:
HookspecMarker("testproject")/HookimplMarker("testproject")withPluginManager(ProjectSpec("testproject")).HookspecMarker(ProjectSpec("testproject"))/HookimplMarker(ProjectSpec("testproject"))withPluginManager("testproject").
These will exercise the ProjectSpec wrapping logic and help catch regressions when mixing marker and manager input types.
Suggested change
| def test_marker_classes_accept_project_spec() -> None: | |
| project = ProjectSpec("testproject") | |
| hookspec_from_project = HookspecMarker(project) | |
| hookimpl_from_project = HookimplMarker(project) | |
| assert hookspec_from_project.project_name == "testproject" | |
| assert hookimpl_from_project.project_name == "testproject" | |
| assert hookspec_from_project._project_spec is project | |
| assert hookimpl_from_project._project_spec is project | |
| def test_marker_classes_accept_project_spec() -> None: | |
| project = ProjectSpec("testproject") | |
| hookspec_from_project = HookspecMarker(project) | |
| hookimpl_from_project = HookimplMarker(project) | |
| assert hookspec_from_project.project_name == "testproject" | |
| assert hookimpl_from_project.project_name == "testproject" | |
| assert hookspec_from_project._project_spec is project | |
| assert hookimpl_from_project._project_spec is project | |
| def test_string_markers_with_project_spec_plugin_manager() -> None: | |
| pm = PluginManager(ProjectSpec("testproject")) | |
| hookspec = HookspecMarker("testproject") | |
| hookimpl = HookimplMarker("testproject") | |
| class Spec: | |
| @hookspec | |
| def hello(self) -> str: | |
| """A simple hook spec.""" | |
| class Impl: | |
| @hookimpl | |
| def hello(self) -> str: | |
| return "world" | |
| pm.add_hookspecs(Spec) | |
| pm.register(Impl()) | |
| assert pm.project_name == "testproject" | |
| assert pm._project_spec is not None | |
| assert pm._project_spec.project_name == "testproject" | |
| assert pm.hook.hello() == ["world"] | |
| def test_project_spec_markers_with_string_plugin_manager() -> None: | |
| pm = PluginManager("testproject") | |
| hookspec = HookspecMarker(ProjectSpec("testproject")) | |
| hookimpl = HookimplMarker(ProjectSpec("testproject")) | |
| class Spec: | |
| @hookspec | |
| def hello(self) -> str: | |
| """A simple hook spec.""" | |
| class Impl: | |
| @hookimpl | |
| def hello(self) -> str: | |
| return "world" | |
| pm.add_hookspecs(Spec) | |
| pm.register(Impl()) | |
| assert pm.project_name == "testproject" | |
| assert pm._project_spec is not None | |
| assert pm._project_spec.project_name == "testproject" | |
| assert pm.hook.hello() == ["world"] |
Comment on lines
+37
to
+48
| def test_project_spec_custom_plugin_manager_class() -> None: | ||
| class CustomPluginManager(PluginManager): | ||
| def __init__(self, project_name: str | ProjectSpec) -> None: | ||
| super().__init__(project_name) | ||
| self.custom_attr = "custom_value" | ||
|
|
||
| project = ProjectSpec("testproject", plugin_manager_cls=CustomPluginManager) | ||
| pm = project.create_plugin_manager() | ||
|
|
||
| assert isinstance(pm, CustomPluginManager) | ||
| assert pm.project_name == "testproject" | ||
| assert pm.custom_attr == "custom_value" |
There was a problem hiding this comment.
suggestion (testing): Strengthen tests to assert that custom PluginManager instances are independent across multiple calls
The ProjectSpec docstring guarantees that create_plugin_manager() returns fresh, independent instances. For the custom PluginManager case, this test currently only validates type and attributes for a single instance. Please also verify independence by, for example:
- Calling
create_plugin_manager()twice withCustomPluginManagerand assertingpm1 is not pm2. - Optionally registering different plugins on each and asserting they remain isolated.
This would mirror test_project_spec_multiple_plugin_managers_independent() and ensure the independence contract holds when plugin_manager_cls is customized.
Suggested change
| def test_project_spec_custom_plugin_manager_class() -> None: | |
| class CustomPluginManager(PluginManager): | |
| def __init__(self, project_name: str | ProjectSpec) -> None: | |
| super().__init__(project_name) | |
| self.custom_attr = "custom_value" | |
| project = ProjectSpec("testproject", plugin_manager_cls=CustomPluginManager) | |
| pm = project.create_plugin_manager() | |
| assert isinstance(pm, CustomPluginManager) | |
| assert pm.project_name == "testproject" | |
| assert pm.custom_attr == "custom_value" | |
| def test_project_spec_custom_plugin_manager_class() -> None: | |
| class CustomPluginManager(PluginManager): | |
| def __init__(self, project_name: str | ProjectSpec) -> None: | |
| super().__init__(project_name) | |
| self.custom_attr = "custom_value" | |
| project = ProjectSpec("testproject", plugin_manager_cls=CustomPluginManager) | |
| pm1 = project.create_plugin_manager() | |
| pm2 = project.create_plugin_manager() | |
| # type and attribute guarantees | |
| assert isinstance(pm1, CustomPluginManager) | |
| assert isinstance(pm2, CustomPluginManager) | |
| assert pm1.project_name == "testproject" | |
| assert pm2.project_name == "testproject" | |
| assert pm1.custom_attr == "custom_value" | |
| assert pm2.custom_attr == "custom_value" | |
| # independence guarantees: fresh instances on each call | |
| assert pm1 is not pm2 | |
| # isolation guarantees: plugins registered on each manager remain independent | |
| plugin1 = object() | |
| plugin2 = object() | |
| pm1.register(plugin1) | |
| pm2.register(plugin2) | |
| plugins1 = set(pm1.get_plugins()) | |
| plugins2 = set(pm2.get_plugins()) | |
| assert plugin1 in plugins1 | |
| assert plugin1 not in plugins2 | |
| assert plugin2 in plugins2 | |
| assert plugin2 not in plugins1 |
RonnyPfannschmidt
force-pushed
the
refactor/hookcaller-and-execution
branch
from
July 24, 2026 16:38
a2063ae to
ea1ab94
Compare
RonnyPfannschmidt
force-pushed
the
refactor/project-spec
branch
from
July 24, 2026 16:38
dec8c94 to
3ce85b3
Compare
RonnyPfannschmidt
force-pushed
the
refactor/hookcaller-and-execution
branch
from
July 24, 2026 16:54
ea1ab94 to
0bd7b6b
Compare
RonnyPfannschmidt
force-pushed
the
refactor/project-spec
branch
from
July 24, 2026 16:54
3ce85b3 to
d6217b7
Compare
RonnyPfannschmidt
force-pushed
the
refactor/hookcaller-and-execution
branch
from
August 12, 2026 10:50
0bd7b6b to
9e5b222
Compare
RonnyPfannschmidt
force-pushed
the
refactor/project-spec
branch
from
August 12, 2026 10:50
d6217b7 to
106949c
Compare
RonnyPfannschmidt
force-pushed
the
refactor/hookcaller-and-execution
branch
from
August 12, 2026 10:57
9e5b222 to
15e5147
Compare
RonnyPfannschmidt
force-pushed
the
refactor/project-spec
branch
from
August 12, 2026 10:57
106949c to
b74f215
Compare
Complete design step 06: ProjectSpec bundles hookspec/hookimpl markers and plugin manager creation under a single project name, with get_hookspec_config / get_hookimpl_config helpers. Markers and PluginManager accept str | ProjectSpec (strings still work; markers and manager now expose project_name as a property delegating to the spec). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
RonnyPfannschmidt
force-pushed
the
refactor/hookcaller-and-execution
branch
from
August 12, 2026 11:14
15e5147 to
dce483a
Compare
RonnyPfannschmidt
force-pushed
the
refactor/project-spec
branch
from
August 12, 2026 11:14
b74f215 to
95512c2
Compare
This was referenced Aug 12, 2026
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
1 participant
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.
Review PR — step 6 of 7.
This PR targets the previous step's branch, so its diff is only this step's change. Review happens here. The corresponding upstream PR, which is the one that actually merges, is pytest-dev#709.
Merges happen upstream one step at a time, bottom-up. When step 6 lands upstream, this PR is closed and the rest of the stack is rebased onto the new
main.refactor/split-hook-modulesrefactor/configuration-objectsrefactor/markers-attach-configrefactor/hookimpl-wrapper-typesrefactor/hookcaller-and-executionrefactor/project-specrefactor/async-submitterChain step 06 of the internal-refactoring series (design/06-project-spec.md).
ProjectSpec bundles hookspec/hookimpl markers and plugin manager creation under one project name; markers and PluginManager accept str | ProjectSpec (strings keep working).
Stacked on the step-05 chain PR.
🤖 Generated with Claude Code
Summary by Sourcery
Add a ProjectSpec abstraction that unifies project-level hook markers and plugin manager creation while keeping existing string-based APIs compatible.
New Features:
Enhancements:
Documentation:
Tests: