Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
10829203:
GetServicePath should perform a scan (Closed)
|
Created:
8 years, 4 months ago by stanleyw Modified:
8 years, 4 months ago CC:
chromium-reviews, dennis_jeffrey, anantha, dyu1, Nirnimesh Base URL:
https://git.chromium.org/git/chromium/src@master Visibility:
Public. |
DescriptionGetServicePath should perform a scan
Sometimes the scan list is not updated. It makes sense
to perform a scan until we see the service path or until a timeout
period is hit.
TEST=run chromeos_wifi_functional
BUG=None
Change-Id: Ia1afd538618a79d10aaab2a0cec12377cc6477b9
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=152022
Patch Set 1 #
Total comments: 2
Patch Set 2 : Rate limiter #
Total comments: 6
Patch Set 3 : Fixed WaitUntil to work with GetServicePath #
Total comments: 2
Patch Set 4 : WaitUntil only returns retval #
Total comments: 1
Messages
Total messages: 15 (0 generated)
The network list is not always up to date so we should perform a network scan whenever we try to call GetServicePath. PTAL
http://codereview.chromium.org/10829203/diff/1/chrome/test/pyautolib/pyauto.py File chrome/test/pyautolib/pyauto.py (right): http://codereview.chromium.org/10829203/diff/1/chrome/test/pyautolib/pyauto.p... chrome/test/pyautolib/pyauto.py:5081: while time.time() < end_time: How fast does this spin? i.e. are we spiking the CPU and we burn through this loop or is NetworkScan slow enough we don't own the device?
Rate limit using a sleep. None of the calls in between appear to be blocking so yes, it would spin really fast, not that it had any effect while testing, but we should probably slow it down. http://codereview.chromium.org/10829203/diff/1/chrome/test/pyautolib/pyauto.py File chrome/test/pyautolib/pyauto.py (right): http://codereview.chromium.org/10829203/diff/1/chrome/test/pyautolib/pyauto.p... chrome/test/pyautolib/pyauto.py:5081: while time.time() < end_time: On 2012/08/06 23:39:13, krisr wrote: > How fast does this spin? i.e. are we spiking the CPU and we burn through this > loop or is NetworkScan slow enough we don't own the device? Done.
http://codereview.chromium.org/10829203/diff/4001/chrome/test/pyautolib/pyaut... File chrome/test/pyautolib/pyauto.py (right): http://codereview.chromium.org/10829203/diff/4001/chrome/test/pyautolib/pyaut... chrome/test/pyautolib/pyauto.py:5068: """Returns the service path associated with an SSID. Update to mention "Wait until" http://codereview.chromium.org/10829203/diff/4001/chrome/test/pyautolib/pyaut... chrome/test/pyautolib/pyauto.py:5081: while time.time() < end_time: Do not reinvent polling loop. Use WaitUntil()
http://codereview.chromium.org/10829203/diff/4001/chrome/test/pyautolib/pyaut... File chrome/test/pyautolib/pyauto.py (right): http://codereview.chromium.org/10829203/diff/4001/chrome/test/pyautolib/pyaut... chrome/test/pyautolib/pyauto.py:5081: while time.time() < end_time: I did initially. Unfortunately, I need a particular piece of data from the loop, specifically service_path that made the call to WaitUntil either somewhat clunky or I would have to repeat the loop to obtain this information. On 2012/08/07 23:44:50, Nirnimesh wrote: > Do not reinvent polling loop. Use WaitUntil()
http://codereview.chromium.org/10829203/diff/4001/chrome/test/pyautolib/pyaut... File chrome/test/pyautolib/pyauto.py (right): http://codereview.chromium.org/10829203/diff/4001/chrome/test/pyautolib/pyaut... chrome/test/pyautolib/pyauto.py:5081: while time.time() < end_time: On 2012/08/08 00:35:17, stanleyw wrote: > I did initially. Unfortunately, I need a particular piece of data from the > loop, specifically service_path that made the call to WaitUntil either somewhat > clunky or I would have to repeat the loop to obtain this information. > > On 2012/08/07 23:44:50, Nirnimesh wrote: > > Do not reinvent polling loop. Use WaitUntil() > WaitUntil returns the value that the callable returns. def _get_it(): service_list = self.GetNetworkInfo().get('wifi_networks', []) for service_path, service_obj in service_list.iteritems(): ... ... if ... return service_path return None service_path = self.WaitUntil(_get_it, timeout=timeout, retry_sleep=1)
WaitUntil wasn't returning the return value of |function| so I added a flag keep_retval to return either True (some tests rely on this being True), or retval depending on whether or not the flag was set. Also fixed the logic so expected_retval so only performs exected_retval equivalence to non-none values of expected_retval. Updated GetServicePath to use WaitUntil. On 2012/08/08 00:48:26, Nirnimesh wrote: > http://codereview.chromium.org/10829203/diff/4001/chrome/test/pyautolib/pyaut... > File chrome/test/pyautolib/pyauto.py (right): > > http://codereview.chromium.org/10829203/diff/4001/chrome/test/pyautolib/pyaut... > chrome/test/pyautolib/pyauto.py:5081: while time.time() < end_time: > On 2012/08/08 00:35:17, stanleyw wrote: > > I did initially. Unfortunately, I need a particular piece of data from the > > loop, specifically service_path that made the call to WaitUntil either > somewhat > > clunky or I would have to repeat the loop to obtain this information. > > > > On 2012/08/07 23:44:50, Nirnimesh wrote: > > > Do not reinvent polling loop. Use WaitUntil() > > > > WaitUntil returns the value that the callable returns. > > def _get_it(): > service_list = self.GetNetworkInfo().get('wifi_networks', []) > for service_path, service_obj in service_list.iteritems(): > ... > ... > if ... > return service_path > return None > > service_path = self.WaitUntil(_get_it, timeout=timeout, retry_sleep=1)
(see prev comment for changes) http://codereview.chromium.org/10829203/diff/4001/chrome/test/pyautolib/pyaut... File chrome/test/pyautolib/pyauto.py (right): http://codereview.chromium.org/10829203/diff/4001/chrome/test/pyautolib/pyaut... chrome/test/pyautolib/pyauto.py:5068: """Returns the service path associated with an SSID. On 2012/08/07 23:44:50, Nirnimesh wrote: > Update to mention "Wait until" Done. http://codereview.chromium.org/10829203/diff/4001/chrome/test/pyautolib/pyaut... chrome/test/pyautolib/pyauto.py:5081: while time.time() < end_time: Re-did WaitUntil to do this. On 2012/08/08 00:48:26, Nirnimesh wrote: > On 2012/08/08 00:35:17, stanleyw wrote: > > I did initially. Unfortunately, I need a particular piece of data from the > > loop, specifically service_path that made the call to WaitUntil either > somewhat > > clunky or I would have to repeat the loop to obtain this information. > > > > On 2012/08/07 23:44:50, Nirnimesh wrote: > > > Do not reinvent polling loop. Use WaitUntil() > > > > WaitUntil returns the value that the callable returns. > > def _get_it(): > service_list = self.GetNetworkInfo().get('wifi_networks', []) > for service_path, service_obj in service_list.iteritems(): > ... > ... > if ... > return service_path > return None > > service_path = self.WaitUntil(_get_it, timeout=timeout, retry_sleep=1)
http://codereview.chromium.org/10829203/diff/8002/chrome/test/pyautolib/pyaut... File chrome/test/pyautolib/pyauto.py (right): http://codereview.chromium.org/10829203/diff/8002/chrome/test/pyautolib/pyaut... chrome/test/pyautolib/pyauto.py:813: return {True: retval, False: True}[keep_retval] I think it'll be fine if you just return retval here (ie don't add the option in line 761). The truth value still evaluates to True, so users will continue to work
http://codereview.chromium.org/10829203/diff/8002/chrome/test/pyautolib/pyaut... File chrome/test/pyautolib/pyauto.py (right): http://codereview.chromium.org/10829203/diff/8002/chrome/test/pyautolib/pyaut... chrome/test/pyautolib/pyauto.py:813: return {True: retval, False: True}[keep_retval] On 2012/08/08 19:51:13, Nirnimesh wrote: > I think it'll be fine if you just return retval here (ie don't add the option in > line 761). The truth value still evaluates to True, so users will continue to > work Done.
LGTM http://codereview.chromium.org/10829203/diff/9002/chrome/test/pyautolib/pyaut... File chrome/test/pyautolib/pyauto.py (right): http://codereview.chromium.org/10829203/diff/9002/chrome/test/pyautolib/pyaut... chrome/test/pyautolib/pyauto.py:798: True. append: (or matches expect_retval)
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/stanleyw@chromium.org/10829203/9002
Try job failure for 10829203-9002 (retry) on android for steps "compile, build" (clobber build). It's a second try, previously, steps "compile, build" failed. http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=android&nu...
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/stanleyw@chromium.org/10829203/9002
Change committed as 152022 |
