launchpad-reviewers team mailing list archive

[Guruprasad <mp+418035@xxxxxxxxxxxxxxxxxx>]

Diff comments:

> diff --git a/lpcraft/commands/tests/test_clean.py b/lpcraft/commands/tests/test_clean.py
> index 5a11555..150676a 100644
> --- a/lpcraft/commands/tests/test_clean.py
> +++ b/lpcraft/commands/tests/test_clean.py
> @@ -24,6 +24,31 @@ class TestClean(CommandBaseTestCase):
>          os.chdir(self.tmp_project_path)
>          self.addCleanup(os.chdir, cwd)
>  
> +    def test_config_file_not_under_project_directory(self):
> +        paths = [
> +            "/",
> +            "/etc/init.d",
> +            "../../foo",
> +            "a/b/c/../../../../d",
> +        ]
> +
> +        for path in paths:
> +            config_file = f"{path}/config.yaml"
> +            result = self.run_command(
> +                "clean",
> +                "-c",
> +                config_file,
> +            )
> +            config_file_path = Path(config_file).resolve()
> +            # The exception message differs between Python 3.8 and 3.9+.

Ugh! This is ugly.

> +            # That is why the `result` object is manually tested below.
> +            self.assertEqual(1, result.exit_code)
> +            self.assertEqual(1, len(result.errors))
> +            self.assertTrue(isinstance(result.errors[0], CommandError))
> +            message = result.errors[0].args[0]
> +            self.assertIn(str(config_file_path), message)
> +            self.assertIn(str(self.tmp_project_path), message)
> +
>      def test_missing_config_file(self):
>          result = self.run_command("clean")
>  
> diff --git a/lpcraft/config.py b/lpcraft/config.py
> index 0777dd3..6b9286b 100644
> --- a/lpcraft/config.py
> +++ b/lpcraft/config.py
> @@ -127,5 +127,11 @@ class Config(ModelConfigDefaults):
>      @classmethod
>      def load(cls, path: Path) -> "Config":
>          """Load config from the indicated file name."""
> +        # XXX lgp171188 2022-03-31: it is not a good idea to evaluate

I placed the logic to check the path here because it is used by all the commands and avoids repeating the same code.

> +        # `Path.cwd()` in multiple places to determine the project directory.
> +        # This could be made available as an attribute on the `Config` class
> +        # instead.
> +        project_dir = Path.cwd()
> +        check_relative_path(path.resolve(), project_dir)
>          content = load_yaml(path)
>          return cls.parse_obj(content)


-- 
https://code.launchpad.net/~lgp171188/lpcraft/+git/lpcraft/+merge/418035
Your team Launchpad code reviewers is requested to review the proposed merge of ~lgp171188/lpcraft:contain-config-file-path-under-project-dir into lpcraft:main.