From 0dd63d70841ddd6f483f74b42b0c912969092252 Mon Sep 17 00:00:00 2001
From: Eric Harney <eharney@redhat.com>
Date: Wed, 30 Aug 2017 11:49:38 -0400
Subject: [PATCH] Add contributor doc on assertEqual vs assertFalse

Explain a caveat of using assertFalse.

Change-Id: I2d535477dae071b80b0469f4b51287a49846d01d
---
 cinder/hacking/checks.py           |  6 ++--
 doc/source/contributor/testing.rst | 48 ++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 2 deletions(-)

diff --git a/cinder/hacking/checks.py b/cinder/hacking/checks.py
index 48e9ef8ff08..8b9626fd8af 100644
--- a/cinder/hacking/checks.py
+++ b/cinder/hacking/checks.py
@@ -58,8 +58,6 @@ no_contextlib_nested = re.compile(r"\s*with (contextlib\.)?nested\(")
 logging_instance = re.compile(
     r"(.)*LOG\.(warning|info|debug|error|exception)\(")
 
-assert_None = re.compile(
-    r".*assertEqual\(None, .*\)")
 assert_True = re.compile(
     r".*assertEqual\(True, .*\)")
 
@@ -447,6 +445,10 @@ def no_test_log(logical_line, filename, noqa):
 
 
 def validate_assertTrue(logical_line):
+    # Note: a comparable check cannot be implemented for
+    # assertFalse(), because assertFalse(None) passes.
+    # Therefore, assertEqual(False, value) is required to
+    # have the strongest test.
     if re.match(assert_True, logical_line):
         msg = ("C313: Unit tests should use assertTrue(value) instead"
                " of using assertEqual(True, value).")
diff --git a/doc/source/contributor/testing.rst b/doc/source/contributor/testing.rst
index f7355f03472..328cbf50cbf 100644
--- a/doc/source/contributor/testing.rst
+++ b/doc/source/contributor/testing.rst
@@ -160,6 +160,54 @@ To Fix:
 
     sudo dnf install python3-devel
 
+
+**Assertion types in unit tests**
+
+In general, it is best to use the most specific assertion possible in a unit
+test, to have the strongest validation of code behavior.
+
+For example:
+
+.. code-block:: python
+
+    self.assertEqual("in-use", volume.status)
+
+is preferred over
+
+.. code-block:: python
+
+    self.assertIsNotNone(volume.status)
+
+or
+
+Test methods that implement comparison checks are also generally preferred
+over writing code into assertEqual() or assertTrue().
+
+.. code-block:: python
+
+   self.assertGreater(2, volume.size)
+
+is preferred over
+
+.. code-block:: python
+
+   self.assertTrue(2 > volume.size)
+
+However, assertFalse() behavior is not obvious in this regard.  Since
+``None`` evaluates to ``False`` in Python, the following check will pass when
+x is ``False`` or ``None``.
+
+.. code-block:: python
+
+   self.assertFalse(x)
+
+Therefore, it is preferable to use:
+
+.. code-block:: python
+
+   self.assertEqual(x, False)
+
+
 .. rubric:: Footnotes
 
 .. [#f1] See :doc:`jenkins`.