Merge lp:~cr3/checkbox/network_bandwidth_test into lp:checkbox

[Marc Tardif]

Tested on Precise:

$ sudo PYTHONPATH=. ./scripts/network_bandwidth_test
$ echo $?
0

[Javier Collado (javier.collado)]

Looking at the script and the checkbox jobs that use it, I have a couple of comments:

- It looks like the requirement for package.name == 'nmap' is missing for the jobs generated by networking/multi_nic, that is, for networking/multi_nic_$2.

- I don't like much this regular expression pattern:
alpha_numeric = r"[^ ]+"

because it allows other whitespace characters like \t. I'd say that a better choice would be just r"\S".

- One final comment, just for my own curiosity (not really relevant for this merge request). In networking/bandwidth I see:
requires:
 package.name == 'linux'
 package.name == 'ethtool' and package.name == 'nmap'
 device.category == 'NETWORK'

and in networking/multi_nic_$2 I see:
requires: device.path == "$1" and package.name == 'linux' and package.name == 'ethtool'

My understanding is that multiline requirements are evaluated as if they were linked with an 'and' operator. Is that the case? Is there a practical difference between using multiline vs. single line requirements?

review: Needs Information

[Javier Collado (javier.collado)]

> - I don't like much this regular expression pattern:
> alpha_numeric = r"[^ ]+"
>
> because it allows other whitespace characters like \t. I'd say that a better
> choice would be just r"\S".

I meant r"\S+" to be consistent with the original pattern, not just r"\S".

[lp:~cr3/checkbox/network_bandwidth_test]

1432. By Marc Tardif on 2012-06-13

Fixed the requires line in the networking/multi_nic_$2 job to include the nmap package name.

1433. By Marc Tardif on 2012-06-13

Renamed the regular expression called alpha_numeric to non_space.

[Marc Tardif (cr3)]

1. Fixed the requires line in the networking/multi_nic_$2 job to include the nmap package name.

2. The non-space regular expression is actually correct considering the output that it parses from commands like ifconfig for example. These commands separate tokens with the space character that couldn't be anything else like a tab for example. Also, a token might contain a dash which is not included in the \S regular expression. So, I have renamed the variable to be less confusing becuase you were right that alpha_numeric should be represented with \S. Instead, I have renamed the variable to non_space.

3. One of the reasons to order requires on separate lines is to determine the appropriate package when reporting a bug for a test that failed. It does a best effort based on the device category but also on the package name, where the first package takes precedence over the subsequent ones in the list.

review: Needs Resubmitting

[Javier Collado (javier.collado)]

Thanks for those changes.

I ran the script and checked with flake8 and didn't find any python3 related issue.

review: Approve

=== modified file 'debian/changelog'
--- debian/changelog	2012-06-12 21:13:14 +0000
+++ debian/changelog	2012-06-13 14:09:26 +0000
@@ -18,6 +18,7 @@
     test.
   * [FEATURE] Python 2 to 3 conversion:
     * scripts/cking_suite
+    * scripts/network_bandwidth_test
   * Removed sleep_test script no longer used by any test definition.
 
   [Daniel Manrique]

=== modified file 'jobs/networking.txt.in'
--- jobs/networking.txt.in	2012-04-11 22:53:52 +0000
+++ jobs/networking.txt.in	2012-06-13 14:09:26 +0000
@@ -110,7 +110,10 @@
  cat <<'EOF' | run_templates -s 'udev_resource | filter_templates -w "category=NETWORK" | awk "/path: / { print \$2 }" | xargs -n 1 sh -c "for i in \`ls /sys\$0/net 2>/dev/null\`; do echo \$0 \$i; done"'
  plugin: shell
  name: networking/multi_nic_$2
- requires: device.path == "$1" and package.name == 'linux' and package.name == 'ethtool'
+ requires:
+  package.name == 'linux'
+  package.name == 'ethtool' and package.name == 'nmap'
+  device.path == "$1"
  user: root
  command: ethtool $2 | tail -1 | grep "detected: yes" && network_bandwidth_test --interface=$2 --scan=3 --log-level=debug
  description:

=== modified file 'scripts/network_bandwidth_test'
--- scripts/network_bandwidth_test	2012-05-22 18:01:21 +0000
+++ scripts/network_bandwidth_test	2012-06-13 14:09:26 +0000
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python3
 
 import os
 import re
@@ -50,7 +50,7 @@
     output_patterns = {}
 
     # Convenient output patterns
-    alpha_numeric = r"[^ ]+"
+    non_space = r"[^ ]+"
 
     def __init__(self, *arguments, **options):
         if len(arguments) != self.argument_count:
@@ -107,10 +107,10 @@
             stderr=subprocess.PIPE)
         error = process.stderr.read()
         if error:
-            raise CommandException(error)
+            raise CommandException(error.decode("utf-8"))
 
         output = process.stdout.read()
-        return self.parse_output(output)
+        return self.parse_output(output.decode("utf-8"))
 
 
 class NetworkConfigOutput(CommandOutput):
@@ -142,12 +142,12 @@
 
     output_factory = NetworkConfigOutput
     output_patterns = {
-        "name": r"(%s).*Link encap" % Command.alpha_numeric,
+        "name": r"(%s).*Link encap" % Command.non_space,
         "broadcast": r"Bcast:(%s)" % ipv4,
         "collisions": "collisions:(\d+)",
         "hwaddr": r"HWaddr (%s)" % mac_address,
         "inet_addr": r"inet addr:(%s)" % ipv4,
-        "link_encap": r"Link encap:(%s)" % Command.alpha_numeric,
+        "link_encap": r"Link encap:(%s)" % Command.non_space,
         "netmask": r"Mask:(%s)" % ipv4,
         "metric": r"Metric:(\d+)",
         "mtu": r"MTU:(\d+)",
@@ -227,19 +227,19 @@
 
     fraction = r"\d+(/\d+)?"
     numeric = r"[\d\.]+"
-    numeric_with_unit = r"%s( %s)?" % (numeric, Command.alpha_numeric)
+    numeric_with_unit = r"%s( %s)?" % (numeric, Command.non_space)
 
     output_patterns = {
         "access_point": r"Access Point: (.*)",
         "bit_rate": r"Bit Rate[=:](%s)/s" % numeric_with_unit,
-        "channel": r"Channel=(%s)" % Command.alpha_numeric,
+        "channel": r"Channel=(%s)" % Command.non_space,
         "essid": r"ESSID:\"?([^\"]+)\"?",
         "fragment_thr": r"Fragment thr:(\w+)",
         "frequency": r"Frequency:(%s)" % numeric_with_unit,
         "invalid_misc": r"Invalid misc:(\d+)",
         "link_quality": r"Link Quality[=:](%s)" % fraction,
         "missed_beacon": r"Missed beacon:(\d+)",
-        "mode": r"Mode:(%s)" % Command.alpha_numeric,
+        "mode": r"Mode:(%s)" % Command.non_space,
         "noise_level": r"Noise level[=:](%s)" % numeric_with_unit,
         "power_management": r"Power Management:(.*)",
         "retry_limit": r"Retry limit:(\w+)",
@@ -384,7 +384,7 @@
         return ret
 
     def count_0_bits(self):
-        num = long(self.binary)
+        num = int(self.binary)
         if num < 0:
             raise ValueError("Only positive Numbers please: %s" % (num))
         ret = 0
@@ -407,7 +407,7 @@
         return "%s/%s" % (self.address, self.prefix)
 
     def _check_netmask(self, masklen):
-        num = long(self.netmask.binary)
+        num = int(self.netmask.binary)
         bits = masklen
 
         # remove zero bits at the end