wifiblaster: fix flaky test due to random numbers.
WifiblasterController uses random.expovariate to determine the next
measurement time. Exponential distributions can return arbitrarily large
values with vanishingly small but positive probability. This results in
flakiness when wifiblaster_controller_test checks that a measurement
will run within a "long enough" time, since no time is guaranteed to be
long enough.
Also fixed a bug that was introduced in
8c0cae0c66f701f9d83e9bab39bf3baa6417bf99 but went undetected due to a
bug in wifiblaster_controller_test. Measurements are supposed to start
one interval after they are enabled, but the test allowed them to start
immediately.
Change-Id: Iae4eda91d4aee0ade7af2100ff5ed180b841daf4
diff --git a/waveguide/waveguide.py b/waveguide/waveguide.py
index cbc9fe8..87324c3 100755
--- a/waveguide/waveguide.py
+++ b/waveguide/waveguide.py
@@ -907,7 +907,8 @@
wifiblaster.enable Enable WiFi performance measurement.
wifiblaster.interval Average time between automated measurements in
seconds, or 0 to disable automated measurements.
- wifiblaster.measureall Unix time at which to measure all clients.
+ wifiblaster.measureall Unix time at which to measure all clients, or 0
+ to disable measureall requests.
wifiblaster.onassociation Enable WiFi performance measurement after clients
associate.
@@ -938,6 +939,20 @@
except ValueError:
return None
+ def _GetParameters(self):
+ """Reads and returns all parameters if valid, or Nones."""
+ duration = self._ReadParameter('duration', float)
+ enable = self._ReadParameter('enable', self._StrToBool)
+ fraction = self._ReadParameter('fraction', int)
+ interval = self._ReadParameter('interval', float)
+ measureall = self._ReadParameter('measureall', float)
+ onassociation = self._ReadParameter('onassociation', self._StrToBool)
+ size = self._ReadParameter('size', int)
+ if (duration > 0 and enable and fraction > 0 and interval >= 0
+ and measureall >= 0 and size > 0):
+ return (duration, fraction, interval, measureall, onassociation, size)
+ return (None, None, None, None, None, None)
+
def _SaveResult(self, line):
"""Save wifiblaster result to the status file for that client."""
g = re.search(MACADDR_REGEX, line)
@@ -975,23 +990,18 @@
"""Return the time of the next measurement event."""
return self._next_measurement_time
- def Measure(self, interface, client):
+ def Measure(self, interface, client, duration, fraction, size):
"""Measures the performance of a client."""
- enable = self._ReadParameter('enable', self._StrToBool)
- duration = self._ReadParameter('duration', float)
- fraction = self._ReadParameter('fraction', int)
- size = self._ReadParameter('size', int)
- if enable and duration > 0 and fraction > 0 and size > 0:
- RunProc(callback=self._HandleResults,
- args=[WIFIBLASTER_BIN, '-i', interface, '-d', str(duration),
- '-f', str(fraction), '-s', str(size),
- helpers.DecodeMAC(client)])
+ RunProc(callback=self._HandleResults,
+ args=[WIFIBLASTER_BIN, '-i', interface, '-d', str(duration),
+ '-f', str(fraction), '-s', str(size),
+ helpers.DecodeMAC(client)])
def MeasureOnAssociation(self, interface, client):
"""Measures the performance of a client after association."""
- onassociation = self._ReadParameter('onassociation', self._StrToBool)
+ (duration, fraction, _, _, onassociation, size) = self._GetParameters()
if onassociation:
- self.Measure(interface, client)
+ self.Measure(interface, client, duration, fraction, size)
def Poll(self, now):
"""Polls the state machine."""
@@ -1005,29 +1015,33 @@
# Inter-arrival times in a Poisson process are exponentially distributed.
# The timebase slip prevents a burst of measurements in case we fall
# behind.
- self._next_measurement_time = now + random.expovariate(1 / interval)
+ self._next_measurement_time = now + random.expovariate(1.0 / interval)
- interval = self._ReadParameter('interval', float)
- if interval <= 0:
+ # Read parameters.
+ (duration, fraction, interval, measureall, _, size) = self._GetParameters()
+
+ # Handle automated mode.
+ if interval > 0:
+ if self._interval != interval:
+ # Enable or change interval.
+ StartMeasurementTimer(interval)
+ elif now >= self._next_measurement_time:
+ # Measure a random client.
+ StartMeasurementTimer(interval)
+ try:
+ (interface, client) = random.choice(self._GetAllClients())
+ except IndexError:
+ pass
+ else:
+ self.Measure(interface, client, duration, fraction, size)
+ else:
Disable()
- elif self._interval != interval:
- # Enable or change interval.
- StartMeasurementTimer(interval)
- elif now >= self._next_measurement_time:
- # Measure a random client.
- StartMeasurementTimer(interval)
- try:
- (interface, client) = random.choice(self._GetAllClients())
- except IndexError:
- pass
- else:
- self.Measure(interface, client)
- measureall = self._ReadParameter('measureall', float)
+ # Handle measureall request.
if time.time() >= measureall and measureall > self._last_measureall_time:
self._last_measureall_time = measureall
for (interface, client) in self._GetAllClients():
- self.Measure(interface, client)
+ self.Measure(interface, client, duration, fraction, size)
# Poll again in at most one second. This allows parameter changes (e.g. a
# measureall request or a long interval to a short interval) to take effect
diff --git a/waveguide/wifiblaster_controller_test.py b/waveguide/wifiblaster_controller_test.py
index 6b562b1..9e300f2 100755
--- a/waveguide/wifiblaster_controller_test.py
+++ b/waveguide/wifiblaster_controller_test.py
@@ -17,6 +17,7 @@
import glob
import os
+import random
import shutil
import sys
import tempfile
@@ -102,6 +103,7 @@
d = tempfile.mkdtemp()
old_wifiblaster_dir = waveguide.WIFIBLASTER_DIR
waveguide.WIFIBLASTER_DIR = tempfile.mkdtemp()
+ oldexpovariate = random.expovariate
oldpath = os.environ['PATH']
oldtime = time.time
@@ -110,6 +112,7 @@
return faketime[0]
try:
+ random.expovariate = lambda lambd: random.uniform(0, 2 * 1.0 / lambd)
time.time = FakeTime
os.environ['PATH'] = 'fake:' + os.environ['PATH']
sys.path.insert(0, 'fake')
@@ -141,11 +144,6 @@
WriteConfig('measureall', '0')
WriteConfig('size', '1470')
- # Disabled. No measurements should be run.
- print manager.GetState()
- for t in xrange(0, 100):
- wc.Poll(t)
-
def CountRuns():
try:
v = open('fake/wifiblaster.out').readlines()
@@ -155,16 +153,24 @@
os.unlink('fake/wifiblaster.out')
return len(v)
- CountRuns() # get rid of any leftovers
+ # Get rid of any leftovers.
+ CountRuns()
+
+ # Disabled.
+ # No measurements should be run.
+ print manager.GetState()
+ for t in xrange(0, 100):
+ wc.Poll(t)
wvtest.WVPASSEQ(CountRuns(), 0)
+ # Enabled.
# The first measurement should be one cycle later than the start time.
# This is not an implementation detail: it prevents multiple APs from
# running simultaneous measurements if measurements are enabled at the
# same time.
WriteConfig('enable', 'True')
wc.Poll(100)
- wvtest.WVPASSGE(wc.NextMeasurement(), 100)
+ wvtest.WVPASSGT(wc.NextMeasurement(), 100)
for t in xrange(101, 200):
wc.Poll(t)
wvtest.WVPASSGE(CountRuns(), 1)
@@ -183,20 +189,16 @@
wc.Poll(t)
wvtest.WVPASSGE(CountRuns(), 1)
- # Run the measurement at t=400 to restart the timer.
- wc.Poll(400)
- wvtest.WVPASSGE(CountRuns(), 0)
-
# Next poll should be in at most one second regardless of interval.
- wvtest.WVPASSLE(wc.NextTimeout(), 401)
+ wvtest.WVPASSLE(wc.NextTimeout(), 400)
- # Enabled with longer average interval. The change in interval should
+ # Enabled with shorter average interval. The change in interval should
# trigger a change in next poll timeout.
WriteConfig('interval', '0.5')
old_to = wc.NextMeasurement()
- wc.Poll(401)
+ wc.Poll(400)
wvtest.WVPASSNE(old_to, wc.NextMeasurement())
- for t in xrange(402, 500):
+ for t in xrange(401, 500):
wc.Poll(t)
wvtest.WVPASSGE(CountRuns(), 1)
@@ -218,6 +220,7 @@
manager.GetState().assoc[0].mac)
wvtest.WVPASSEQ(CountRuns(), 1)
finally:
+ random.expovariate = oldexpovariate
time.time = oldtime
shutil.rmtree(d)
os.environ['PATH'] = oldpath
