Merge lp:~roadmr/checkbox/1185508-memory-compare-units into lp:checkbox

[Daniel Manrique]

providers:checkbox: Improved memory size presentation in memory_compare.

providers:checkbox: Improved memory size presentation in memory_compare.

Sizes will always look like:

X.XX$UNIT

where UNIT can be bytes or QiB (where Q in turn is the appropriate metric prefix).

For instance:

456.00 bytes (the decimal number is redundant; if this is too ugly let me know and I can fix it, though it would balloon the dead-simple method to about twice its size :D)

7.56GiB

1023.99TiB (e.g. if I add 0.01 TiB here, it would become 1.00 PiB; also, if the quantity is already an integer I can remove the decimals, at the expense of more time and code).

[Zygmunt Krynicki (zyga)]

(venv)zyga@silverblade:/ramdisk/1185508-memory-compare-units/providers/plainbox-provider-checkbox$ ./bin/memory_compare
Results:
 /proc/meminfo reports: 15.37GiB
 lshw reports: 15.37GiB
Traceback (most recent call last):
  File "./bin/memory_compare", line 92, in bytes_to_human
    exponent = log(my_bytes, 1024)
ValueError: math domain error

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "./bin/memory_compare", line 140, in <module>
    sys.exit(main())
  File "./bin/memory_compare", line 125, in main
    "%d%% variance allowed." % (bytes_to_human(difference), percentage, threshold))
  File "./bin/memory_compare", line 94, in bytes_to_human
    raise ValueError("cannot compute base-1024 logarithm of {!r}: {}".format(my_bytes, exc))
ValueError: cannot compute base-1024 logarithm of 0: math domain error

Hmm?

review: Needs Fixing

[Zygmunt Krynicki (zyga)]

That was with the following delta:

(venv)zyga@silverblade:/ramdisk/1185508-memory-compare-units/providers/plainbox-provider-checkbox$ bzr diff
=== modified file 'providers/plainbox-provider-checkbox/bin/memory_compare'
--- providers/plainbox-provider-checkbox/bin/memory_compare 2014-09-03 22:04:18 +0000
+++ providers/plainbox-provider-checkbox/bin/memory_compare 2014-09-03 22:35:14 +0000
@@ -88,7 +88,10 @@
     suffixes = ["bytes", "KiB", "MiB", "GiB", "TiB",
                 "PiB", "EiB", "ZiB", "YiB"]
     # my_bytes' base-1024 logarithm.
- exponent = log(my_bytes, 1024)
+ try:
+ exponent = log(my_bytes, 1024)
+ except ValueError as exc:
+ raise ValueError("cannot compute base-1024 logarithm of {!r}: {}".format(my_bytes, exc))
     suffix = suffixes[int(exponent)]
     scalar = my_bytes / (1024**int(exponent))

@@ -96,10 +99,6 @@

 def main():
- if os.geteuid() != 0:
- print("This script must be run as root.", file=sys.stderr)
- return 1
-
     installed_memory = get_installed_memory_size()
     visible_memory = get_visible_memory_size()
     threshold = get_threshold(installed_memory)

[Daniel Manrique (roadmr)]

Wow :) Passing '0' to the function is possible (as you saw) and easy to handle, but mathematically the logarithm will also barf if I pass a negative value; how to handle this is less clear, and though it *should* not happen, it could happen if for some reason reported memory is less than visible memory. Perhaps we should catch that possibility earlier; or strip the sign before doing the calculation and then reattaching it before outputting the string.

I'll think of a good way to handle that case, will resubmit soon.

[lp:~roadmr/checkbox/1185508-memory-compare-units]

3245. By Daniel Manrique on 2014-09-04

providers:checkbox: Improved memory size presentation in memory_compare

[Daniel Manrique (roadmr)]

Fixed, it should now handle both 0-byte and negative sizes correctly (in addition it won't crash if you give something larger than 1024 Yottabytes, which is unlikely, but at least it will now output something sensible).

[Zygmunt Krynicki (zyga)]

Thanks :-)

review: Approve

=== modified file 'providers/plainbox-provider-checkbox/bin/memory_compare'
--- providers/plainbox-provider-checkbox/bin/memory_compare	2014-08-11 21:21:14 +0000
+++ providers/plainbox-provider-checkbox/bin/memory_compare	2014-09-04 14:27:47 +0000
@@ -24,6 +24,7 @@
 
 import os
 import sys
+from math import log, copysign
 from subprocess import check_output, PIPE
 
 from checkbox_support.parsers.lshwjson import LshwJsonParser
@@ -81,6 +82,32 @@
         return 10
 
 
+def bytes_to_human(my_bytes):
+    """ Convert my_bytes to a scaled representation with a
+    suffix
+    """
+    if my_bytes == 0:
+        return "0 bytes"
+
+    suffixes = ["bytes", "KiB", "MiB", "GiB", "TiB",
+                "PiB", "EiB", "ZiB", "YiB"]
+
+    try:
+        sign = copysign(1, my_bytes)
+    except OverflowError as excp:
+        return "(Number too large: {})".format(excp)
+    my_bytes = abs(my_bytes)
+    # my_bytes' base-1024 logarithm.
+    exponent = log(my_bytes, 1024)
+    try:
+        suffix = suffixes[int(exponent)]
+    except IndexError:
+        return "(Number too large)"
+    scalar = my_bytes / (1024**int(exponent))
+
+    return "{:.2f} {}".format(sign * scalar, suffix)
+
+
 def main():
     if os.geteuid() != 0:
         print("This script must be run as root.", file=sys.stderr)
@@ -95,9 +122,11 @@
         percentage = difference / installed_memory * 100
     except ZeroDivisionError:
         print("Results:")
-        print("\t/proc/meminfo reports:\t%s kB" % (visible_memory / 1024),
+        print("\t/proc/meminfo reports:\t{}".format(
+              bytes_to_human(visible_memory)),
               file=sys.stderr)
-        print("\tlshw reports:\t%s kB" % (installed_memory / 1024),
+        print("\tlshw reports:\t{}".format(
+              bytes_to_human(installed_memory)),
               file=sys.stderr)
         print("\nFAIL: Either lshw or /proc/meminfo returned a memory size "
               "of 0 kB", file=sys.stderr)
@@ -105,17 +134,20 @@
 
     if percentage <= threshold:
         print("Results:")
-        print("\t/proc/meminfo reports:\t%s kB" % (visible_memory / 1024))
-        print("\tlshw reports:\t%s kB" % (installed_memory / 1024))
-        print("\nPASS: Meminfo reports %d bytes less than lshw, a "
+        print("\t/proc/meminfo reports:\t{}".format(
+              bytes_to_human(visible_memory)))
+        print("\tlshw reports:\t{}".format(bytes_to_human(installed_memory)))
+        print("\nPASS: Meminfo reports %s less than lshw, a "
               "difference of %.2f%%. This is less than the "
-              "%d%% variance allowed." % (difference, percentage, threshold))
+              "%d%% variance allowed." % (bytes_to_human(difference),
+                                          percentage, threshold))
         return 0
     else:
         print("Results:", file=sys.stderr)
-        print("\t/proc/meminfo reports:\t%s kB" % (visible_memory / 1024),
+        print("\t/proc/meminfo reports:\t{}".format(
+              bytes_to_human(visible_memory)),
               file=sys.stderr)
-        print("\tlshw reports:\t%s kB" % (installed_memory / 1024),
+        print("\tlshw reports:\t{}".format(bytes_to_human(installed_memory)),
               file=sys.stderr)
         print("\nFAIL: Meminfo reports %d bytes less than lshw, "
               "a difference of %.2f%%. Only a variance of %d%% in "