| Message ID | 1459578262-6388-1-git-send-email-janusz.dziedzic@tieto.com |
|---|---|
| State | Changes Requested |
| Headers | show |
On Sat, 2016-04-02 at 08:24 +0200, Janusz Dziedzic wrote: > - subprocess.check_call(['iw', 'dev', dev[0].ifname, 'set', > 'type', 'adhoc']) > - subprocess.check_call(['iw', 'dev', dev[0].ifname, 'ibss', > 'join', > - 'ibss-test', '2412', 'HT20', 'fixed- > freq', > - '02:22:33:44:55:66']) > + dev[0].host.execute("iw dev " + dev[0].ifname + " set type > adhoc") > I suppose I should've seen that in some other patch in the previous series, but can we perhaps find a way to not have to do string manipulation here? host.execute() should probably take a list just like subprocess.Popen() does, and execute it as such, rather than passing it through a shell? And even if that's absolutely not possible (using ssh?) then we should probably do string quoting in a central place in the code. johannes
On 2 April 2016 at 20:32, Johannes Berg <johannes@sipsolutions.net> wrote: > On Sat, 2016-04-02 at 08:24 +0200, Janusz Dziedzic wrote: > >> - subprocess.check_call(['iw', 'dev', dev[0].ifname, 'set', >> 'type', 'adhoc']) >> - subprocess.check_call(['iw', 'dev', dev[0].ifname, 'ibss', >> 'join', >> - 'ibss-test', '2412', 'HT20', 'fixed- >> freq', >> - '02:22:33:44:55:66']) >> + dev[0].host.execute("iw dev " + dev[0].ifname + " set type >> adhoc") >> > I suppose I should've seen that in some other patch in the previous > series, but can we perhaps find a way to not have to do string > manipulation here? host.execute() should probably take a list just like > subprocess.Popen() does, and execute it as such, rather than passing it > through a shell? And even if that's absolutely not possible (using > ssh?) then we should probably do string quoting in a central place in > the code. > I decide to not fix all problems in one patchset - to much changes. First I added hwsim_wrapper() for remote test execution with current hwsim test cases. After that I can fix test cases one by one. Regarding host.execute(), this using subprocess.check_output( ..., shell=False) Anyway I think this is possible to get list and build string in Host() class for ssh case. while for local_execute() and execute(host=None) I split command back to the list. BR Janusz > johannes
diff --git a/tests/hwsim/test_ibss.py b/tests/hwsim/test_ibss.py index cc6bfc1..f5e060f 100644 --- a/tests/hwsim/test_ibss.py +++ b/tests/hwsim/test_ibss.py @@ -8,7 +8,6 @@ import logging logger = logging.getLogger() import time import re -import subprocess import hwsim_utils from utils import alloc_fail @@ -299,10 +298,8 @@ def test_ibss_open_fixed_bssid(dev): def test_ibss_open_retry(dev): """IBSS open (no security) with cfg80211 retry workaround""" - subprocess.check_call(['iw', 'dev', dev[0].ifname, 'set', 'type', 'adhoc']) - subprocess.check_call(['iw', 'dev', dev[0].ifname, 'ibss', 'join', - 'ibss-test', '2412', 'HT20', 'fixed-freq', - '02:22:33:44:55:66']) + dev[0].host.execute("iw dev " + dev[0].ifname + " set type adhoc") + dev[0].host.execute("iw dev " + dev[0].ifname + " ibss join ibss-test 2412 HT20 fixed-freq 02:22:33:44:55:66") ssid="ibss" try: dev[0].request("AP_SCAN 2") @@ -312,7 +309,7 @@ def test_ibss_open_retry(dev): dev[0].request("REASSOCIATE") bssid0 = wait_ibss_connection(dev[0]) - subprocess.check_call(['iw', 'dev', dev[0].ifname, 'ibss', 'leave']) + dev[0].host.execute("iw dev" + dev[0].ifname + " ibss leave") time.sleep(1) dev[0].request("DISCONNECT") finally: @@ -358,13 +355,14 @@ def test_ibss_5ghz(dev): try: _test_ibss_5ghz(dev) finally: - subprocess.call(['iw', 'reg', 'set', '00']) + dev[0].host.execute("iw reg set 00") + dev[1].host.execute("iw reg set 00") dev[0].flush_scan_cache() dev[1].flush_scan_cache() def _test_ibss_5ghz(dev): - subprocess.call(['iw', 'reg', 'set', 'US']) for i in range(2): + dev[i].host.execute("iw reg set US") for j in range(5): ev = dev[i].wait_event(["CTRL-EVENT-REGDOM-CHANGE"], timeout=5) if ev is None: @@ -395,15 +393,16 @@ def test_ibss_vht_80p80(dev): try: _test_ibss_vht_80p80(dev) finally: - subprocess.call(['iw', 'reg', 'set', '00']) + dev[0].host.execute("iw reg set 00") + dev[1].host.execute("iw reg set 00") dev[0].flush_scan_cache() dev[1].flush_scan_cache() def _test_ibss_vht_80p80(dev): - subprocess.call(['iw', 'reg', 'set', 'US']) for i in range(2): + dev[i].host.execute("iw reg set US") for j in range(5): - ev = dev[i].wait_event(["CTRL-EVENT-REGDOM-CHANGE"], timeout=5) + ev = dev[i].wait_event(["CTRL-EVENT-REGDOM-CHANGE"], timeout=10) if ev is None: raise Exception("No regdom change event") if "alpha2=US" in ev:
In case of remote hosts, we can't just use subprocesss() while we have to run command on remote host. Signed-off-by: Janusz Dziedzic <janusz.dziedzic@tieto.com> --- tests/hwsim/test_ibss.py | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-)