Make the import-surface scan cover symlinked external plugins
Path.rglob does not descend symlinks, so a symlinked external plugin (the ADR-003 dev loop) silently escaped the contract-purity scan. The scanner now resolves plugin dirs before walking, a regression test plants a symlinked plugin with a real violation and asserts it is flagged, and the known-limitation notes in the external-repo docs are lifted. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -266,12 +266,13 @@ manifest parses, the `core_version` range admits the framework's
|
|||||||
contract test run errors and the script exits nonzero), models expose
|
contract test run errors and the script exits nonzero), models expose
|
||||||
`__tablename__`, and hooks return the right shapes.
|
`__tablename__`, and hooks return the right shapes.
|
||||||
|
|
||||||
One gap to know about: the framework's `test_plugins_only_import_contract_surface`
|
The import-surface scan (`test_plugins_only_import_contract_surface`) covers
|
||||||
scans `plugins/` with `Path.rglob`, which does not descend symlinked directories
|
symlinked plugins too: the scanner resolves each plugin directory before
|
||||||
on CPython 3.12. So that particular sub-test does not see a symlinked external
|
walking it, because `Path.rglob` alone does not descend symlinks (pinned by
|
||||||
plugin's source. Keep an equivalent import-surface assertion in your own
|
`test_import_scan_covers_symlinked_plugins`). You can additionally keep an
|
||||||
`tests/` so your CI still enforces "core imports only via `shopdb.api`". A
|
equivalent import-surface assertion in your own `tests/`, so violations fail
|
||||||
minimal version:
|
in your repo's CI even when run without the framework harness. A minimal
|
||||||
|
version:
|
||||||
|
|
||||||
```python
|
```python
|
||||||
# tests/test_import_surface.py
|
# tests/test_import_surface.py
|
||||||
|
|||||||
@@ -32,12 +32,12 @@
|
|||||||
# # Offline: test a bundled plugin against this checkout, no network
|
# # Offline: test a bundled plugin against this checkout, no network
|
||||||
# LOCAL_FRAMEWORK=. PLUGIN_DIR=plugins/warranty scripts/test-external-plugin.sh
|
# LOCAL_FRAMEWORK=. PLUGIN_DIR=plugins/warranty scripts/test-external-plugin.sh
|
||||||
#
|
#
|
||||||
# Note on coverage: a symlinked plugin is discovered and loaded by the plugin
|
# Coverage: a symlinked plugin is discovered and loaded by the plugin loader
|
||||||
# loader (so loadability, manifest validity, and the core_version range are all
|
# (loadability, manifest validity, core_version range) AND scanned by the
|
||||||
# checked), but the import-surface scan in test_plugin_contract.py uses
|
# import-surface contract test - the scanner resolves each plugin dir before
|
||||||
# Path.rglob over plugins/, which does not descend symlinked directories on
|
# walking, since bare Path.rglob does not descend symlinks. Optionally keep an
|
||||||
# CPython 3.12. To have that scan cover your plugin too, keep an equivalent
|
# equivalent import-surface assertion in your own tests/ so violations fail
|
||||||
# import-surface assertion in your own tests/ (see docs/PLUGIN-EXTERNAL-REPO.md).
|
# in your repo's CI too (see docs/PLUGIN-EXTERNAL-REPO.md).
|
||||||
|
|
||||||
set -eu
|
set -eu
|
||||||
|
|
||||||
|
|||||||
@@ -209,8 +209,17 @@ def _plugin_source_files():
|
|||||||
# Skip migrations/ - Alembic env.py is framework glue that legitimately
|
# Skip migrations/ - Alembic env.py is framework glue that legitimately
|
||||||
# calls the shared runner in shopdb.plugins.alembic_template (see ADR-008),
|
# calls the shared runner in shopdb.plugins.alembic_template (see ADR-008),
|
||||||
# not plugin domain code. Mirrors the style check excluding versions/.
|
# not plugin domain code. Mirrors the style check excluding versions/.
|
||||||
return [p for p in root.rglob('*.py')
|
# Resolve each plugin dir before rglob: rglob does not descend symlinked
|
||||||
if '__pycache__' not in p.parts and 'migrations' not in p.parts]
|
# directories, and external plugins arrive as symlinks per ADR-003.
|
||||||
|
files = [p for p in root.glob('*.py')]
|
||||||
|
for plugindir in sorted(root.iterdir()):
|
||||||
|
if not plugindir.is_dir():
|
||||||
|
continue
|
||||||
|
for path in plugindir.resolve().rglob('*.py'):
|
||||||
|
if '__pycache__' in path.parts or 'migrations' in path.parts:
|
||||||
|
continue
|
||||||
|
files.append(path)
|
||||||
|
return files
|
||||||
|
|
||||||
|
|
||||||
def test_plugins_only_import_contract_surface():
|
def test_plugins_only_import_contract_surface():
|
||||||
@@ -231,3 +240,31 @@ def test_plugins_only_import_contract_surface():
|
|||||||
'Plugins must import core only via shopdb.api or shopdb.plugins.base. '
|
'Plugins must import core only via shopdb.api or shopdb.plugins.base. '
|
||||||
'Violations:\n' + '\n'.join(violations)
|
'Violations:\n' + '\n'.join(violations)
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_import_scan_covers_symlinked_plugins(tmp_path):
|
||||||
|
"""External plugins symlinked into plugins/ ARE scanned for violations.
|
||||||
|
|
||||||
|
Path.rglob does not descend symlinked directories, so a naive scan would
|
||||||
|
silently skip external plugins installed per ADR-003 (clone or symlink
|
||||||
|
into plugins/<name>/). The scanner resolves each plugin dir first; this
|
||||||
|
test plants a symlinked plugin with a violating import and asserts the
|
||||||
|
scan both sees the file and flags the violation.
|
||||||
|
"""
|
||||||
|
root = Path(__file__).resolve().parent.parent / 'plugins'
|
||||||
|
external = tmp_path / 'symlinkdemo'
|
||||||
|
external.mkdir()
|
||||||
|
# violating import: internal core path, not the contract surface
|
||||||
|
(external / 'plugin.py').write_text('from shopdb.extensions import db\n')
|
||||||
|
|
||||||
|
link = root / 'zzsymlinkdemo'
|
||||||
|
link.symlink_to(external, target_is_directory=True)
|
||||||
|
try:
|
||||||
|
scanned = {str(p) for p in _plugin_source_files()}
|
||||||
|
assert any('symlinkdemo' in s for s in scanned), (
|
||||||
|
'symlinked plugin sources were not scanned'
|
||||||
|
)
|
||||||
|
with pytest.raises(AssertionError, match='shopdb.extensions'):
|
||||||
|
test_plugins_only_import_contract_surface()
|
||||||
|
finally:
|
||||||
|
link.unlink()
|
||||||
|
|||||||
Reference in New Issue
Block a user