Skip to content

Commit b141425

Browse files
xwjiang-msxwjiang-ms
authored
[pre-commit] Fix style issues in test scripts under tests/acl folder (#6679)
* [pre-commit] Fix style issues in test scripts under `tests/acl` folder What is the motivation for this PR? pre-commit is a static analysis tool introduced recently. This tool is not able to do diff-only check. It checks the whole files touched by PR. We can't blame PR author for legacy issues. That's why currently the pre-commit check is only optional. To ensure that we can make pre-commit a mandatory check for PRs submitted to this repository, we need to fix all the legacy issues complained by pre-commit. How did you do it? This change fixes the style issues of test scripts directly under the `tests/acl` folder How did you verify/test it? Any platform specific information? Supported testbed topology if it's a new test case?
1 parent 2fd1ece commit b141425

File tree

5 files changed

+189
-133
lines changed

5 files changed

+189
-133
lines changed

‎tests/acl/conftest.py‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +0,0 @@
1-

‎tests/acl/files/acl_rules_del.json‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,3 @@
66
}
77
}
88
}
9-

‎tests/acl/null_route/test_null_route_helper.py‎

Lines changed: 43 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
from ptf.mask import Mask
1010
import ptf.packet as scapy
1111

12-
from tests.common.fixtures.ptfhost_utils import remove_ip_addresses # lgtm[py/unused-import]
12+
from tests.common.fixtures.ptfhost_utils import remove_ip_addresses # noqa F401
1313
import ptf.testutils as testutils
1414
from tests.common.plugins.loganalyzer.loganalyzer import LogAnalyzer, LogAnalyzerError
1515

@@ -40,24 +40,37 @@
4040

4141
TEST_DATA = [
4242
# src_ip, action, expected_result
43-
("1.2.3.4", "", FORWARD), # Should be forwared in default
44-
("fc03:1001::1", "", FORWARD), # Should be forwared in default
45-
46-
("1.2.3.4", "block {} 1.2.3.4".format(ACL_TABLE_NAME_V4), DROP), # Verify block ipv4 without prefix len
47-
("1.2.3.4", "unblock {} 1.2.3.4/32".format(ACL_TABLE_NAME_V4), FORWARD), # Verify unblock ipv4 with prefix len
48-
("1.2.3.4", "block {} 1.2.3.4/32".format(ACL_TABLE_NAME_V4), DROP), # Verify block ipv4 with prefix len
49-
("1.2.3.4", "block {} 1.2.3.4/32".format(ACL_TABLE_NAME_V4), DROP), # Verify double-block dosen't cause issue
50-
("1.2.3.4", "unblock {} 1.2.3.4/32".format(ACL_TABLE_NAME_V4), FORWARD), # Verify unblock ipv4 with prefix len
51-
("1.2.3.4", "unblock {} 1.2.3.4/32".format(ACL_TABLE_NAME_V4), FORWARD), # Verify double-unblock doesn't cause issue
52-
53-
("fc03:1000::1", "block {} fc03:1000::1".format(ACL_TABLE_NAME_V6), DROP), # Verify block ipv6 without prefix len
54-
("fc03:1000::1", "unblock {} fc03:1000::1/128".format(ACL_TABLE_NAME_V6), FORWARD), # Verify unblock ipv6 with prefix len
55-
("fc03:1000::1", "block {} fc03:1000::1/128".format(ACL_TABLE_NAME_V6), DROP), # Verify block ipv6 with prefix len
56-
("fc03:1000::1", "block {} fc03:1000::1/128".format(ACL_TABLE_NAME_V6), DROP), # Verify double-block dosen't cause issue
57-
("fc03:1000::1", "unblock {} fc03:1000::1/128".format(ACL_TABLE_NAME_V6), FORWARD), # Verify unblock ipv4 with prefix len
58-
("fc03:1000::1", "unblock {} fc03:1000::1/128".format(ACL_TABLE_NAME_V6), FORWARD), # Verify double-unblock doesn't cause issue
43+
("1.2.3.4", "", FORWARD), # Should be forwared in default
44+
("fc03:1001::1", "", FORWARD), # Should be forwared in default
45+
46+
("1.2.3.4", "block {} 1.2.3.4"
47+
.format(ACL_TABLE_NAME_V4), DROP), # Verify block ipv4 without prefix len
48+
("1.2.3.4", "unblock {} 1.2.3.4/32"
49+
.format(ACL_TABLE_NAME_V4), FORWARD), # Verify unblock ipv4 with prefix len
50+
("1.2.3.4", "block {} 1.2.3.4/32"
51+
.format(ACL_TABLE_NAME_V4), DROP), # Verify block ipv4 with prefix len
52+
("1.2.3.4", "block {} 1.2.3.4/32"
53+
.format(ACL_TABLE_NAME_V4), DROP), # Verify double-block dosen't cause issue
54+
("1.2.3.4", "unblock {} 1.2.3.4/32"
55+
.format(ACL_TABLE_NAME_V4), FORWARD), # Verify unblock ipv4 with prefix len
56+
("1.2.3.4", "unblock {} 1.2.3.4/32"
57+
.format(ACL_TABLE_NAME_V4), FORWARD), # Verify double-unblock doesn't cause issue
58+
59+
("fc03:1000::1", "block {} fc03:1000::1"
60+
.format(ACL_TABLE_NAME_V6), DROP), # Verify block ipv6 without prefix len
61+
("fc03:1000::1", "unblock {} fc03:1000::1/128".
62+
format(ACL_TABLE_NAME_V6), FORWARD), # Verify unblock ipv6 with prefix len
63+
("fc03:1000::1", "block {} fc03:1000::1/128"
64+
.format(ACL_TABLE_NAME_V6), DROP), # Verify block ipv6 with prefix len
65+
("fc03:1000::1", "block {} fc03:1000::1/128"
66+
.format(ACL_TABLE_NAME_V6), DROP), # Verify double-block dosen't cause issue
67+
("fc03:1000::1", "unblock {} fc03:1000::1/128"
68+
.format(ACL_TABLE_NAME_V6), FORWARD), # Verify unblock ipv4 with prefix len
69+
("fc03:1000::1", "unblock {} fc03:1000::1/128"
70+
.format(ACL_TABLE_NAME_V6), FORWARD), # Verify double-unblock doesn't cause issue
5971
]
6072

73+
6174
@pytest.fixture(scope="module", autouse=True)
6275
def remove_dataacl_table(rand_selected_dut):
6376
"""
@@ -82,8 +95,8 @@ def remove_dataacl_table(rand_selected_dut):
8295
output = rand_selected_dut.shell("sonic-cfggen -j {} --var-json \"ACL_TABLE\"".format(config_db_json))['stdout']
8396
try:
8497
entry = json.loads(output)[TABLE_NAME]
85-
cmd_create_table = "config acl add table {} {} -p {} -s {}".format(TABLE_NAME, entry['type'], \
86-
",".join(entry['ports']), entry['stage'])
98+
cmd_create_table = "config acl add table {} {} -p {} -s {}"\
99+
.format(TABLE_NAME, entry['type'], ",".join(entry['ports']), entry['stage'])
87100
logger.info("Restoring ACL table {}".format(TABLE_NAME))
88101
rand_selected_dut.shell(cmd_create_table)
89102
except Exception as e:
@@ -94,7 +107,7 @@ def remove_acl_table(duthost):
94107
"""
95108
A helper function to remove ACL table for testing
96109
"""
97-
cmds= [
110+
cmds = [
98111
"config acl remove table {}".format(ACL_TABLE_NAME_V4),
99112
"config acl remove table {}".format(ACL_TABLE_NAME_V6)
100113
]
@@ -144,8 +157,10 @@ def apply_pre_defined_rules(rand_selected_dut, create_acl_table):
144157
time.sleep(5)
145158
yield
146159
# Clear ACL rules
147-
rand_selected_dut.shell('sonic-db-cli CONFIG_DB keys "ACL_RULE|{}*" | xargs sonic-db-cli CONFIG_DB del'.format(ACL_TABLE_NAME_V4))
148-
rand_selected_dut.shell('sonic-db-cli CONFIG_DB keys "ACL_RULE|{}*" | xargs sonic-db-cli CONFIG_DB del'.format(ACL_TABLE_NAME_V6))
160+
rand_selected_dut.shell('sonic-db-cli CONFIG_DB keys "ACL_RULE|{}*" | xargs sonic-db-cli CONFIG_DB del'
161+
.format(ACL_TABLE_NAME_V4))
162+
rand_selected_dut.shell('sonic-db-cli CONFIG_DB keys "ACL_RULE|{}*" | xargs sonic-db-cli CONFIG_DB del'
163+
.format(ACL_TABLE_NAME_V6))
149164

150165

151166
@pytest.fixture(scope="module")
@@ -157,7 +172,7 @@ def setup_ptf(rand_selected_dut, ptfhost, tbinfo):
157172
vlan_name = ""
158173
mg_facts = rand_selected_dut.get_extended_minigraph_facts(tbinfo)
159174
for vlan_info in mg_facts["minigraph_vlan_interfaces"]:
160-
ip_ver = ipaddress.ip_network(vlan_info['addr'], False).version
175+
ip_ver = ipaddress.ip_network(vlan_info['addr'], False).version
161176
dst_ports[ip_ver] = str(ipaddress.ip_address(vlan_info['addr']) + 1) + '/' + str(vlan_info['prefixlen'])
162177
vlan_name = vlan_info['attachto']
163178

@@ -177,7 +192,7 @@ def generate_packet(src_ip, dst_ip, dst_mac):
177192
"""
178193
Build ipv4 and ipv6 packets/expected_packets for testing.
179194
"""
180-
if ipaddress.ip_network(unicode(src_ip), False).version == 4:
195+
if ipaddress.ip_network(str(src_ip), False).version == 4:
181196
pkt = testutils.simple_ip_packet(eth_dst=dst_mac, ip_src=src_ip, ip_dst=dst_ip)
182197
exp_pkt = Mask(pkt)
183198
exp_pkt.set_do_not_care_scapy(scapy.Ether, "dst")
@@ -209,7 +224,8 @@ def send_and_verify_packet(ptfadapter, pkt, exp_pkt, tx_port, rx_port, expected_
209224
def test_null_route_helper(rand_selected_dut, tbinfo, ptfadapter, apply_pre_defined_rules, setup_ptf):
210225
"""
211226
Test case to verify script null_route_helper.
212-
Some packets are generated as defined in TEST_DATA and sent to DUT, and verify if packet is forwarded or dropped as expected.
227+
Some packets are generated as defined in TEST_DATA and sent to DUT,
228+
and verify if packet is forwarded or dropped as expected.
213229
"""
214230
ptf_port_info = setup_ptf
215231
rx_port = ptf_port_info['port']
@@ -228,9 +244,9 @@ def test_null_route_helper(rand_selected_dut, tbinfo, ptfadapter, apply_pre_defi
228244
src_ip = test_item[0]
229245
action = test_item[1]
230246
expected_result = test_item[2]
231-
ip_ver = ipaddress.ip_network(unicode(src_ip), False).version
247+
ip_ver = ipaddress.ip_network(str(src_ip), False).version
232248
logger.info("Testing with src_ip = {} action = {} expected_result = {}"
233-
.format(src_ip, action, expected_result))
249+
.format(src_ip, action, expected_result))
234250
pkt, exp_pkt = generate_packet(src_ip, DST_IP[ip_ver], router_mac)
235251
if action != "":
236252
rand_selected_dut.shell(NULL_ROUTE_HELPER + " " + action)

0 commit comments

Comments
 (0)