Skip to content

Commit 62fcf1b

Browse files
mgornymgorny
authored
ProviderInfo: Update validation to disallow plugin_api w/o requires (#122)
Remove the obsolete assumption that `plugin_api` could be specified instead of `requires`. Update tests for that. Signed-off-by: Michał Górny <mgorny@quansight.com>
1 parent 9e335f8 commit 62fcf1b

5 files changed

Lines changed: 37 additions & 22 deletions

File tree

‎tests/plugins/test_loader.py‎

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
from variantlib.constants import VARIANT_INFO_NAMESPACE_KEY
1414
from variantlib.constants import VARIANT_INFO_PROVIDER_DATA_KEY
1515
from variantlib.constants import VARIANT_INFO_PROVIDER_PLUGIN_API_KEY
16+
from variantlib.constants import VARIANT_INFO_PROVIDER_REQUIRES_KEY
1617
from variantlib.constants import VARIANTS_JSON_VARIANT_DATA_KEY
1718
from variantlib.errors import PluginError
1819
from variantlib.errors import ValidationError
@@ -364,19 +365,23 @@ def test_load_plugin_invalid_arg() -> None:
364365
],
365366
providers={
366367
"test_namespace": ProviderInfo(
367-
plugin_api="tests.mocked_plugins:MockedPluginA"
368+
requires=["variantlib"],
369+
plugin_api="tests.mocked_plugins:MockedPluginA",
368370
),
369371
"second_namespace": ProviderInfo(
370372
# always true
371373
enable_if="python_version >= '3.10'",
374+
requires=["variantlib"],
372375
plugin_api="tests.mocked_plugins:MockedPluginB",
373376
),
374377
"incompatible_namespace": ProviderInfo(
375378
# always false (hopefully)
376379
enable_if='platform_machine == "frobnicator"',
380+
requires=["variantlib"],
377381
plugin_api="tests.mocked_plugins:MockedPluginC",
378382
),
379383
"one_more": ProviderInfo(
384+
requires=["variantlib"],
380385
plugin_api="tests.mocked_plugins:NoSuchClass",
381386
optional=True,
382387
),
@@ -388,9 +393,11 @@ def test_load_plugin_invalid_arg() -> None:
388393
{VARIANT_INFO_NAMESPACE_KEY} = ["test_namespace", "second_namespace"]
389394
390395
[{PYPROJECT_TOML_TOP_KEY}.{VARIANT_INFO_PROVIDER_DATA_KEY}.test_namespace]
396+
{VARIANT_INFO_PROVIDER_REQUIRES_KEY} = ["variantlib"]
391397
{VARIANT_INFO_PROVIDER_PLUGIN_API_KEY} = "tests.mocked_plugins:MockedPluginA"
392398
393399
[{PYPROJECT_TOML_TOP_KEY}.{VARIANT_INFO_PROVIDER_DATA_KEY}.second_namespace]
400+
{VARIANT_INFO_PROVIDER_REQUIRES_KEY} = ["variantlib"]
394401
{VARIANT_INFO_PROVIDER_PLUGIN_API_KEY} = "tests.mocked_plugins:MockedPluginB"
395402
""")
396403
),
@@ -401,10 +408,12 @@ def test_load_plugin_invalid_arg() -> None:
401408
},
402409
VARIANT_INFO_PROVIDER_DATA_KEY: {
403410
"test_namespace": {
404-
VARIANT_INFO_PROVIDER_PLUGIN_API_KEY: "tests.mocked_plugins:MockedPluginA" # noqa: E501
411+
VARIANT_INFO_PROVIDER_REQUIRES_KEY: ["variantlib"],
412+
VARIANT_INFO_PROVIDER_PLUGIN_API_KEY: "tests.mocked_plugins:MockedPluginA", # noqa: E501
405413
},
406414
"second_namespace": {
407-
VARIANT_INFO_PROVIDER_PLUGIN_API_KEY: "tests.mocked_plugins:MockedPluginB" # noqa: E501
415+
VARIANT_INFO_PROVIDER_REQUIRES_KEY: ["variantlib"],
416+
VARIANT_INFO_PROVIDER_PLUGIN_API_KEY: "tests.mocked_plugins:MockedPluginB", # noqa: E501
408417
},
409418
},
410419
VARIANTS_JSON_VARIANT_DATA_KEY: {},
@@ -486,10 +495,12 @@ def test_optional_plugins(value: bool | list[VariantNamespace], expected: bool)
486495
],
487496
providers={
488497
"test_namespace": ProviderInfo(
489-
plugin_api="tests.mocked_plugins:MockedPluginA"
498+
requires=["variantlib"], plugin_api="tests.mocked_plugins:MockedPluginA"
490499
),
491500
"second_namespace": ProviderInfo(
492-
plugin_api="tests.mocked_plugins:MockedPluginB", optional=True
501+
requires=["variantlib"],
502+
plugin_api="tests.mocked_plugins:MockedPluginB",
503+
optional=True,
493504
),
494505
},
495506
)
@@ -534,10 +545,10 @@ def test_filter_plugins(value: list[VariantNamespace]) -> None:
534545
],
535546
providers={
536547
"test_namespace": ProviderInfo(
537-
plugin_api="tests.mocked_plugins:MockedPluginA"
548+
requires=["variantlib"], plugin_api="tests.mocked_plugins:MockedPluginA"
538549
),
539550
"second_namespace": ProviderInfo(
540-
plugin_api="tests.mocked_plugins:MockedPluginB"
551+
requires=["variantlib"], plugin_api="tests.mocked_plugins:MockedPluginB"
541552
),
542553
},
543554
)
@@ -580,9 +591,10 @@ def test_package_defined_properties(include_build_plugins: bool) -> None:
580591
},
581592
providers={
582593
"test_namespace": ProviderInfo(
583-
plugin_api="tests.mocked_plugins:MockedPluginA"
594+
requires=["variantlib"], plugin_api="tests.mocked_plugins:MockedPluginA"
584595
),
585596
"second_namespace": ProviderInfo(
597+
requires=["variantlib"],
586598
plugin_api="tests.mocked_plugins:MockedPluginB",
587599
plugin_use=PluginUse.BUILD,
588600
),

‎tests/test_api.py‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ def test_get_variants_by_priority_roundtrip(
115115
VARIANT_INFO_PROVIDER_DATA_KEY: {
116116
namespace: {
117117
VARIANT_INFO_PROVIDER_PLUGIN_API_KEY: plugin_api,
118-
VARIANT_INFO_PROVIDER_REQUIRES_KEY: [],
118+
VARIANT_INFO_PROVIDER_REQUIRES_KEY: ["variantlib"],
119119
}
120120
for namespace, plugin_api in plugin_apis.items()
121121
},
@@ -301,15 +301,18 @@ def test_validate_variant(optional: bool) -> None:
301301
property_priorities={"private": {"build_type": ["debug", "release"]}},
302302
providers={
303303
"test_namespace": ProviderInfo(
304+
requires=["variantlib"],
304305
plugin_api="tests.mocked_plugins:MockedPluginA",
305306
optional=optional,
306307
),
307308
"second_namespace": ProviderInfo(
309+
requires=["variantlib"],
308310
plugin_api="tests.mocked_plugins:MockedPluginB",
309311
optional=optional,
310312
plugin_use=PluginUse.BUILD,
311313
),
312314
"incompatible_namespace": ProviderInfo(
315+
requires=["variantlib"],
313316
plugin_api="tests.mocked_plugins:MockedPluginC",
314317
optional=optional,
315318
),
@@ -453,10 +456,10 @@ def common_variant_info() -> VariantInfo:
453456
namespace_priorities=["test_namespace", "second_namespace"],
454457
providers={
455458
"test_namespace": ProviderInfo(
456-
plugin_api="tests.mocked_plugins:MockedPluginA"
459+
requires=["variantlib"], plugin_api="tests.mocked_plugins:MockedPluginA"
457460
),
458461
"second_namespace": ProviderInfo(
459-
plugin_api="tests.mocked_plugins:MockedPluginB"
462+
requires=["variantlib"], plugin_api="tests.mocked_plugins:MockedPluginB"
460463
),
461464
},
462465
)
@@ -499,10 +502,10 @@ def test_check_variant_supported_generic() -> None:
499502
namespace_priorities=["test_namespace", "second_namespace"],
500503
providers={
501504
"test_namespace": ProviderInfo(
502-
plugin_api="tests.mocked_plugins:MockedPluginA"
505+
requires=["variantlib"], plugin_api="tests.mocked_plugins:MockedPluginA"
503506
),
504507
"second_namespace": ProviderInfo(
505-
plugin_api="tests.mocked_plugins:MockedPluginB"
508+
requires=["variantlib"], plugin_api="tests.mocked_plugins:MockedPluginB"
506509
),
507510
},
508511
)
@@ -696,6 +699,7 @@ def test_make_variant_dist_info_expand_build_plugin_properties(
696699
namespace_priorities=["test_namespace"],
697700
providers={
698701
"test_namespace": ProviderInfo(
702+
requires=["variantlib"],
699703
plugin_api=plugin_api,
700704
optional=True,
701705
plugin_use=plugin_use,
@@ -710,6 +714,7 @@ def test_make_variant_dist_info_expand_build_plugin_properties(
710714
},
711715
VARIANT_INFO_PROVIDER_DATA_KEY: {
712716
"test_namespace": {
717+
VARIANT_INFO_PROVIDER_REQUIRES_KEY: ["variantlib"],
713718
VARIANT_INFO_PROVIDER_OPTIONAL_KEY: True,
714719
VARIANT_INFO_PROVIDER_PLUGIN_API_KEY: plugin_api,
715720
},
@@ -765,6 +770,7 @@ def test_make_variant_dist_info_invalid_build_plugin() -> None:
765770
namespace_priorities=["test_namespace"],
766771
providers={
767772
"test_namespace": ProviderInfo(
773+
requires=["variantlib"],
768774
plugin_api=plugin_api,
769775
optional=True,
770776
plugin_use=PluginUse.BUILD,
@@ -796,6 +802,7 @@ def test_make_variant_dist_info_really_invalid_build_plugin() -> None:
796802
namespace_priorities=["second_namespace"],
797803
providers={
798804
"second_namespace": ProviderInfo(
805+
requires=["variantlib"],
799806
plugin_api=plugin_api,
800807
plugin_use=PluginUse.BUILD,
801808
)

‎tests/test_pyproject_toml.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,7 @@ def test_extra_provider_data_key() -> None:
393393
VARIANT_INFO_PROVIDER_DATA_KEY: {
394394
"ns": {
395395
VARIANT_INFO_PROVIDER_PLUGIN_API_KEY: "frobnicate:Plugin",
396+
VARIANT_INFO_PROVIDER_REQUIRES_KEY: ["foo"],
396397
"foo": {},
397398
}
398399
}
@@ -410,7 +411,7 @@ def test_conversion(cls: type[VariantPyProjectToml | VariantsJson]) -> None:
410411
pyproj.namespace_priorities.append("ns4")
411412
pyproj.feature_priorities["ns4"] = ["foo"]
412413
pyproj.property_priorities["ns2"]["foo"] = ["bar"]
413-
pyproj.providers["ns4"] = ProviderInfo(plugin_api="foo:bar")
414+
pyproj.providers["ns4"] = ProviderInfo(requires=["foo"], plugin_api="foo:bar")
414415
pyproj.providers["ns1"].enable_if = None
415416
pyproj.providers["ns2"].requires.append("frobnicate")
416417

‎tests/test_variants_json.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ def test_conversion(cls: type[VariantPyProjectToml | VariantsJson]) -> None:
253253
variants_json.namespace_priorities.append("ns")
254254
variants_json.feature_priorities["ns"] = ["foo"]
255255
variants_json.property_priorities["fictional_tech"]["foo"] = ["bar"]
256-
variants_json.providers["ns"] = ProviderInfo(plugin_api="foo:bar")
256+
variants_json.providers["ns"] = ProviderInfo(requires=["bar"], plugin_api="foo:bar")
257257
variants_json.providers["fictional_hw"].enable_if = None
258258
variants_json.providers["fictional_tech"].requires.append("frobnicate")
259259

‎variantlib/models/variant_info.py‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -50,14 +50,9 @@ class ProviderInfo:
5050
requires: list[str] = field(default_factory=list)
5151

5252
def __post_init__(self) -> None:
53-
if (
54-
self.plugin_use != PluginUse.NONE
55-
and self.plugin_api is None
56-
and not self.requires
57-
):
53+
if self.plugin_use != PluginUse.NONE and not self.requires:
5854
raise ValidationError(
59-
"Either plugin-api or requires need to be specified when "
60-
f"plugin-use != '{PluginUse.NONE}'"
55+
f"requires need to be specified when plugin-use != '{PluginUse.NONE}'"
6156
)
6257

6358
@property

0 commit comments

Comments
 (0)