From 2fd213a6e565feef60364f99c4d0b54f9655ee39 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ren=C3=A9=20Nussbaumer?= <rn@google.com>
Date: Tue, 19 Jun 2012 16:24:21 +0200
Subject: [PATCH] Update the hooks documentation
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Also provide some extended unittests to catch those cases.

Signed-off-by: RenΓ© Nussbaumer <rn@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>
Reviewed-by: Agata Murawska <agatamurawska@google.com>
---
 doc/hooks.rst         | 20 -----------
 test/docs_unittest.py | 81 ++++++++++++++++++++++++++++++-------------
 2 files changed, 57 insertions(+), 44 deletions(-)

diff --git a/doc/hooks.rst b/doc/hooks.rst
index ca384d685..be4c730b5 100644
--- a/doc/hooks.rst
+++ b/doc/hooks.rst
@@ -148,16 +148,6 @@ Changes a node's parameters.
 :pre-execution: master node, the target node
 :post-execution: master node, the target node
 
-OP_NODE_EVACUATE
-++++++++++++++++
-
-Relocate secondary instances from a node.
-
-:directory: node-evacuate
-:env. vars: NEW_SECONDARY, NODE_NAME
-:pre-execution: master node, target node
-:post-execution: master node, target node
-
 OP_NODE_MIGRATE
 ++++++++++++++++
 
@@ -344,16 +334,6 @@ Remove an instance.
 :pre-execution: master node
 :post-execution: master node, primary and secondary nodes
 
-OP_INSTANCE_REPLACE_DISKS
-+++++++++++++++++++++++++
-
-Replace an instance's disks.
-
-:directory: mirror-replace
-:env. vars: MODE, NEW_SECONDARY, OLD_SECONDARY
-:pre-execution: master node, primary and secondary nodes
-:post-execution: master node, primary and secondary nodes
-
 OP_INSTANCE_GROW_DISK
 +++++++++++++++++++++
 
diff --git a/test/docs_unittest.py b/test/docs_unittest.py
index c43792060..d91976ce3 100755
--- a/test/docs_unittest.py
+++ b/test/docs_unittest.py
@@ -86,6 +86,11 @@ def _ReadDocFile(filename):
 
 
 class TestHooksDocs(unittest.TestCase):
+  HOOK_PATH_OK = frozenset([
+    "master-ip-turnup",
+    "master-ip-turndown",
+    ])
+
   def test(self):
     """Check whether all hooks are documented.
 
@@ -98,34 +103,62 @@ class TestHooksDocs(unittest.TestCase):
     assert len(lu2opcode) == len(mcpu.Processor.DISPATCH_TABLE), \
       "Found duplicate entries"
 
-    for name in dir(cmdlib):
-      obj = getattr(cmdlib, name)
-
-      if (isinstance(obj, type) and
-          issubclass(obj, cmdlib.LogicalUnit) and
-          hasattr(obj, "HPATH")):
-        self._CheckHook(name, obj, hooksdoc, lu2opcode)
-
-  def _CheckHook(self, name, lucls, hooksdoc, lu2opcode):
-    opcls = lu2opcode.get(lucls, None)
-
-    if lucls.HTYPE is None:
-      return
+    hooks_paths = frozenset(re.findall("^:directory:\s*(.+)\s*$", hooksdoc,
+                                       re.M))
+    self.assertTrue(self.HOOK_PATH_OK.issubset(hooks_paths),
+                    msg="Whitelisted path not found in documentation")
+
+    raw_hooks_ops = re.findall("^OP_(?!CODE$).+$", hooksdoc, re.M)
+    hooks_ops = set()
+    duplicate_ops = set()
+    for op in raw_hooks_ops:
+      if op in hooks_ops:
+        duplicate_ops.add(op)
+      else:
+        hooks_ops.add(op)
 
-    # TODO: Improve this test (e.g. find hooks documented but no longer
-    # existing)
+    self.assertFalse(duplicate_ops,
+                     msg="Found duplicate opcode documentation: %s" %
+                         utils.CommaJoin(duplicate_ops))
 
-    if opcls:
-      self.assertTrue(re.findall("^%s$" % re.escape(opcls.OP_ID),
-                                 hooksdoc, re.M),
-                      msg=("Missing hook documentation for %s" %
-                           (opcls.OP_ID)))
+    seen_paths = set()
+    seen_ops = set()
 
-    pattern = r"^:directory:\s*%s\s*$" % re.escape(lucls.HPATH)
+    self.assertFalse(duplicate_ops,
+                     msg="Found duplicated hook documentation: %s" %
+                         utils.CommaJoin(duplicate_ops))
 
-    self.assert_(re.findall(pattern, hooksdoc, re.M),
-                 msg=("Missing documentation for hook %s/%s" %
-                      (lucls.HTYPE, lucls.HPATH)))
+    for name in dir(cmdlib):
+      lucls = getattr(cmdlib, name)
+
+      if (isinstance(lucls, type) and
+          issubclass(lucls, cmdlib.LogicalUnit) and
+          hasattr(lucls, "HPATH")):
+        if lucls.HTYPE is None:
+          continue
+
+        opcls = lu2opcode.get(lucls, None)
+
+        if opcls:
+          seen_ops.add(opcls.OP_ID)
+          self.assertTrue(opcls.OP_ID in hooks_ops,
+                          msg="Missing hook documentation for %s" %
+                              opcls.OP_ID)
+        self.assertTrue(lucls.HPATH in hooks_paths,
+                        msg="Missing documentation for hook %s/%s" %
+                            (lucls.HTYPE, lucls.HPATH))
+        seen_paths.add(lucls.HPATH)
+
+    missed_ops = hooks_ops - seen_ops
+    missed_paths = hooks_paths - seen_paths - self.HOOK_PATH_OK
+
+    self.assertFalse(missed_ops,
+                     msg="Op documents hook not existing anymore: %s" %
+                         utils.CommaJoin(missed_ops))
+
+    self.assertFalse(missed_paths,
+                     msg="Hook path does not exist in opcode: %s" %
+                         utils.CommaJoin(missed_paths))
 
 
 class TestRapiDocs(unittest.TestCase):
-- 
GitLab