tests: clean up logging

Tests currently expect the logger to be poked from run_tests.py.
The tests should run without any magic values.  This change sets a default
null logger and removes the hasattr checks for the logger.

For reference, see: https://docs.python-guide.org/writing/logging/

Type: test

Change-Id: I98f953d73d12d00e74b59c94a0fb8c7a625b9c44
Signed-off-by: Paul Vinciguerra <pvinci@vinciconsulting.com>

diff --git a/test/framework.py b/test/framework.py
index 9eea8bb..c9d72a7 100644 (file)
--- a/test/framework.py
+++ b/test/framework.py
@@ -2,6 +2,7 @@
 
 from __future__ import print_function
 import gc
+import logging
 import sys
 import os
 import select
@@ -52,6 +53,12 @@ try:
 except NameError:
     pass
 
+logger = logging.getLogger(__name__)
+
+# Set up an empty logger for the testcase that can be overridden as necessary
+null_logger = logging.getLogger('VppTestCase')
+null_logger.addHandler(logging.NullHandler())
+
 PASS = 0
 FAIL = 1
 ERROR = 2
@@ -275,6 +282,7 @@ class VppTestCase(unittest.TestCase):
 
     extra_vpp_punt_config = []
     extra_vpp_plugin_config = []
+    logger = null_logger
     vapi_response_timeout = 5
 
     @property
@@ -1147,17 +1155,16 @@ class VppTestCase(unittest.TestCase):
                 time.sleep(0)
             return
 
-        if hasattr(cls, 'logger'):
-            cls.logger.debug("Starting sleep for %es (%s)", timeout, remark)
+        cls.logger.debug("Starting sleep for %es (%s)", timeout, remark)
         before = time.time()
         time.sleep(timeout)
         after = time.time()
-        if hasattr(cls, 'logger') and after - before > 2 * timeout:
+        if after - before > 2 * timeout:
             cls.logger.error("unexpected self.sleep() result - "
                              "slept for %es instead of ~%es!",
                              after - before, timeout)
-        if hasattr(cls, 'logger'):
-            cls.logger.debug(
+
+        cls.logger.debug(
                 "Finished sleep (%s) - slept %es (wanted %es)",
                 remark, after - before, timeout)
 
@@ -1297,22 +1304,20 @@ class VppTestResult(unittest.TestResult):
                     failed_dir,
                     '%s-FAILED' %
                     os.path.basename(self.current_test_case_info.tempdir))
-                if self.current_test_case_info.logger:
-                    self.current_test_case_info.logger.debug(
+
+                self.current_test_case_info.logger.debug(
                         "creating a link to the failed test")
-                    self.current_test_case_info.logger.debug(
+                self.current_test_case_info.logger.debug(
                         "os.symlink(%s, %s)" %
                         (self.current_test_case_info.tempdir, link_path))
                 if os.path.exists(link_path):
-                    if self.current_test_case_info.logger:
-                        self.current_test_case_info.logger.debug(
+                    self.current_test_case_info.logger.debug(
                             'symlink already exists')
                 else:
                     os.symlink(self.current_test_case_info.tempdir, link_path)
 
             except Exception as e:
-                if self.current_test_case_info.logger:
-                    self.current_test_case_info.logger.error(e)
+                self.current_test_case_info.logger.error(e)
 
     def send_result_through_pipe(self, test, result):
         if hasattr(self, 'test_framework_result_pipe'):

diff --git a/test/util.py b/test/util.py
index c86d602..c7e4693 100644 (file)
--- a/test/util.py
+++ b/test/util.py
@@ -2,6 +2,7 @@
 
 import abc
 import ipaddress
+import logging
 import socket
 from socket import AF_INET6
 import six
@@ -20,6 +21,10 @@ from scapy.utils6 import in6_mactoifaceid
 from io import BytesIO
 from vpp_papi import mac_pton
 
+# Set up an empty logger for the testcase that can be overridden as necessary
+null_logger = logging.getLogger('VppTestCase.util')
+null_logger.addHandler(logging.NullHandler())
+
 
 def ppp(headline, packet):
     """ Return string containing the output of scapy packet.show() call. """
@@ -272,20 +277,7 @@ class L4_CONN_SIDE:
     L4_CONN_SIDE_ONE = 1
 
 
-class LoggerWrapper(object):
-    def __init__(self, logger=None):
-        self._logger = logger
-
-    def debug(self, *args, **kwargs):
-        if self._logger:
-            self._logger.debug(*args, **kwargs)
-
-    def error(self, *args, **kwargs):
-        if self._logger:
-            self._logger.error(*args, **kwargs)
-
-
-def fragment_rfc791(packet, fragsize, _logger=None):
+def fragment_rfc791(packet, fragsize, logger=null_logger):
     """
     Fragment an IPv4 packet per RFC 791
     :param packet: packet to fragment
@@ -293,7 +285,6 @@ def fragment_rfc791(packet, fragsize, _logger=None):
     :note: IP options are not supported
     :returns: list of fragments
     """
-    logger = LoggerWrapper(_logger)
     logger.debug(ppp("Fragmenting packet:", packet))
     packet = packet.__class__(scapy.compat.raw(packet))  # recalc. all values
     if len(packet[IP].options) > 0:
@@ -325,13 +316,13 @@ def fragment_rfc791(packet, fragsize, _logger=None):
     p[IP].frag = fo + nfb
     del p[IP].chksum
 
-    more_fragments = fragment_rfc791(p, fragsize, _logger)
+    more_fragments = fragment_rfc791(p, fragsize, logger)
     pkts.extend(more_fragments)
 
     return pkts
 
 
-def fragment_rfc8200(packet, identification, fragsize, _logger=None):
+def fragment_rfc8200(packet, identification, fragsize, logger=null_logger):
     """
     Fragment an IPv6 packet per RFC 8200
     :param packet: packet to fragment
@@ -339,7 +330,6 @@ def fragment_rfc8200(packet, identification, fragsize, _logger=None):
     :note: IP options are not supported
     :returns: list of fragments
     """
-    logger = LoggerWrapper(_logger)
     packet = packet.__class__(scapy.compat.raw(packet))  # recalc. all values
     if len(packet) <= fragsize:
         return [packet]