From ae211d3b25135781fe8593fb9c8ff483bc6e4530 Mon Sep 17 00:00:00 2001
From: Hilary Oliver <hilary.j.oliver@gmail.com>
Date: Thu, 27 Jan 2022 13:28:59 +1300
Subject: [PATCH] Refactor config options checking a bit.

---
 cylc/flow/scripts/config.py                 | 50 +++++--------
 tests/functional/cylc-config/09-platforms.t |  1 -
 tests/unit/scripts/test_config.py           | 83 +--------------------
 3 files changed, 19 insertions(+), 115 deletions(-)

diff --git a/cylc/flow/scripts/config.py b/cylc/flow/scripts/config.py
index c8dab4244f7..948ca7b9b3b 100755
--- a/cylc/flow/scripts/config.py
+++ b/cylc/flow/scripts/config.py
@@ -137,45 +137,33 @@ def get_config_file_hierarchy(workflow_id: Optional[str] = None) -> List[str]:
     return filepaths
 
 
-def options_are_valid(options: 'Values', wid: Optional[str] = None) -> None:
-    """Check parser for options that should not be allowed.
-    """
-    if (
-        options.print_platform_names and
-        options.print_platforms
-    ):
-        raise UserInputError(
-            '--platform-names is not compatible with --platforms'
-        )
-    if (
-        wid is not None
-        and (options.print_platform_names or options.print_platforms)
-    ):
-        raise UserInputError(
-            '--platform-names and --platforms are not compatible '
-            'with providing a workflow ID.'
-        )
-
-
 @cli_function(get_option_parser)
 def main(
     parser: COP,
     options: 'Values',
     *ids,
 ) -> None:
-    if not ids:
-        if options.print_platform_names or options.print_platforms:
-            # Get platform information:
-            glbl_cfg().platform_dump(
-                options.print_platform_names,
-                options.print_platforms
+
+    if options.print_platform_names and options.print_platforms:
+        options.print_platform_names = False
+
+    if options.print_platform_names or options.print_platforms:
+        # Get platform information:
+        if ids:
+            raise UserInputError(
+                "Workflow IDs are incompatible with --platform options."
             )
-            return
+        glbl_cfg().platform_dump(
+            options.print_platform_names,
+            options.print_platforms
+        )
+        return
 
-        elif options.print_hierarchy:
-            print("\n".join(get_config_file_hierarchy()))
-            return
+    if options.print_hierarchy:
+        print("\n".join(get_config_file_hierarchy()))
+        return
 
+    if not ids:
         glbl_cfg().idump(
             options.item,
             not options.defaults,
@@ -190,8 +178,6 @@ def main(
         constraint='workflows',
     )
 
-    options_are_valid(options, workflow_id)
-
     if options.print_hierarchy:
         print("\n".join(get_config_file_hierarchy(workflow_id)))
         return
diff --git a/tests/functional/cylc-config/09-platforms.t b/tests/functional/cylc-config/09-platforms.t
index 2414a6125b9..06ad5001fc0 100755
--- a/tests/functional/cylc-config/09-platforms.t
+++ b/tests/functional/cylc-config/09-platforms.t
@@ -35,7 +35,6 @@ __HEREDOC__
 
 export CYLC_CONF_PATH="${PWD}"
 
-
 TEST_NAME="${TEST_NAME_BASE}-names"
 run_ok "${TEST_NAME}" cylc config --platform-names
 cmp_ok "${TEST_NAME}.stdout" <<__HEREDOC__
diff --git a/tests/unit/scripts/test_config.py b/tests/unit/scripts/test_config.py
index b641b904880..64478621ab4 100644
--- a/tests/unit/scripts/test_config.py
+++ b/tests/unit/scripts/test_config.py
@@ -23,10 +23,7 @@
 import pytest
 from pytest import param
 
-from cylc.flow.scripts.config import (
-    get_config_file_hierarchy,
-    options_are_valid
-)
+from cylc.flow.scripts.config import get_config_file_hierarchy
 from cylc.flow.cfgspec.globalcfg import GlobalConfig
 
 
@@ -203,81 +200,3 @@ def test_cylc_site_conf_path_env_var(
     GlobalConfig.get_inst()
 
     assert capload == files
-
-
-@pytest.mark.parametrize(
-    'expect, wid_given, p_name_set, p_set',
-    [
-        (
-            {
-                'log': '--platform-names is not compatible with --platforms',
-                'result': False
-            },
-            'Foo', True, True
-        ),
-        (
-            {
-                'log': (
-                    '--platform-names and --platforms are not compatible '
-                    'with providing a workflow registration.'
-                ),
-                'result': False
-            },
-            'Foo', True, False
-        ),
-            (
-            {
-                'log': None,
-                'result': True
-            },
-            'Foo', False, False
-        ),
-        (
-            {
-                'log': (
-                    '--platform-names and --platforms are not compatible '
-                    'with providing a workflow registration.'
-                ),
-                'result': False
-            },
-            'Foo', False, True
-        ),
-        (
-            {
-                'log': '--platform-names is not compatible with --platforms',
-                'result': False
-            },
-            None, True, True
-        ),
-        (
-            {'log': None, 'result': True},
-            None, True, False
-        ),
-        (
-            {'log': None, 'result': True},
-            None, False, False
-        ),
-        (
-            {'log': None, 'result': True},
-            None, False, True
-        ),
-    ]
-)
-def test_options_are_valid(
-    caplog, expect, wid_given, p_name_set, p_set,
-):
-    """Test that bad sets of options are not allowed."""
-    # Setup a mock options object:
-    options = SimpleNamespace(
-        print_platform_names=p_name_set,
-        print_platforms=p_set
-    )
-
-    # Run the function under test:
-    if expect['result'] == True:
-        # Function does nothing:
-        assert caplog.records == []
-    else:
-        # Function Raises:
-        with pytest.raises(UserInputError):
-            options_are_valid(options, wid=wid_given)