fix(models/project): make no parts error explicit (#2073)

canonical · Jan 14, 2025 · a93a2d9 · a93a2d9
1 parent a1a3d4b
commit a93a2d9
Show file tree

Hide file tree

Showing 12 changed files with 171 additions and 50 deletions.
diff --git a/charmcraft/models/project.py b/charmcraft/models/project.py
@@ -1,4 +1,4 @@
-# Copyright 2023-2024 Canonical Ltd.
+# Copyright 2023,2025 Canonical Ltd.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -420,7 +420,6 @@ class CharmcraftProject(models.Project, metaclass=abc.ABCMeta):
             f"and ${const.STORE_REGISTRY_ENV_VAR} environment variables instead."
         ),
     )
-    parts: dict[str, dict[str, Any]] = pydantic.Field(default_factory=dict)
 
     # Default project properties that Charmcraft currently does not use. Types are set
     # to be Optional[None], preventing them from being used, but allow them to be used
@@ -593,29 +592,6 @@ class CharmProject(CharmcraftProject):
         description="A multi-line summary of your charm."
     )
 
-    parts: dict[str, dict[str, Any]] = pydantic.Field(
-        default={"charm": {"plugin": "charm", "source": "."}},
-        description=textwrap.dedent(
-            """\
-            Configures the various mechanisms to obtain, process and prepare data from
-            different sources that end up being a part of the final charm.
-
-            Keys are user-defined part names. The value of each key is a map where keys
-            are part names. Charmcraft provides 3 plugins: charm, bundle, reactive.
-
-            Example::
-
-                parts:
-                  libs:
-                    plugin: dump
-                    source: /usr/local/lib/
-                    organize:
-                      "libxxx.so*": lib/
-                    prime:
-                      - lib/""",
-        ),
-    )
-
     actions: dict[str, Any] | None = pydantic.Field(
         default=None,
         description=textwrap.dedent(
@@ -1066,6 +1042,29 @@ class BasesCharm(CharmProject):
 
     base: None = None
 
+    parts: dict[str, dict[str, Any]] = pydantic.Field(
+        default={"charm": {"plugin": "charm", "source": "."}},
+        description=textwrap.dedent(
+            """\
+            Configures the various mechanisms to obtain, process and prepare data from
+            different sources that end up being a part of the final charm.
+
+            Keys are user-defined part names. The value of each key is a map where keys
+            are part names. Charmcraft provides 3 plugins: charm, bundle, reactive.
+
+            Example::
+
+                parts:
+                  libs:
+                    plugin: dump
+                    source: /usr/local/lib/
+                    organize:
+                      "libxxx.so*": lib/
+                    prime:
+                      - lib/""",
+        ),
+    )
+
 
 class PlatformCharm(CharmProject):
     """Model for defining a charm using Platforms."""
@@ -1075,6 +1074,29 @@ class PlatformCharm(CharmProject):
     build_base: BuildBaseStr | None = None
     platforms: dict[str, models.Platform | None]  # type: ignore[assignment]
 
+    parts: dict[str, dict[str, Any]] = pydantic.Field(
+        description=textwrap.dedent(
+            """\
+            Configures the various mechanisms to obtain, process and prepare data from
+            different sources that end up being a part of the final charm.
+
+            Keys are user-defined part names. The value of each key is a map where keys
+            are part names. Charmcraft provides 3 plugins: charm, bundle, reactive.
+
+            Example::
+
+                parts:
+                  libs:
+                    plugin: dump
+                    source: /usr/local/lib/
+                    organize:
+                      "libxxx.so*": lib/
+                    prime:
+                      - lib/""",
+        ),
+        min_length=1,
+    )
+
     @pydantic.model_validator(mode="after")
     def _validate_dev_base_needs_build_base(self) -> Self:
         if not self.build_base and self.base in const.DEVEL_BASE_STRINGS:
@@ -1107,6 +1129,10 @@ class Bundle(CharmcraftProject):
     description: pydantic.StrictStr | None = None
     platforms: None = None  # type: ignore[assignment]
 
+    parts: dict[str, dict[str, Any]] = pydantic.Field(
+        default_factory=lambda: {"bundle": {"plugin": "bundle", "source": "."}}
+    )
+
     @pydantic.model_validator(mode="before")
     @classmethod
     def preprocess_bundle(cls, values: dict[str, Any]) -> dict[str, Any]:

diff --git a/tests/integration/commands/test_expand_extensions.py b/tests/integration/commands/test_expand_extensions.py
@@ -1,4 +1,4 @@
-# Copyright 2023 Canonical Ltd.
+# Copyright 2023,2025 Canonical Ltd.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -49,7 +49,7 @@ def fake_extensions(stub_extensions):
 @pytest.mark.parametrize(
     ("charmcraft_yaml", "expected"),
     [
-        (
+        pytest.param(
             dedent(
                 f"""
                 name: test-charm-name
@@ -60,6 +60,9 @@ def fake_extensions(stub_extensions):
                 base: [email protected]
                 platforms:
                   amd64:
+                parts:
+                  my-part:
+                    plugin: nil
                 """
             ),
             dedent(
@@ -70,13 +73,52 @@ def fake_extensions(stub_extensions):
                 base: [email protected]
                 platforms:
                   amd64: null
-                parts: {}
+                parts:
+                  my-part:
+                    plugin: nil
                 type: charm
                 terms:
                 - https://example.com/test
                 """
             ),
-        )
+            id="platforms",
+        ),
+        pytest.param(
+            dedent(
+                f"""
+                name: test-charm-name
+                type: charm
+                summary: test-summary
+                description: test-description
+                extensions: [{TestExtension.name}]
+                bases:
+                  - name: ubuntu
+                    channel: "22.04"
+                """
+            ),
+            dedent(
+                """\
+                name: test-charm-name
+                summary: test-summary
+                description: test-description
+                parts:
+                  charm:
+                    plugin: charm
+                    source: .
+                type: charm
+                terms:
+                - https://example.com/test
+                bases:
+                - build-on:
+                  - name: ubuntu
+                    channel: '22.04'
+                  run-on:
+                  - name: ubuntu
+                    channel: '22.04'
+                """
+            ),
+            id="bases",
+        ),
     ],
 )
 def test_expand_extensions_simple(

diff --git a/tests/integration/commands/test_pack.py b/tests/integration/commands/test_pack.py
@@ -1,4 +1,4 @@
-# Copyright 2024 Canonical Ltd.
+# Copyright 2024-2025 Canonical Ltd.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -97,7 +97,7 @@ def test_build_basic_bundle(monkeypatch, capsys, app, new_path, bundle_yaml, fil
                         "build-for": ["amd64"],
                     }
                 },
-                "parts": {},
+                "parts": {"my-part": {"plugin": "nil"}},
             },
             "ubuntu-22.04-amd64",
             marks=pytest.mark.skipif(
@@ -114,7 +114,7 @@ def test_build_basic_bundle(monkeypatch, capsys, app, new_path, bundle_yaml, fil
                 "description": "A charm for testing",
                 "base": "[email protected]",
                 "platforms": {util.get_host_architecture(): None},
-                "parts": {},
+                "parts": {"my-part": {"plugin": "nil"}},
             },
             util.get_host_architecture(),
             marks=pytest.mark.skipif(
@@ -132,7 +132,7 @@ def test_build_basic_bundle(monkeypatch, capsys, app, new_path, bundle_yaml, fil
                 "base": "[email protected]",
                 "build-base": "ubuntu@devel",
                 "platforms": {util.get_host_architecture(): None},
-                "parts": {},
+                "parts": {"my-part": {"plugin": "nil"}},
             },
             util.get_host_architecture(),
             marks=pytest.mark.skipif(

diff --git a/tests/integration/conftest.py b/tests/integration/conftest.py
@@ -1,4 +1,4 @@
-# Copyright 2024 Canonical Ltd.
+# Copyright 2024-2025 Canonical Ltd.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -51,6 +51,7 @@ def charm_project(
         | {
             "base": f"{distro_id}@{distro_version}",
             "platforms": {util.get_host_architecture(): None},
+            "parts": {"charm": {"plugin": "charm", "source": "."}},
         },
     )
 

diff --git a/tests/integration/sample-charms/actions-included/charmcraft.yaml b/tests/integration/sample-charms/actions-included/charmcraft.yaml
@@ -10,6 +10,7 @@ platforms:
 parts:
   charm:
     plugin: charm
+    source: .
 
 actions:
   pause:

diff --git a/tests/test_infra.py b/tests/test_infra.py
@@ -1,4 +1,4 @@
-# Copyright 2020-2022 Canonical Ltd.
+# Copyright 2020-2022,2025 Canonical Ltd.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -52,7 +52,7 @@ def get_python_filepaths() -> list[str]:
 def test_ensure_copyright() -> None:
     """Check that all non-empty Python files have copyright somewhere in the first 5 lines."""
     issues = []
-    regex = re.compile(r"# Copyright \d{4}(-\d{4})? Canonical Ltd.$")
+    regex = re.compile(r"# Copyright \d{4}([-,]\d{4})* Canonical Ltd.$")
     for filepath in get_python_filepaths():
         if Path(filepath).stat().st_size == 0:
             continue

diff --git a/tests/unit/models/invalid_charms_yaml/platforms-empty-parts.yaml b/tests/unit/models/invalid_charms_yaml/platforms-empty-parts.yaml
@@ -0,0 +1,12 @@
+type: charm
+name: basic-charm
+summary: The most basic valid charm
+description: |
+  The most basic possible valid charmcraft.yaml that doesn't need other files and gets returned to its own value.
+  Note that this means we cannot use short-form bases here because this charm is meant to be rewritable.
+base: [email protected]
+platforms:
+  amd64:
+    build-on: [amd64]
+    build-for: [amd64]
+parts: {}
diff --git a/tests/unit/models/invalid_charms_yaml/platforms-no-parts.yaml b/tests/unit/models/invalid_charms_yaml/platforms-no-parts.yaml
@@ -0,0 +1,11 @@
+type: charm
+name: basic-charm
+summary: The most basic valid charm
+description: |
+  The most basic possible valid charmcraft.yaml that doesn't need other files and gets returned to its own value.
+  Note that this means we cannot use short-form bases here because this charm is meant to be rewritable.
+base: [email protected]
+platforms:
+  amd64:
+    build-on: [amd64]
+    build-for: [amd64]
diff --git a/tests/unit/models/test_project.py b/tests/unit/models/test_project.py
@@ -1,4 +1,4 @@
-# Copyright 2023 Canonical Ltd.
+# Copyright 2023,2025 Canonical Ltd.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -993,6 +993,7 @@ def test_from_yaml_file_exception(
                 "description": "",
                 "base": "[email protected]",
                 "platforms": {"amd64": None},
+                "parts": {"my-part": {"plugin": "nil"}},
                 "charmhub": {"api_url": "http://charmhub.io"},
             },
         ),
@@ -1127,19 +1128,16 @@ def test_read_charm_from_yaml_file_self_contained_success(tmp_path, filename: st
                 - field 'name' required in top-level configuration
                 - field 'summary' required in top-level configuration
                 - field 'description' required in top-level configuration
-                - field 'bases' required in top-level configuration"""
+                - field 'platforms' required in top-level configuration
+                - field 'parts' required in top-level configuration"""
             ),
         ),
         (
             "invalid-type.yaml",
             dedent(
                 """\
-                Bad invalid-type.yaml content:
-                - field 'name' required in top-level configuration
-                - field 'summary' required in top-level configuration
-                - field 'description' required in top-level configuration
-                - input should be 'charm' (in field 'type')
-                - field 'bases' required in top-level configuration"""
+                Bad charmcraft.yaml content:
+                - field type cannot be 'invalid'"""
             ),
         ),
         (
@@ -1151,13 +1149,31 @@ def test_read_charm_from_yaml_file_self_contained_success(tmp_path, filename: st
                 - base requires 'platforms' definition: {'name': 'ubuntu', 'channel': 'devel'} (in field 'bases[1]')"""
             ),
         ),
+        pytest.param(
+            "platforms-no-parts.yaml",
+            dedent(
+                """\
+                Bad platforms-no-parts.yaml content:
+                - field 'parts' required in top-level configuration"""
+            ),
+            id="no-parts-in-platform-charm",
+        ),
+        pytest.param(
+            "platforms-empty-parts.yaml",
+            dedent(
+                """\
+                Bad platforms-empty-parts.yaml content:
+                - dictionary should have at least 1 item after validation, not 0 (in field 'parts')"""
+            ),
+            id="empty-parts-in-platform-charm",
+        ),
     ],
 )
 def test_read_charm_from_yaml_file_error(filename, errors):
     file_path = pathlib.Path(__file__).parent / "invalid_charms_yaml" / filename
 
     with pytest.raises(CraftValidationError) as exc:
-        _ = project.BasesCharm.from_yaml_file(file_path)
+        _ = project.CharmcraftProject.from_yaml_file(file_path)
 
     assert exc.value.args[0] == errors
 

diff --git a/tests/unit/models/valid_charms_yaml/basic-platforms.yaml b/tests/unit/models/valid_charms_yaml/basic-platforms.yaml
@@ -10,4 +10,6 @@ platforms:
   amd64:
     build-on: [amd64]
     build-for: [amd64]
-parts: {}
+parts:
+  my-part:
+    plugin: nil
diff --git a/tests/unit/services/test_package.py b/tests/unit/services/test_package.py
@@ -1,4 +1,4 @@
-# Copyright 2023-2024 Canonical Ltd.
+# Copyright 2023-2025 Canonical Ltd.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -404,7 +404,7 @@ def test_get_manifest_bases_from_platforms(
             "base": base,
             "build-base": build_base,
             "platforms": platforms,
-            "parts": {},
+            "parts": {"my-part": {"plugin": "nil"}},
         }
     )
     package_service._project = charm
@@ -427,7 +427,7 @@ def test_get_manifest_bases_from_platforms_invalid(package_service):
             "base": None,
             "build-base": None,
             "platforms": {"amd64": None},
-            "parts": {},
+            "parts": {"my-part": {"plugin": "nil"}},
         }
     )
     package_service._project = charm
-Original file line number
+Diff line change
@@ Expand Up / @@ -10,6 +10,7 @@ platforms: @@
     parts:
       charm:
         plugin: charm
+        source: .
     actions:
       pause:
@@ Expand Down @@