From 162710696b03c6b7f85dfe0ac17a845450a737ae Mon Sep 17 00:00:00 2001 From: Sheng Yu Date: Fri, 12 Apr 2024 16:18:47 -0400 Subject: [PATCH] feat(lint): migrate lint command --- snapcraft/application.py | 6 +- snapcraft/cli.py | 1 - snapcraft/commands/__init__.py | 2 + snapcraft/commands/core22/__init__.py | 2 - snapcraft/commands/{core22 => }/lint.py | 15 ++-- snapcraft/commands/unimplemented.py | 6 -- tests/spread/core24/linters-file/task.yaml | 2 - tests/unit/commands/test_lint.py | 82 ++++++++++------------ 8 files changed, 53 insertions(+), 63 deletions(-) rename snapcraft/commands/{core22 => }/lint.py (97%) diff --git a/snapcraft/application.py b/snapcraft/application.py index 6a8a92b1f62..d92a94ebe64 100644 --- a/snapcraft/application.py +++ b/snapcraft/application.py @@ -158,6 +158,10 @@ def _get_dispatcher(self) -> craft_cli.Dispatcher: # project's base (if any). Here, we handle the case where there *is* # a project and it's core24, which means it should definitely fall into # the craft-application-based flow. + if "lint" in sys.argv: + # We don't need to check for core24 if we're just linting + return super()._get_dispatcher() + if self._snapcraft_yaml_path: with self._snapcraft_yaml_path.open() as file: yaml_data = util.safe_yaml_load(file) @@ -280,7 +284,7 @@ def create_app() -> Snapcraft: "Other", list(craft_app_commands.get_other_command_group().commands) + [ - unimplemented.Lint, + commands.LintCommand, unimplemented.Init, ], ) diff --git a/snapcraft/cli.py b/snapcraft/cli.py index 92d8bea9b37..75f70ca3b84 100644 --- a/snapcraft/cli.py +++ b/snapcraft/cli.py @@ -121,7 +121,6 @@ craft_cli.CommandGroup( "Other", [ - commands.core22.LintCommand, commands.core22.InitCommand, ], ), diff --git a/snapcraft/commands/__init__.py b/snapcraft/commands/__init__.py index a8549e867c9..ffb82f27687 100644 --- a/snapcraft/commands/__init__.py +++ b/snapcraft/commands/__init__.py @@ -38,6 +38,7 @@ StoreLegacyValidateCommand, ) from .lifecycle import SnapCommand +from .lint import LintCommand from .manage import StoreCloseCommand, StoreReleaseCommand from .names import ( StoreLegacyListCommand, @@ -58,6 +59,7 @@ __all__ = [ "ExpandExtensions", + "LintCommand", "ListExtensions", "RemoteBuildCommand", "SnapCommand", diff --git a/snapcraft/commands/core22/__init__.py b/snapcraft/commands/core22/__init__.py index 3655f678f78..737700cdcbf 100644 --- a/snapcraft/commands/core22/__init__.py +++ b/snapcraft/commands/core22/__init__.py @@ -33,7 +33,6 @@ StageCommand, TryCommand, ) -from .lint import LintCommand from .remote import RemoteBuildCommand __all__ = [ @@ -42,7 +41,6 @@ "ExpandExtensionsCommand", "ExtensionsCommand", "InitCommand", - "LintCommand", "ListExtensionsCommand", "ListPluginsCommand", "PackCommand", diff --git a/snapcraft/commands/core22/lint.py b/snapcraft/commands/lint.py similarity index 97% rename from snapcraft/commands/core22/lint.py rename to snapcraft/commands/lint.py index 5c2f761e6e8..d1e3e8f5b45 100644 --- a/snapcraft/commands/core22/lint.py +++ b/snapcraft/commands/lint.py @@ -1,6 +1,6 @@ # -*- Mode:Python; indent-tabs-mode:nil; tab-width:4 -*- # -# Copyright 2023 Canonical Ltd. +# Copyright 2023-2024 Canonical Ltd. # # This program is free software: you can redistribute it and/or modify # it under the terms of the GNU General Public License version 3 as @@ -24,9 +24,10 @@ import textwrap from contextlib import contextmanager from pathlib import Path, PurePosixPath -from typing import Iterator, Optional +from typing import Any, Iterator, Optional -from craft_cli import BaseCommand, emit +from craft_application.commands import ExtensibleCommand +from craft_cli import emit from craft_cli.errors import ArgumentParsingError from craft_providers.multipass import MultipassProvider from craft_providers.util import snap_cmd @@ -42,9 +43,10 @@ ) -class LintCommand(BaseCommand): +class LintCommand(ExtensibleCommand): """Lint-related commands.""" + always_load_project = False name = "lint" help_msg = "Lint a snap file" overview = textwrap.dedent( @@ -58,7 +60,7 @@ class LintCommand(BaseCommand): ) @overrides - def fill_parser(self, parser: "argparse.ArgumentParser") -> None: + def _fill_parser(self, parser: argparse.ArgumentParser) -> None: parser.add_argument( "snap_file", metavar="snap-file", @@ -78,8 +80,7 @@ def fill_parser(self, parser: "argparse.ArgumentParser") -> None: help="Set https proxy", ) - @overrides - def run(self, parsed_args: argparse.Namespace): + def _run(self, parsed_args: argparse.Namespace, **kwargs: Any) -> None: """Run the linter command. :param parsed_args: snapcraft's argument namespace diff --git a/snapcraft/commands/unimplemented.py b/snapcraft/commands/unimplemented.py index 69d1b164a8c..fef4ce8b118 100644 --- a/snapcraft/commands/unimplemented.py +++ b/snapcraft/commands/unimplemented.py @@ -105,9 +105,3 @@ class Try( UnimplementedMixin, commands.core22.TryCommand ): # noqa: D101 (missing docstring) pass - - -class Lint( - UnimplementedMixin, commands.core22.LintCommand -): # noqa: D101 (missing docstring) - pass diff --git a/tests/spread/core24/linters-file/task.yaml b/tests/spread/core24/linters-file/task.yaml index 155e905bf1c..eda4cd685ac 100644 --- a/tests/spread/core24/linters-file/task.yaml +++ b/tests/spread/core24/linters-file/task.yaml @@ -1,6 +1,4 @@ summary: Run snapcraft lint on a snap file. -# Disabled from regular CI runs until we have "snapcraft lint" in core24 -manual: true environment: SNAP: lint-file diff --git a/tests/unit/commands/test_lint.py b/tests/unit/commands/test_lint.py index cee9df46d95..666ff31cabb 100644 --- a/tests/unit/commands/test_lint.py +++ b/tests/unit/commands/test_lint.py @@ -25,8 +25,8 @@ from craft_providers.bases import BuilddBaseAlias from craft_providers.multipass import MultipassProvider -from snapcraft import cli, models -from snapcraft.commands.core22.lint import LintCommand +from snapcraft import application, models +from snapcraft.commands.lint import LintCommand from snapcraft.errors import SnapcraftError from snapcraft.meta.snap_yaml import SnapMetadata @@ -81,9 +81,7 @@ def mock_argv(mocker, fake_snap_file): @pytest.fixture def mock_capture_logs_from_instance(mocker): - return mocker.patch( - "snapcraft.commands.core22.lint.providers.capture_logs_from_instance" - ) + return mocker.patch("snapcraft.commands.lint.providers.capture_logs_from_instance") @pytest.fixture @@ -100,16 +98,14 @@ def mock_get_base_configuration(mocker): @pytest.fixture def mock_is_managed_mode(mocker): - return mocker.patch( - "snapcraft.commands.core22.lint.is_managed_mode", return_value=False - ) + return mocker.patch("snapcraft.commands.lint.is_managed_mode", return_value=False) @pytest.fixture def mock_provider(mocker, mock_instance, fake_provider): _mock_provider = Mock(wraps=fake_provider) mocker.patch( - "snapcraft.commands.core22.lint.providers.get_provider", + "snapcraft.commands.lint.providers.get_provider", return_value=_mock_provider, ) return _mock_provider @@ -118,13 +114,13 @@ def mock_provider(mocker, mock_instance, fake_provider): @pytest.fixture def mock_run_linters(mocker): return mocker.patch( - "snapcraft.commands.core22.lint.linters.run_linters", return_value=Mock() + "snapcraft.commands.lint.linters.run_linters", return_value=Mock() ) @pytest.fixture def mock_report(mocker): - return mocker.patch("snapcraft.commands.core22.lint.linters.report") + return mocker.patch("snapcraft.commands.lint.linters.report") def test_lint_default( @@ -144,7 +140,7 @@ def test_lint_default( fake_snap_file.touch() fake_assert_file.touch() - cli.run() + application.main() mock_ensure_provider_is_available.assert_called_once() mock_get_base_configuration.assert_called_once_with( @@ -202,7 +198,7 @@ def test_lint_http_https_proxy( ], ) - cli.run() + application.main() mock_get_base_configuration.assert_called_once_with( alias=BuilddBaseAlias.JAMMY, @@ -228,7 +224,7 @@ def test_lint_assert_file_missing( # create a snap file fake_snap_file.touch() - cli.run() + application.main() mock_instance.push_file.assert_called_once_with( source=fake_snap_file, @@ -259,7 +255,7 @@ def test_lint_assert_file_not_valid( # make the assertion filepath a directory fake_assert_file.mkdir() - cli.run() + application.main() mock_instance.push_file.assert_called_once_with( source=fake_snap_file, @@ -285,14 +281,14 @@ def test_lint_multipass_not_supported( """Raise an error if Multipass is used as the build provider.""" _mock_provider = Mock(wraps=fake_provider, spec=MultipassProvider) mocker.patch( - "snapcraft.commands.core22.lint.providers.get_provider", + "snapcraft.commands.lint.providers.get_provider", return_value=_mock_provider, ) # create a snap file fake_snap_file.touch() - cli.run() + application.main() out, err = capsys.readouterr() assert not out @@ -303,7 +299,7 @@ def test_lint_multipass_not_supported( def test_lint_default_snap_file_missing(capsys, fake_snap_file, mock_argv): """Raise an error if the snap file does not exist.""" - cli.run() + application.main() out, err = capsys.readouterr() assert not out @@ -315,7 +311,7 @@ def test_lint_default_snap_file_not_valid(capsys, fake_snap_file, mock_argv): # make the snap filepath a directory fake_snap_file.mkdir() - cli.run() + application.main() out, err = capsys.readouterr() assert not out @@ -340,7 +336,7 @@ def test_lint_execute_run_error( mock_instance.execute_run.side_effect = CalledProcessError(cmd="err", returncode=1) - cli.run() + application.main() mock_capture_logs_from_instance.assert_called_once() out, err = capsys.readouterr() @@ -389,14 +385,14 @@ def test_lint_managed_mode( fake_snap_metadata.confinement = confinement fake_snap_metadata.grade = grade mocker.patch( - "snapcraft.commands.core22.lint.snap_yaml.read", return_value=fake_snap_metadata + "snapcraft.commands.lint.snap_yaml.read", return_value=fake_snap_metadata ) mocker.patch( - "snapcraft.commands.core22.lint.LintCommand._load_project", + "snapcraft.commands.lint.LintCommand._load_project", return_value=fake_snapcraft_project, ) - cli.run() + application.main() mock_run_linters.assert_called_once_with( lint=models.Lint(ignore=["classic"]), @@ -445,14 +441,14 @@ def test_lint_managed_mode_without_snapcraft_yaml( # mock data from the unsquashed snap mocker.patch( - "snapcraft.commands.core22.lint.snap_yaml.read", return_value=fake_snap_metadata + "snapcraft.commands.lint.snap_yaml.read", return_value=fake_snap_metadata ) mocker.patch( - "snapcraft.commands.core22.lint.LintCommand._load_project", + "snapcraft.commands.lint.LintCommand._load_project", return_value=None, ) - cli.run() + application.main() mock_run_linters.assert_called_once_with( lint=models.Lint(ignore=["classic"]), @@ -507,14 +503,14 @@ def test_lint_managed_mode_unsquash_error( # mock data from the unsquashed snap mocker.patch( - "snapcraft.commands.core22.lint.snap_yaml.read", return_value=fake_snap_metadata + "snapcraft.commands.lint.snap_yaml.read", return_value=fake_snap_metadata ) mocker.patch( - "snapcraft.commands.core22.lint.LintCommand._load_project", + "snapcraft.commands.lint.LintCommand._load_project", return_value=fake_snapcraft_project, ) - cli.run() + application.main() out, err = capsys.readouterr() assert not out @@ -550,14 +546,14 @@ def test_lint_managed_mode_snap_install_error( # mock data from the unsquashed snap mocker.patch( - "snapcraft.commands.core22.lint.snap_yaml.read", return_value=fake_snap_metadata + "snapcraft.commands.lint.snap_yaml.read", return_value=fake_snap_metadata ) mocker.patch( - "snapcraft.commands.core22.lint.LintCommand._load_project", + "snapcraft.commands.lint.LintCommand._load_project", return_value=fake_snapcraft_project, ) - cli.run() + application.main() out, err = capsys.readouterr() assert not out @@ -593,14 +589,14 @@ def test_lint_managed_mode_assert( # mock data from the unsquashed snap mocker.patch( - "snapcraft.commands.core22.lint.snap_yaml.read", return_value=fake_snap_metadata + "snapcraft.commands.lint.snap_yaml.read", return_value=fake_snap_metadata ) mocker.patch( - "snapcraft.commands.core22.lint.LintCommand._load_project", + "snapcraft.commands.lint.LintCommand._load_project", return_value=fake_snapcraft_project, ) - cli.run() + application.main() mock_run_linters.assert_called_once_with( lint=models.Lint(ignore=["classic"]), @@ -657,14 +653,14 @@ def test_lint_managed_mode_assert_error( # mock data from the unsquashed snap mocker.patch( - "snapcraft.commands.core22.lint.snap_yaml.read", return_value=fake_snap_metadata + "snapcraft.commands.lint.snap_yaml.read", return_value=fake_snap_metadata ) mocker.patch( - "snapcraft.commands.core22.lint.LintCommand._load_project", + "snapcraft.commands.lint.LintCommand._load_project", return_value=fake_snapcraft_project, ) - cli.run() + application.main() mock_run_linters.assert_called_once_with( lint=models.Lint(ignore=["classic"]), @@ -749,17 +745,17 @@ def test_lint_managed_mode_with_lint_config( # mock data from the unsquashed snap mocker.patch( - "snapcraft.commands.core22.lint.snap_yaml.read", return_value=fake_snap_metadata + "snapcraft.commands.lint.snap_yaml.read", return_value=fake_snap_metadata ) # add a lint config to the project fake_snapcraft_project.lint = project_lint mocker.patch( - "snapcraft.commands.core22.lint.LintCommand._load_project", + "snapcraft.commands.lint.LintCommand._load_project", return_value=fake_snapcraft_project, ) - cli.run() + application.main() # lint config from project should be passed to `run_linter()` mock_run_linters.assert_called_once_with( @@ -812,9 +808,7 @@ def test_load_project_complex(mocker, tmp_path): This includes lint, parse-info, architectures, and advanced grammar. """ # mock for advanced grammar parsing (i.e. `on amd64:`) - mocker.patch( - "snapcraft.commands.core22.lint.get_host_architecture", return_value="amd64" - ) + mocker.patch("snapcraft.commands.lint.get_host_architecture", return_value="amd64") # create a snap file (tmp_path / "snap").mkdir()