New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 830833 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner: ----
Closed: Oct 8
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 3
Type: Task



Sign in to add a comment

Factor out retry methods in autotest

Project Member Reported by zmarcus@google.com, Apr 9 2018

Issue description

Autotest has a retry pattern in use across a handful of files. Move this to a generally accessible location for wifi code and convert existing "wait_for_*" methods to wrap around it.

Some such files are visible using:
grep --include \*.py -rnl "while time.time() - start_time < timeout_seconds"

Issue brought up in chromium review 941797
 
Cc: briannorris@chromium.org
Components: -Infra>Client>ChromeOS>Test OS>Systems>Network
Moving component to OS>Systems>Network since we plan to fix it in our team. 

Labels: OS-Chrome
https://cs.corp.google.com/chromeos_public/src/third_party/autotest/files/client/common_lib/utils.py?q=poll_for_cond+package:%5Echromeos_public$&l=2704

Easy pickings: looks like I accidentally reimplemented this in network_WiFi_Reset.
A rough search shows 40+ files needing this. Since this is a P3, we'll leave this here and come back to it as a noogler task or have someone clean it up when they are bored. 

$ git grep -l "while time.time()" # top-level autotests folder
client/common_lib/cros/avahi_utils.py
client/common_lib/cros/fake_device_server/fake_gcd_helper.py
client/common_lib/cros/network/iw_runner.py
client/common_lib/cros/tendo/buffet_dbus_helper.py
client/common_lib/cros/tendo/buffet_tester.py
client/common_lib/cros/tendo/peerd_config.py
client/cros/cellular/base_station_8960.py
client/cros/cellular/base_station_pxt.py
client/cros/chrooted_avahi.py
client/cros/cros_ui.py
client/cros/networking/wifi_proxy.py
client/cros/tendo/peerd_dbus_helper.py
client/site_tests/cellular_SuspendResume/cellular_SuspendResume.py
client/site_tests/graphics_Stress/graphics_Stress.py
client/site_tests/graphics_WebGLManyPlanetsDeep/graphics_WebGLManyPlanetsDeep.py
client/site_tests/network_DefaultProfileCreation/network_DefaultProfileCreation.py
client/site_tests/network_EthernetStressPlug/network_EthernetStressPlug.py
client/site_tests/network_RackWiFiConnect/network_RackWiFiConnect.py
client/site_tests/power_BacklightControl/power_BacklightControl.py
client/site_tests/power_LowMemorySuspend/power_LowMemorySuspend.py
client/tests/uptime/control
server/cros/ap_configurators/dynamic_ap_configurator.py
server/cros/chaos_lib/chaos_runner.py
server/cros/chaos_lib/static_runner.py
server/cros/network/connection_worker.py
server/cros/network/netperf_runner.py
server/cros/network/wifi_client.py
server/cros/network/wpa_cli_proxy.py
server/cros/servo/chrome_cr50.py
server/cros/update_engine/omaha_devserver.py
server/hosts/abstract_ssh.py
server/hosts/cros_host.py
server/hosts/ssh_multiplex.py
server/site_linux_router.py
server/site_tests/hardware_StorageStress/hardware_StorageStress.py
server/site_tests/network_DiskFull/network_DiskFull.py
server/site_tests/network_StressServoEthernetPlug/network_StressServoEthernetPlug.py
server/site_tests/network_WiFi_ChannelScanDwellTime/network_WiFi_ChannelScanDwellTime.py
server/site_tests/network_WiFi_ConnectionIdentifier/network_WiFi_ConnectionIdentifier.py
server/site_tests/network_WiFi_MalformedProbeResp/network_WiFi_MalformedProbeResp.py
server/site_tests/network_WiFi_PMKSACaching/network_WiFi_PMKSACaching.py
server/site_tests/network_WiFi_Reset/network_WiFi_Reset.py
server/site_tests/network_WiFi_RoamDbus/network_WiFi_RoamDbus.py

Project Member

Comment 4 by bugdroid1@chromium.org, May 14 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/fe67b65d5ad249c32b5823f5f02846abca978eff

commit fe67b65d5ad249c32b5823f5f02846abca978eff
Author: Zachary Marcus <zmarcus@google.com>
Date: Mon May 14 22:33:10 2018

autotest: convert "wait_for_*" to wrap existing util function

autotest client has a utils file with a poll_for_condition()
replace relevant implementations of the wait_for logic with
calls to that

Test anything touched by change that can be done in a standard wificell

BUG= chromium:830833 
TEST=test_that -b cave {DUT_HOSTNAME} suite:wifi_matfunc
     test_that -b kevin {DUT_HOSTNAME} network_WiFi_Reset
     python client/common_lib/cros/network/iw_runner_unittest.py
     test_that -b cave {DUT_HOSTNAME} network_WiFi_ScanPerformance \
             network_WiFi_ProfileGUID network_WiFi_MissingBeacons \
             network_WiFi_Prefer5Ghz network_WiFi_RoamFT.PSK \
             network_WiFi_RoamFT.EAP network_WiFi_ConnectOnResume \
             network_WiFi_RetryConnectHidden network_WiFi_VerifyRouter

Change-Id: If829c8d73e17b48c2e72a7765603477f2ce4ad5a
Reviewed-on: https://chromium-review.googlesource.com/1008636
Commit-Ready: Kirtika Ruchandani <kirtika@chromium.org>
Tested-by: Kirtika Ruchandani <kirtika@chromium.org>
Reviewed-by: Kirtika Ruchandani <kirtika@chromium.org>

[modify] https://crrev.com/fe67b65d5ad249c32b5823f5f02846abca978eff/server/cros/network/wifi_client.py
[modify] https://crrev.com/fe67b65d5ad249c32b5823f5f02846abca978eff/server/cros/network/wpa_cli_proxy.py
[modify] https://crrev.com/fe67b65d5ad249c32b5823f5f02846abca978eff/server/site_tests/network_WiFi_RoamDbus/network_WiFi_RoamDbus.py
[modify] https://crrev.com/fe67b65d5ad249c32b5823f5f02846abca978eff/server/site_tests/network_WiFi_Reset/network_WiFi_Reset.py
[modify] https://crrev.com/fe67b65d5ad249c32b5823f5f02846abca978eff/client/cros/networking/wifi_proxy.py
[modify] https://crrev.com/fe67b65d5ad249c32b5823f5f02846abca978eff/client/common_lib/utils.py
[modify] https://crrev.com/fe67b65d5ad249c32b5823f5f02846abca978eff/server/site_tests/network_WiFi_PMKSACaching/network_WiFi_PMKSACaching.py
[modify] https://crrev.com/fe67b65d5ad249c32b5823f5f02846abca978eff/client/common_lib/cros/network/iw_runner.py

Labels: wifi-test-failures
Project Member

Comment 6 by bugdroid1@chromium.org, May 29 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/67a2fdc36228057611563f7720424068cf724cec

commit 67a2fdc36228057611563f7720424068cf724cec
Author: Zachary Marcus <zmarcus@google.com>
Date: Tue May 29 02:37:23 2018

autotest: refactor: reuse poll_for in ssh hosts

Use utils.poll_for_condition when starting a multiplexed ssh connection
Removes the duplicated logic used when starting up wifi tests

BUG= chromium:830833 
TEST=test_that -b {DUT_BOARDNAME} {DUT_HOSTNAME} suite:wifi_matfunc

Change-Id: Ie03da21404e08891778d7ea365237c988f393e4c
Reviewed-on: https://chromium-review.googlesource.com/1070487
Reviewed-by: Kirtika Ruchandani <kirtika@chromium.org>
Tested-by: Kirtika Ruchandani <kirtika@chromium.org>
Commit-Queue: Kirtika Ruchandani <kirtika@chromium.org>
Trybot-Ready: Kirtika Ruchandani <kirtika@chromium.org>

[modify] https://crrev.com/67a2fdc36228057611563f7720424068cf724cec/server/hosts/ssh_multiplex.py

Cc: akhouderchah@google.com snanda@chromium.org
Owner: ----
Alex, 
Want to take a stab at one of these as your (pre)starter bug?
(assigned to you, though gerrit doesnt let us make you owner just yet).


Project Member

Comment 8 by bugdroid1@chromium.org, Jun 13 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/106bf4e7de092c9c1df560c5734b7ecbc8185904

commit 106bf4e7de092c9c1df560c5734b7ecbc8185904
Author: Alex Khouderchah <akhouderchah@google.com>
Date: Wed Jun 13 08:00:55 2018

network_WiFi_ConnectionIdentifier: Use poll_for_condition

Replaces manual polling logic with the poll_for_condition function
defined in the autotest_lib.client.common_lib.utils module.

BUG= chromium:830833 
TEST=`test_that -b chell $IP network_WiFi_ConnectionIdentifier` passes

Change-Id: I54a954f88243d3f428944577d648d778d957c420
Reviewed-on: https://chromium-review.googlesource.com/1097704
Commit-Ready: Alex Khouderchah <akhouderchah@google.com>
Tested-by: Alex Khouderchah <akhouderchah@google.com>
Reviewed-by: Kirtika Ruchandani <kirtika@chromium.org>

[modify] https://crrev.com/106bf4e7de092c9c1df560c5734b7ecbc8185904/server/site_tests/network_WiFi_ConnectionIdentifier/network_WiFi_ConnectionIdentifier.py

Project Member

Comment 9 by bugdroid1@chromium.org, Jun 13 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/4983d799c4152e7a7cd129631cf5bc2ccb8c84b4

commit 4983d799c4152e7a7cd129631cf5bc2ccb8c84b4
Author: Alex Khouderchah <akhouderchah@google.com>
Date: Wed Jun 13 16:19:38 2018

network_WiFi_ChannelScanDwellTime: Refactor to use poll_for_condition

Replaces manual polling logic with the poll_for_condition function
defined in the autotest_lib.client.common_lib.utils module.

BUG= chromium:830833 
TEST=`test_that -b chell $IP network_WiFi_ChannelScanDwellTime` passes

Change-Id: I915851dc74931af2467b4f30f3d83c0163fc886a
Reviewed-on: https://chromium-review.googlesource.com/1097984
Commit-Ready: Kirtika Ruchandani <kirtika@chromium.org>
Tested-by: Alex Khouderchah <akhouderchah@google.com>
Reviewed-by: Kirtika Ruchandani <kirtika@chromium.org>

[modify] https://crrev.com/4983d799c4152e7a7cd129631cf5bc2ccb8c84b4/server/site_tests/network_WiFi_ChannelScanDwellTime/network_WiFi_ChannelScanDwellTime.py

Project Member

Comment 10 by bugdroid1@chromium.org, Jun 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/autotest/+/b0c624c970cc6f23acbc02473b6d5bc46f92644a

commit b0c624c970cc6f23acbc02473b6d5bc46f92644a
Author: Alex Khouderchah <akhouderchah@google.com>
Date: Sat Jun 23 00:22:27 2018

site_linux_router: Use poll_for_condition

BUG= chromium:830833 
TEST=`test_that -b chell $IP network_WiFi_Prefer5Ghz` passes

Change-Id: I6a4809a37b9738086e36d94c6e3869b9ec754c61
Reviewed-on: https://chromium-review.googlesource.com/1093961
Commit-Ready: Kirtika Ruchandani <kirtika@chromium.org>
Tested-by: Alex Khouderchah <akhouderchah@google.com>
Tested-by: Kirtika Ruchandani <kirtika@chromium.org>
Reviewed-by: Kirtika Ruchandani <kirtika@chromium.org>

[modify] https://crrev.com/b0c624c970cc6f23acbc02473b6d5bc46f92644a/server/site_linux_router.py

Cc: npoojary@chromium.org
All the WiFi-related changes seem to have been already addressed. Is there anything I can tackle here?
Status: Fixed (was: Untriaged)
Seems like you're probably right. Let's close this.

Sign in to add a comment