From 0401ebb1269925ce18e5a850067acd778687af60 Mon Sep 17 00:00:00 2001
From: Tim Pillinger <26465611+wxtim@users.noreply.github.com>
Date: Wed, 13 Dec 2023 10:03:07 +0000
Subject: [PATCH] Fixed unhelpful upgrade message for
 [scheduling][dependencies]graph = foo=>bar

---
 cylc/flow/cfgspec/workflow.py             | 20 ++++++---
 tests/integration/test_upgrade_warning.py | 52 +++++++++++++++++++++++
 2 files changed, 67 insertions(+), 5 deletions(-)
 create mode 100644 tests/integration/test_upgrade_warning.py

diff --git a/cylc/flow/cfgspec/workflow.py b/cylc/flow/cfgspec/workflow.py
index de919c27c0f..01dfa672243 100644
--- a/cylc/flow/cfgspec/workflow.py
+++ b/cylc/flow/cfgspec/workflow.py
@@ -1966,8 +1966,16 @@ def upgrade_graph_section(cfg: Dict[str, Any], descr: str) -> None:
     # Parsec upgrader cannot do this type of move
     with contextlib.suppress(KeyError):
         if 'dependencies' in cfg['scheduling']:
-            msg_old = '[scheduling][dependencies][X]graph'
-            msg_new = '[scheduling][graph]X'
+            if 'graph' in cfg['scheduling']['dependencies']:
+                msg_old = '[scheduling][dependencies]graph'
+                msg_new = '[scheduling][graph]R1'
+                list_cp = False
+            else:
+                msg_old = '[scheduling][dependencies][X]graph'
+                msg_new = '[scheduling][graph] - for X in:'
+                list_cp = True
+
+
             if 'graph' in cfg['scheduling']:
                 raise UpgradeError(
                     f'Cannot upgrade deprecated item "{msg_old} -> {msg_new}" '
@@ -1988,12 +1996,14 @@ def upgrade_graph_section(cfg: Dict[str, Any], descr: str) -> None:
                         graphdict[key] = value
                         keys.add(key)
                 if keys and not cylc.flow.flags.cylc7_back_compat:
-                    LOG.warning(
+                    msg = (
                         'deprecated graph items were automatically upgraded '
                         f'in "{descr}":\n'
-                        f' * (8.0.0) {msg_old} -> {msg_new} - for X in:\n'
-                        f"       {', '.join(sorted(keys))}"
+                        f' * (8.0.0) {msg_old} -> {msg_new}'
                     )
+                    if list_cp:
+                        msg += f"\n       {', '.join(sorted(keys))}"
+                    LOG.warning(msg)
 
 
 def upgrade_param_env_templates(cfg, descr):
diff --git a/tests/integration/test_upgrade_warning.py b/tests/integration/test_upgrade_warning.py
new file mode 100644
index 00000000000..749f51cd633
--- /dev/null
+++ b/tests/integration/test_upgrade_warning.py
@@ -0,0 +1,52 @@
+# THIS FILE IS PART OF THE CYLC WORKFLOW ENGINE.
+# Copyright (C) NIWA & British Crown (Met Office) & Contributors.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+"""Test upgrade warnings make sense.
+"""
+
+
+def test_graph_upgrade_msg_default(flow, validate, caplog):
+    """It lists Cycling definitions which need upgrading."""
+    id_ = flow({
+        'scheduler': {'allow implicit tasks': True},
+        'scheduling': {
+            'initial cycle point': 1042,
+            'dependencies': {
+                'R1': {'graph': 'foo'},
+                'P1Y': {'graph': 'bar & baz'}
+            }
+        },
+    })
+    validate(id_)
+    assert '[scheduling][dependencies][X]graph' in caplog.messages[0]
+    assert 'for X in:\n       P1Y, R1' in caplog.messages[0]
+
+
+def test_graph_upgrade_msg_graph_equals(flow, validate, caplog):
+    """It gives a more useful message in special case where graph is
+    key rather than section:
+
+    [scheduling]
+        [[dependencies]]
+            graph = foo => bar
+    """
+    id_ = flow({
+        'scheduler': {'allow implicit tasks': True},
+        'scheduling': {'dependencies': {'graph': 'foo => bar'}},
+    })
+    validate(id_)
+    expect = ('[scheduling][dependencies]graph -> [scheduling][graph]R1')
+    assert expect in caplog.messages[0]