From fbedf67980e99c9d0c1811baf8575c81c6a2a9bb Mon Sep 17 00:00:00 2001 From: Sergio Schvezov Date: Wed, 18 Sep 2024 09:31:41 -0300 Subject: [PATCH 1/3] chore: update craft-application to 4.2.2 - Setup the emitter fixture where required - Make more use of pytest.mark.usefixtures for unused fixtures - Remove some of the unused fixtures Signed-off-by: Sergio Schvezov --- requirements-devel.txt | 2 +- requirements-docs.txt | 2 +- requirements.txt | 2 +- tests/unit/commands/test_lifecycle.py | 3 +- tests/unit/commands/test_remote.py | 45 +++++++++++---------------- 5 files changed, 23 insertions(+), 31 deletions(-) diff --git a/requirements-devel.txt b/requirements-devel.txt index cdd42303c3..a160875797 100644 --- a/requirements-devel.txt +++ b/requirements-devel.txt @@ -24,7 +24,7 @@ click==8.1.7 codespell==2.3.0 colorama==0.4.6 coverage==7.6.1 -craft-application==4.1.2 +craft-application==4.2.2 craft-archives==2.0.0 craft-cli==2.7.0 craft-grammar==2.0.0 diff --git a/requirements-docs.txt b/requirements-docs.txt index 61102c9523..a23b44e6db 100644 --- a/requirements-docs.txt +++ b/requirements-docs.txt @@ -19,7 +19,7 @@ chardet==5.2.0 charset-normalizer==3.3.2 click==8.1.7 colorama==0.4.6 -craft-application==4.1.2 +craft-application==4.2.2 craft-archives==2.0.0 craft-cli==2.7.0 craft-grammar==2.0.0 diff --git a/requirements.txt b/requirements.txt index 75f6107ff1..d7265c9402 100644 --- a/requirements.txt +++ b/requirements.txt @@ -7,7 +7,7 @@ cffi==1.17.1 chardet==5.2.0 charset-normalizer==3.3.2 click==8.1.7 -craft-application==4.1.2 +craft-application==4.2.2 craft-archives==2.0.0 craft-cli==2.7.0 craft-grammar==2.0.0 diff --git a/tests/unit/commands/test_lifecycle.py b/tests/unit/commands/test_lifecycle.py index ee8ce3b23d..b4e5ab47a1 100644 --- a/tests/unit/commands/test_lifecycle.py +++ b/tests/unit/commands/test_lifecycle.py @@ -107,7 +107,8 @@ def test_snap_command_fallback(tmp_path, emitter, mocker, fake_services): ) -def test_core24_try_command(tmp_path, mocker, fake_services): +@pytest.mark.usefixtures("emitter") +def test_core24_try_command(tmp_path, fake_services): parsed_args = argparse.Namespace(parts=[], output=tmp_path) cmd = lifecycle.TryCommand({"app": APP_METADATA, "services": fake_services}) diff --git a/tests/unit/commands/test_remote.py b/tests/unit/commands/test_remote.py index c8e109ef91..6ebdc50633 100644 --- a/tests/unit/commands/test_remote.py +++ b/tests/unit/commands/test_remote.py @@ -36,7 +36,8 @@ # remote-build control logic may check if the working dir is a git repo, # so execute all tests inside a test directory -pytestmark = pytest.mark.usefixtures("new_dir") +# The service also emits +pytestmark = pytest.mark.usefixtures("new_dir", "emitter") @pytest.fixture() @@ -163,7 +164,7 @@ def test_no_confirmation_for_private_project( @pytest.mark.parametrize("base", const.CURRENT_BASES) -@pytest.mark.usefixtures("mock_argv") +@pytest.mark.usefixtures("mock_argv", "emitter") def test_command_user_confirms_upload( snapcraft_yaml, base, mock_confirm, fake_services ): @@ -183,7 +184,7 @@ def test_command_user_confirms_upload( @pytest.mark.parametrize("base", const.CURRENT_BASES) -@pytest.mark.usefixtures("mock_argv", "fake_services") +@pytest.mark.usefixtures("mock_argv", "emitter", "fake_services") def test_command_user_denies_upload( capsys, snapcraft_yaml, @@ -206,7 +207,7 @@ def test_command_user_denies_upload( @pytest.mark.parametrize("base", const.CURRENT_BASES) -@pytest.mark.usefixtures("mock_argv", "fake_services") +@pytest.mark.usefixtures("mock_argv", "emitter", "fake_services") def test_command_accept_upload( mocker, snapcraft_yaml, base, mock_confirm, mock_run_remote_build ): @@ -224,7 +225,9 @@ def test_command_accept_upload( @pytest.mark.parametrize("base", const.CURRENT_BASES) -@pytest.mark.usefixtures("mock_argv", "mock_confirm", "fake_services", "fake_sudo") +@pytest.mark.usefixtures( + "mock_argv", "mock_confirm", "emitter", "fake_services", "fake_sudo" +) def test_remote_build_sudo_warns(emitter, snapcraft_yaml, base, mock_run_remote_build): "Check if a warning is shown when snapcraft is run with sudo." snapcraft_yaml_dict = {"base": base, "build-base": "devel", "grade": "devel"} @@ -240,8 +243,8 @@ def test_remote_build_sudo_warns(emitter, snapcraft_yaml, base, mock_run_remote_ @pytest.mark.parametrize("base", const.CURRENT_BASES) -@pytest.mark.usefixtures("mock_argv", "mock_confirm", "fake_services") -def test_launchpad_timeout_default(mocker, snapcraft_yaml, base, fake_services): +@pytest.mark.usefixtures("mock_argv", "mock_confirm", "emitter", "fake_services") +def test_launchpad_timeout_default(mocker, snapcraft_yaml, base): """Check if no timeout is set by default.""" snapcraft_yaml_dict = {"base": base, "build-base": "devel", "grade": "devel"} snapcraft_yaml(**snapcraft_yaml_dict) @@ -256,8 +259,8 @@ def test_launchpad_timeout_default(mocker, snapcraft_yaml, base, fake_services): @pytest.mark.parametrize("base", const.CURRENT_BASES) -@pytest.mark.usefixtures("mock_argv", "mock_confirm", "fake_services") -def test_launchpad_timeout(mocker, snapcraft_yaml, base, fake_services): +@pytest.mark.usefixtures("mock_argv", "mock_confirm", "emitter", "fake_services") +def test_launchpad_timeout(mocker, snapcraft_yaml, base): """Set the timeout for the remote builder.""" mocker.patch.object( sys, "argv", ["snapcraft", "remote-build", "--launchpad-timeout", "100"] @@ -281,7 +284,7 @@ def test_launchpad_timeout(mocker, snapcraft_yaml, base, fake_services): @pytest.mark.parametrize("base", const.CURRENT_BASES) -@pytest.mark.usefixtures("mock_argv", "mock_confirm", "fake_services") +@pytest.mark.usefixtures("mock_argv", "mock_confirm", "emitter", "fake_services") def test_run_core22_and_later(snapcraft_yaml, base, mock_remote_build_run): """Bases that are core22 and later will use craft-application remote-build.""" snapcraft_yaml_dict = {"base": base, "build-base": "devel", "grade": "devel"} @@ -309,15 +312,8 @@ def test_run_core20( @pytest.mark.parametrize("base", const.CURRENT_BASES) -@pytest.mark.usefixtures("mock_confirm", "mock_argv") -def test_run_in_repo_newer_than_core22( - emitter, - mocker, - snapcraft_yaml, - base, - new_dir, - fake_services, -): +@pytest.mark.usefixtures("mock_confirm", "mock_argv", "emitter", "fake_services") +def test_run_in_repo_newer_than_core22(mocker, snapcraft_yaml, base, new_dir): """Bases newer than core22 run craft-application remote-build regardless of being in a repo.""" # initialize a git repo GitRepo(new_dir) @@ -337,15 +333,10 @@ def test_run_in_repo_newer_than_core22( @pytest.mark.usefixtures( "mock_confirm", "mock_argv", + "mock_remote_builder_start_builds", + "fake_services", ) -def test_run_in_shallow_repo_unsupported( - capsys, - new_dir, - snapcraft_yaml, - base, - mock_remote_builder_start_builds, - fake_services, -): +def test_run_in_shallow_repo_unsupported(capsys, new_dir, snapcraft_yaml, base): """devel / core24 and newer bases run new remote-build in a shallow git repo.""" root_path = Path(new_dir) snapcraft_yaml_dict = {"base": base, "build-base": "devel", "grade": "devel"} From 1f4734fb45719229e3aadce0be02f01c00f2fd8d Mon Sep 17 00:00:00 2001 From: Sergio Schvezov Date: Fri, 13 Sep 2024 10:23:45 -0300 Subject: [PATCH 2/3] fix: override _run_inner to catch exceptions Remove the heading from the original error to not give the impression it is a craft application error to the user, but instead an error generated store side. Signed-off-by: Sergio Schvezov --- snapcraft/application.py | 10 ++++------ tests/unit/test_application.py | 8 ++++---- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/snapcraft/application.py b/snapcraft/application.py index 706dd87b21..ee7229fa87 100644 --- a/snapcraft/application.py +++ b/snapcraft/application.py @@ -209,13 +209,13 @@ def _pre_run(self, dispatcher: craft_cli.Dispatcher) -> None: super()._pre_run(dispatcher) @override - def run(self) -> int: + def _run_inner(self) -> int: try: - return_code = super().run() + return_code = super()._run_inner() except craft_store.errors.NoKeyringError as err: self._emit_error( craft_cli.errors.CraftError( - f"craft-store error: {err}", + f"{err}", resolution=( "Ensure the keyring is working or " f"{store.constants.ENVIRONMENT_STORE_CREDENTIALS} " @@ -227,9 +227,7 @@ def run(self) -> int: return_code = 1 except craft_store.errors.CraftStoreError as err: self._emit_error( - craft_cli.errors.CraftError( - f"craft-store error: {err}", resolution=err.resolution - ), + craft_cli.errors.CraftError(f"{err}", resolution=err.resolution), cause=err, ) return_code = 1 diff --git a/tests/unit/test_application.py b/tests/unit/test_application.py index 3f507b8663..e4ee1d5998 100644 --- a/tests/unit/test_application.py +++ b/tests/unit/test_application.py @@ -665,7 +665,7 @@ def test_known_core24(snapcraft_yaml, base, build_base, is_known_core24): ) def test_store_error(mocker, capsys, message, resolution, expected_message): mocker.patch( - "snapcraft.application.Application.run", + "snapcraft.application.Application._run_inner", side_effect=craft_store.errors.CraftStoreError(message, resolution=resolution), ) @@ -673,12 +673,12 @@ def test_store_error(mocker, capsys, message, resolution, expected_message): assert return_code == 1 _, err = capsys.readouterr() - assert f"craft-store error: {expected_message}" in err + assert expected_message in err def test_store_key_error(mocker, capsys): mocker.patch( - "snapcraft.application.Application.run", + "snapcraft.application.Application._run_inner", side_effect=craft_store.errors.NoKeyringError(), ) @@ -692,7 +692,7 @@ def test_store_key_error(mocker, capsys): # pylint: disable=[line-too-long] dedent( """\ - craft-store error: No keyring found to store or retrieve credentials from. + No keyring found to store or retrieve credentials from. Recommended resolution: Ensure the keyring is working or SNAPCRAFT_STORE_CREDENTIALS is correctly exported into the environment For more information, check out: https://snapcraft.io/docs/snapcraft-authentication """ From 19c5d241a4b5a18adfb887cfefc5e0a2a7a7e79f Mon Sep 17 00:00:00 2001 From: Alex Lowe Date: Wed, 18 Sep 2024 15:21:07 -0400 Subject: [PATCH 3/3] build(deps): update craft-application --- requirements-devel.txt | 2 +- requirements-docs.txt | 2 +- requirements.txt | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/requirements-devel.txt b/requirements-devel.txt index a160875797..3db5bdf4ef 100644 --- a/requirements-devel.txt +++ b/requirements-devel.txt @@ -24,7 +24,7 @@ click==8.1.7 codespell==2.3.0 colorama==0.4.6 coverage==7.6.1 -craft-application==4.2.2 +craft-application==4.2.3 craft-archives==2.0.0 craft-cli==2.7.0 craft-grammar==2.0.0 diff --git a/requirements-docs.txt b/requirements-docs.txt index a23b44e6db..ae23b0c27f 100644 --- a/requirements-docs.txt +++ b/requirements-docs.txt @@ -19,7 +19,7 @@ chardet==5.2.0 charset-normalizer==3.3.2 click==8.1.7 colorama==0.4.6 -craft-application==4.2.2 +craft-application==4.2.3 craft-archives==2.0.0 craft-cli==2.7.0 craft-grammar==2.0.0 diff --git a/requirements.txt b/requirements.txt index d7265c9402..fab86ab254 100644 --- a/requirements.txt +++ b/requirements.txt @@ -7,7 +7,7 @@ cffi==1.17.1 chardet==5.2.0 charset-normalizer==3.3.2 click==8.1.7 -craft-application==4.2.2 +craft-application==4.2.3 craft-archives==2.0.0 craft-cli==2.7.0 craft-grammar==2.0.0