Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 3 commits into from
Jan 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 51 additions & 25 deletions charmcraft/models/project.py
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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."""
Expand All @@ -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:
Expand Down Expand Up @@ -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]:
Expand Down
50 changes: 46 additions & 4 deletions tests/integration/commands/test_expand_extensions.py
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -49,7 +49,7 @@ def fake_extensions(stub_extensions):
@pytest.mark.parametrize(
("charmcraft_yaml", "expected"),
[
(
pytest.param(
dedent(
f"""
name: test-charm-name
Expand All @@ -60,6 +60,9 @@ def fake_extensions(stub_extensions):
base: [email protected]
platforms:
amd64:
parts:
my-part:
plugin: nil
"""
),
dedent(
Expand All @@ -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(
Expand Down
8 changes: 4 additions & 4 deletions tests/integration/commands/test_pack.py
lengau marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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(
Expand Down
3 changes: 2 additions & 1 deletion tests/integration/conftest.py
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -51,6 +51,7 @@ def charm_project(
| {
"base": f"{distro_id}@{distro_version}",
"platforms": {util.get_host_architecture(): None},
"parts": {"charm": {"plugin": "charm", "source": "."}},
},
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ platforms:
parts:
charm:
plugin: charm
source: .

actions:
pause:
Expand Down
4 changes: 2 additions & 2 deletions tests/test_infra.py
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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: {}
11 changes: 11 additions & 0 deletions tests/unit/models/invalid_charms_yaml/platforms-no-parts.yaml
Original file line number Diff line number Diff line change
@@ -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]
34 changes: 25 additions & 9 deletions tests/unit/models/test_project.py
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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"},
},
),
Expand Down Expand Up @@ -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'"""
),
),
(
Expand All @@ -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

Expand Down
4 changes: 3 additions & 1 deletion tests/unit/models/valid_charms_yaml/basic-platforms.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,6 @@ platforms:
amd64:
build-on: [amd64]
build-for: [amd64]
parts: {}
parts:
my-part:
plugin: nil
6 changes: 3 additions & 3 deletions tests/unit/services/test_package.py
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
Loading
Loading