cleanup, no warn on tests

Author: kieranmanning

cloudsmith_cli/cli/commands/main.py

@@ -1,7 +1,10 @@
 """Main command/entrypoint."""
 
+from functools import partial
+
 import click
 
+from cloudsmith_cli.cli import config
 from cloudsmith_cli.cli.warnings import get_or_create_warnings
 
 from ...core.api.version import get_version as get_api_version
@@ -53,12 +56,22 @@ def print_version():
     is_flag=True,
     is_eager=True,
 )
+@click.option(
+    "--no-warn",
+    help="Don't display auth or config warnings",
+    envvar="CLOUDSMITH_CLI_NO_WARN",
+    is_flag=True,
+    default=None,
+)
 @decorators.common_cli_config_options
 @click.pass_context
-def main(ctx, opts, version):
+def main(ctx, opts, version, no_warn):
     """Handle entrypoint to CLI."""
     # pylint: disable=unused-argument
 
+    if no_warn:
+        opts.no_warn = True
+
     if version:
         print_version()
     elif ctx.invoked_subcommand is None:
@@ -67,9 +80,12 @@ def main(ctx, opts, version):
 
 @main.result_callback()
 @click.pass_context
-def result_callback(ctx, opts, **kwargs):
+def result_callback(ctx, _, **kwargs):
     """Callback for main function. Required for saving warnings til the end."""
 
     warnings = get_or_create_warnings(ctx)
-    if warnings:
-        click.echo(warnings.report())
+    opts = config.get_or_create_options(ctx)
+
+    if warnings and not opts.no_warn:
+        click_warn_partial = partial(click.secho, fg="yellow")
+        warnings.display(click_warn_partial)

cloudsmith_cli/cli/config.py

@@ -79,7 +79,6 @@ class ConfigReader(ConfigFileReader):
     config_name = "standard"
     config_searchpath = list(_CFG_SEARCH_PATHS)
     config_section_schemas = [ConfigSchema.Default, ConfigSchema.Profile]
-    config_warning_issued = False
 
     @classmethod
     def select_config_schema_for(cls, section_name):
@@ -150,19 +149,6 @@ def has_default_file(cls):
 
         return False
 
-    @classmethod
-    def config_already_warned(cls):
-        """
-        Check if a configuration file warning has been issued.
-        This is required as configs are gathered at the root of the
-        command chain as well as for command verbs
-        """
-        if cls.config_warning_issued:
-            return True
-
-        cls.config_warning_issued = True
-        return False
-
     @classmethod
     def load_config(cls, opts, path=None, warnings=None, profile=None):
         """Load a configuration file into an options object."""
@@ -176,18 +162,20 @@ def load_config(cls, opts, path=None, warnings=None, profile=None):
         config = cls.read_config()
         values = config.get("default", {})
         cls._load_values_into_opts(opts, values)
+        existing_config_paths = {
+            path: os.path.exists(path) for path in cls.config_files
+        }
 
         if profile and profile != "default":
             try:
                 values = config["profile:%s" % profile]
                 cls._load_values_into_opts(opts, values)
             except KeyError:
-                warning = ProfileNotFoundWarning(path=path, profile=profile)
+                warning = ProfileNotFoundWarning(
+                    paths=existing_config_paths, profile=profile
+                )
                 warnings.append(warning)
 
-        existing_config_paths = {
-            path: os.path.exists(path) for path in cls.config_files
-        }
         if not any(list(existing_config_paths.values())):
             config_load_warning = ConfigLoadWarning(
                 paths=existing_config_paths,
@@ -283,13 +271,11 @@ def get_creds_reader():
 
     def load_config_file(self, path, warnings=None, profile=None):
         """Load the standard config file."""
-        print("load_config_file")
         config_cls = self.get_config_reader()
         return config_cls.load_config(self, path, warnings=warnings, profile=profile)
 
     def load_creds_file(self, path, warnings=None, profile=None):
         """Load the credentials config file."""
-        print("load_creds_file")
         config_cls = self.get_creds_reader()
         return config_cls.load_config(self, path, warnings=warnings, profile=profile)
 

cloudsmith_cli/cli/tests/commands/policy/test_licence.py

@@ -85,7 +85,9 @@ def assert_output_matches_policy_config(output, config_file_path):
     return output_table["Identifier"]
 
 
-@pytest.mark.usefixtures("set_api_key_env_var", "set_api_host_env_var")
+@pytest.mark.usefixtures(
+    "set_api_key_env_var", "set_api_host_env_var", "set_no_warn_env_var"
+)
 def test_license_policy_commands(runner, organization, tmp_path):
     """Test CRUD operations for license policies."""
 

cloudsmith_cli/cli/tests/commands/policy/test_vulnerability.py

@@ -87,7 +87,9 @@ def assert_output_matches_policy_config(output, config_file_path):
     return output_table["Identifier"]
 
 
-@pytest.mark.usefixtures("set_api_key_env_var", "set_api_host_env_var")
+@pytest.mark.usefixtures(
+    "set_api_key_env_var", "set_api_host_env_var", "set_no_warn_env_var"
+)
 def test_vulnerability_policy_commands(runner, organization, tmp_path):
     """Test CRUD operations for vulnerability policies."""
 

cloudsmith_cli/cli/tests/commands/test_main.py

@@ -6,14 +6,17 @@
 
 
 class TestMainCommand:
+    @pytest.mark.usefixtures("set_no_warn_env_var")
     @pytest.mark.parametrize("option", ["-V", "--version"])
     def test_main_version(self, runner, option):
         """Test the output of `cloudsmith --version`."""
         result = runner.invoke(main, [option])
         assert result.exit_code == 0
-
-        assert "CLI Package Version: " + get_version() in result.output
-        assert "API Package Version: " + get_api_version() in result.output
+        assert (
+            result.output == "Versions:\n"
+            "CLI Package Version: " + get_version() + "\n"
+            "API Package Version: " + get_api_version() + "\n"
+        )
 
     @pytest.mark.parametrize("option", ["-h", "--help"])
     def test_main_help(self, runner, option):

cloudsmith_cli/cli/tests/commands/test_package_commands.py

@@ -11,7 +11,9 @@
 from ..utils import random_str
 
 
-@pytest.mark.usefixtures("set_api_key_env_var", "set_api_host_env_var")
+@pytest.mark.usefixtures(
+    "set_api_key_env_var", "set_api_host_env_var", "set_no_warn_env_var"
+)
 @pytest.mark.parametrize(
     "filesize",
     [
@@ -80,7 +82,9 @@ def test_push_and_delete_raw_package(
     assert len(data) == 0
 
 
-@pytest.mark.usefixtures("set_api_key_env_var", "set_api_host_env_var")
+@pytest.mark.usefixtures(
+    "set_api_key_env_var", "set_api_host_env_var", "set_no_warn_env_var"
+)
 def test_list_packages_with_sort(runner, organization, tmp_repository, tmp_path):
     """Test listing packages with different sort options."""
     org_repo = f'{organization}/{tmp_repository["slug"]}'

cloudsmith_cli/cli/tests/commands/test_repos.py

@@ -72,7 +72,9 @@ def assert_output_is_equal_to_repo_config(output, organisation, repo_config_file
     )
 
 
-@pytest.mark.usefixtures("set_api_key_env_var", "set_api_host_env_var")
+@pytest.mark.usefixtures(
+    "set_api_key_env_var", "set_api_host_env_var", "set_no_warn_env_var"
+)
 def test_repos_commands(runner, organization, tmp_path):
     """Test CRUD operations for repositories."""
 

cloudsmith_cli/cli/tests/commands/test_upstream.py

@@ -6,7 +6,9 @@
 from ..utils import random_str
 
 
-@pytest.mark.usefixtures("set_api_key_env_var", "set_api_host_env_var")
+@pytest.mark.usefixtures(
+    "set_api_key_env_var", "set_api_host_env_var", "set_no_warn_env_var"
+)
 @pytest.mark.parametrize("upstream_format", UPSTREAM_FORMATS)
 def test_upstream_commands(
     runner, organization, upstream_format, tmp_repository, tmp_path

cloudsmith_cli/cli/tests/conftest.py

@@ -73,3 +73,9 @@ def set_api_host_env_var(api_host):
 def set_api_key_env_var(api_key):
     """Set the CLOUDSMITH_API_KEY environment variable."""
     os.environ["CLOUDSMITH_API_KEY"] = api_key
+
+
+@pytest.fixture()
+def set_no_warn_env_var():
+    """Set the CLOUDSMITH_API_KEY environment variable."""
+    os.environ["CLOUDSMITH_CLI_NO_WARN"] = "True"

cloudsmith_cli/cli/warnings.py

@@ -10,8 +10,14 @@ class CliWarning(ABC):
     def __repr__(self):
         return self.__str__()
 
-    def __str__(self):
-        return f"{self.__class__.__name__}"
+    def __hash__(self):
+        return hash(self.__str__())
+
+    def __eq__(self, other):
+        if not isinstance(other, self.__class__):
+            return False
+
+        return self.__dict__ == other.__dict__
 
 
 class ConfigLoadWarning(CliWarning):
@@ -23,22 +29,24 @@ def __init__(self, paths: Dict[str, bool]):
         self.paths = paths
         self.message = "Failed to load config files. Tried the following paths: \n"
         for path, exists in paths.items():
-            self.message += f"  - {path} - exists: {exists})\n"
-        self.message += "You may need to run `cloudsmith login` to authenticate and create a config file."
+            self.message += f"  - {path} - exists: {exists}\n"
+        self.message += "You may need to run `cloudsmith login` to authenticate and create a config file. \n"
 
     def __str__(self):
         return f"{self.__class__.__name__} - {self.paths}"
 
 
 class ProfileNotFoundWarning(CliWarning):
     """
-    Warning for issues loading the configuration file.
+    Warning for issues finding the requested profile.
     """
 
-    def __init__(self, path, profile):
-        self.path = path
+    def __init__(self, paths, profile):
+        self.path = paths
         self.profile = profile
-        self.message = f"Failed to load config file: {path} for profile: {profile}"
+        self.message = f"Failed to load profile {profile} from config. Tried the following paths: \n"
+        for path, exists in paths.items():
+            self.message += f"  - {path} - exists: {exists}\n"
 
     def __str__(self):
         return f"{self.__class__.__name__} - {self.path} - {self.profile}"
@@ -51,11 +59,11 @@ class ApiAuthenticationWarning(CliWarning):
 
     def __init__(self, cloudsmith_host):
         self.cloudsmith_host = cloudsmith_host
-        self.message = "\n".join(
+        self.message = "".join(
             [
-                "Failed to authenticate with Cloudsmith API",
-                "Please check your credentials and try again",
-                f"Host: {cloudsmith_host}",
+                "Failed to authenticate with Cloudsmith API ",
+                "Please check your credentials and try again.\n",
+                f"Host: {cloudsmith_host}\n",
             ]
         )
 
@@ -78,20 +86,21 @@ def append(self, warning: CliWarning):
     def __dedupe__(self) -> List[CliWarning]:
         return list(set(self.warnings))
 
-    def report(self) -> List[CliWarning]:
-        return self.__dedupe__()
-
-    def __str__(self) -> str:
-        return ",".join([str(x) for x in self.warnings])
+    def display(self, display_fn):
+        for warning in self.__dedupe__():
+            display_fn(warning.message)
 
     def __repr__(self) -> str:
+        return self.__str__()
+
+    def __str__(self) -> str:
         return ",".join([str(x) for x in self.warnings])
 
     def __len__(self) -> int:
         return len(self.warnings)
 
 
 def get_or_create_warnings(ctx):
-    """Get or create the options object."""
+    """Get or create the warnings object."""
 
     return ctx.ensure_object(CliWarnings)