Merge pull request #1566 from cbowman0/coverage_render_glyph

Adding test coverage for render/glyph.py mark ii

Author: obfuscurity

webapp/graphite/render/glyph.py

@@ -165,6 +165,12 @@ def __init__(self, minValue, maxValue, unitSystem=None):
 
   @staticmethod
   def checkFinite(value, name='value'):
+    """Check that value is a finite number.
+
+    If it is, return it. If not, raise GraphError describing the
+    problem, using name in the error message.
+    """
+
     if math.isnan(value):
       raise GraphError('Encountered NaN %s' % (name,))
     elif math.isinf(value):
@@ -173,12 +179,31 @@ def checkFinite(value, name='value'):
 
   @staticmethod
   def chooseDelta(x):
+    """Choose a reasonable axis range given that one limit is x.
+
+    Given that end of the axis range (i.e., minValue or maxValue) is
+    x, choose a reasonable distance to the other limit.
+    """
+
     if abs(x) < 1.0e-9:
       return 1.0
     else:
       return 0.1 * abs(x)
 
   def reconcileLimits(self):
+    """If self.minValue is not less than self.maxValue, fix the problem.
+
+    If self.minValue is not less than self.maxValue, adjust
+    self.minValue and/or self.maxValue (depending on which was not
+    specified explicitly by the user) to make self.minValue <
+    self.maxValue. If the user specified both limits explicitly, then
+    raise GraphError.
+    """
+
+    if self.minValue < self.maxValue:
+      # The limits are already OK.
+      return
+
     minFixed = (self.minValueSource in ['min'])
     maxFixed = (self.maxValueSource in ['max', 'limit'])
 
@@ -196,6 +221,26 @@ def reconcileLimits(self):
       self.maxValue = average + delta
 
   def applySettings(self, axisMin=None, axisMax=None, axisLimit=None):
+    """Apply the specified settings to this axis.
+
+    Set self.minValue, self.minValueSource, self.maxValue,
+    self.maxValueSource, and self.axisLimit reasonably based on the
+    parameters provided.
+
+    Arguments:
+
+    axisMin -- a finite number, or None to choose a round minimum
+        limit that includes all of the data.
+
+    axisMax -- a finite number, 'max' to use the maximum value
+        contained in the data, or None to choose a round maximum limit
+        that includes all of the data.
+
+    axisLimit -- a finite number to use as an upper limit on maxValue,
+        or None to impose no upper limit.
+
+    """
+
     if axisMin is not None and not math.isnan(axisMin):
       self.minValueSource = 'min'
       self.minValue = self.checkFinite(axisMin, 'axis min')
@@ -206,24 +251,32 @@ def applySettings(self, axisMin=None, axisMax=None, axisLimit=None):
       self.maxValueSource = 'max'
       self.maxValue = self.checkFinite(axisMax, 'axis max')
 
-    if axisLimit is not None and not math.isnan(axisLimit):
-      if axisLimit < self.maxValue:
-        self.maxValue = self.checkFinite(axisLimit, 'axis limit')
-        self.maxValueSource = 'limit'
-        # The limit has already been imposed, so there is no need to
-        # remember it:
-        self.axisLimit = None
-      elif not math.isinf(axisLimit):
-        # We still need to remember axisLimit to avoid rounding top to
-        # a value larger than axisLimit:
-        self.axisLimit = axisLimit
-    else:
+    if axisLimit is None or math.isnan(axisLimit):
       self.axisLimit = None
+    elif axisLimit < self.maxValue:
+      self.maxValue = self.checkFinite(axisLimit, 'axis limit')
+      self.maxValueSource = 'limit'
+      # The limit has already been imposed, so there is no need to
+      # remember it:
+      self.axisLimit = None
+    elif math.isinf(axisLimit):
+      # It must be positive infinity, which is the same as no limit:
+      self.axisLimit = None
+    else:
+      # We still need to remember axisLimit to avoid rounding top to
+      # a value larger than axisLimit:
+      self.axisLimit = axisLimit
 
-    if not (self.minValue < self.maxValue):
-      self.reconcileLimits()
+    self.reconcileLimits()
 
   def makeLabel(self, value):
+    """Create a label for the specified value.
+
+    Create a label string containing the value and its units (if any),
+    based on the values of self.step, self.span, and self.unitSystem.
+
+    """
+
     value, prefix = format_units(value, self.step, system=self.unitSystem)
     span, spanPrefix = format_units(self.span, self.step, system=self.unitSystem)
     if prefix:
@@ -246,11 +299,17 @@ def makeLabel(self, value):
 
 
 class _LinearAxisTics(_AxisTics):
+  """Axis ticmarks with uniform spacing."""
+
   def __init__(self, minValue, maxValue, unitSystem=None):
     _AxisTics.__init__(self, minValue, maxValue, unitSystem=unitSystem)
     self.step = None
+    self.span = None
+    self.binary = None
 
   def setStep(self, step):
+    """Set the size of steps between ticmarks."""
+
     self.step = self.checkFinite(float(step), 'axis step')
 
   def generateSteps(self, minStep):
@@ -279,7 +338,7 @@ def computeSlop(self, step, divisor):
     """Compute the slop that would result from step and divisor.
 
     Return the slop, or None if this combination can't cover the full
-    range.
+    range. See chooseStep() for the definition of "slop".
 
     """
 
@@ -419,6 +478,8 @@ def __init__(self, minValue, maxValue, unitSystem=None, base=10.0):
     if base <= 1.0:
       raise GraphError('Logarithmic base must be greater than one')
     self.base = self.checkFinite(base, 'log base')
+    self.step = None
+    self.span = None
 
   def setStep(self, step):
     # step is ignored for Logarithmic tics:
@@ -1780,7 +1841,7 @@ def toSeconds(t):
 
 
 def safeArgs(args):
-  """Iterate over valid, finite values an in iterable.
+  """Iterate over valid, finite values in an iterable.
 
   Skip any items that are None, NaN, or infinite.
   """
@@ -1862,6 +1923,9 @@ def format_units(v, step=None, system="si"):
     http://en.wikipedia.org/wiki/Binary_prefix
   """
 
+  if v is None:
+    return v, ""
+
   if step is None:
     condition = lambda size: abs(v) >= size
   else:
@@ -1880,6 +1944,11 @@ def format_units(v, step=None, system="si"):
 
 
 def find_x_times(start_dt, unit, step):
+  if not isinstance(start_dt, datetime):
+    raise ValueError("Invalid start_dt: %s" % start_dt)
+  if not isinstance(step, int) or not step > 0:
+    raise ValueError("Invalid step value: %s" % step)
+
   if unit == SEC:
     dt = start_dt.replace(second=start_dt.second - (start_dt.second % step))
     x_delta = timedelta(seconds=step)