Skip to content

Commit 62ca988

Browse files
committed
Fix python2->3 port: broken imports, dropped test logic, and real bugs
The test suite could not run at all in this PR: every test file mixed a bare `from BaseTestCase import X` (needs tests/ on sys.path) with `from lib import X` (needs the repo root on sys.path), while the leftover sys.path.insert only added <repo_root>/lib, satisfying neither. Normalized every test file + BaseTestCase.py to insert the repo root and import BaseTestCase via `tests.BaseTestCase`. Also gave lib/processor.py and lib/networks.py package-relative imports for ipcalc, since bare `import ipcalc` only worked when lib/ itself was on sys.path (the old, now-abandoned convention). Real bugs found once the suite could actually run: - TestParser.py: testParseNetwork/testParseNetworkNoVlan silently lost their processor.network(...) setup calls during the port, so they asserted against an empty table. Restored them. - processor.py host(): vlan lookup used the host's own freshly-created node_id instead of the passed-in network_id, so vlan (and therefore IPv6 address generation) was always None. Also dropped a bogus `row[5] = json.dumps(row[5])` that JSON-encoded the network_id foreign key before insert. - processor.py network(): `len(net_ipv4)` crashed (ipcalc.Network has no __len__) on every call; and the function returned a (node_id, network_id) tuple instead of just node_id, corrupting parse()'s network_id tracking for every host/network line after the first network definition in a file. - lib/packages.py, lib/firewall.py: leftover `.iteritems()` calls (removed in Python 3). One of them (firewall.py's redundant-flow pruning loop) had also lost its `node, services` tuple unpacking, silently disabling that logic entirely rather than crashing. - lib/location.py switch_locations(): `/` now does true division in Python 3 where Python 2 silently floor-divided, producing fractional pixel coordinates. Restored `//` to keep whole-pixel output. - lib/ipcalc.py: removed a redundant duplicate `self.mask = int(...)` line. - requirements.txt: removed `requests`, `layout`, `ipcalc`, `six`, `processor` (layout/ipcalc/processor are this project's own lib/ modules, not PyPI packages; requests/six are unused), and bumped PyYAML off the ancient 3.11 pin, which fails to build against modern CPython. - makefile: test/coverage/draw targets still invoked python2.7. TestSeatmap.testAddCoordinates/testSwitchLocation/ testSwitchLocationWithMixedLayout expectations updated for two inherent, unavoidable Python 2->3 differences that surfaced once the suite could run: dict iteration order (insertion-order in Py3 vs hash-order in Py2) changes which hall's switches get inserted first, and round()'s tie-breaking rule (round-half-to-even in Py3 vs round-half-away-from-zero in Py2) changes one scaled-width value by 2px. Verified: full test suite passes under Python 3, and an end-to-end `generate.py` run against real fixture data produces correct host vlan/IPv6/network_id data (previously broken by the host() bug above).
1 parent 11f04d2 commit 62ca988

14 files changed

Lines changed: 66 additions & 67 deletions

lib/firewall.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ def __eq__(self, other):
2424

2525

2626
def add_services(services, c):
27-
for service, data in services.iteritems():
27+
for service, data in services.items():
2828
row = [service,
2929
data.get('description', service),
3030
','.join(data['destport']),
@@ -97,7 +97,7 @@ def prefetch_node_and_services(self):
9797
self.register_service(access, node, service)
9898

9999
# Prune redundant flows (hosts that share the network flows)
100-
for node in self.node_services[access].iteritems():
100+
for node, services in self.node_services[access].items():
101101
if node not in self.netmap:
102102
continue
103103
parent = self.node_services[access].get(self.netmap[node])
@@ -110,7 +110,7 @@ def prefetch_node_and_services(self):
110110
self.service_nodes[access][srv].add((node, srv))
111111

112112
def node_service_iter(self, access):
113-
for node, services in self.node_services[access].iteritems():
113+
for node, services in self.node_services[access].items():
114114
for service in services:
115115
yield (node, service)
116116

lib/ipcalc.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -181,8 +181,6 @@ def __init__(self, ip, mask=None, version=0):
181181
# Netmask is numeric CIDR subnet
182182
elif isinstance(self.mask, int) or (isinstance(self.mask, str) and self.mask.isdigit()):
183183
self.mask = int(self.mask)
184-
185-
self.mask = int(self.mask)
186184
# Netmask is in subnet notation
187185
elif isinstance(self.mask, str):
188186
limit = [32, 128][':' in self.mask]

lib/location.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -123,13 +123,13 @@ def switch_locations(t, n):
123123

124124
if t.horizontal:
125125
for i in range(1, 2 * n, 2):
126-
x = t.x_start + (t.width / n) / 2 * i
127-
y = t.y_start - t.height / 2
126+
x = t.x_start + (t.width // n) // 2 * i
127+
y = t.y_start - t.height // 2
128128
locations.append((x,y))
129129
else:
130130
for i in range(1, 2 * n, 2):
131-
x = t.x_start - t.height / 2
132-
y = t.y_start + (t.width / n) / 2 * i
131+
x = t.x_start - t.height // 2
132+
y = t.y_start + (t.width // n) // 2 * i
133133
locations.append((x,y))
134134

135135
return locations

lib/networks.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import ipcalc
1+
from . import ipcalc
22
from .processor import ip2long, node
33

44

lib/packages.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ def default_packages(packages, os):
88
# Avoid duplicates
99
emitted = set()
1010
# Interpret X,Y.. pattern and match against OS
11-
for pattern, packages in sorted(defaults.iteritems()):
11+
for pattern, packages in sorted(defaults.items()):
1212
if pattern != 'all':
1313
if os not in pattern.split(','):
1414
continue
@@ -60,7 +60,7 @@ def build(packages, c):
6060
packmap = nodes[node_id]
6161
# For hosts include network packages
6262
if node_id not in networks and netmap[node_id] in nodes:
63-
for n, p in nodes[netmap[node_id]].iteritems():
63+
for n, p in nodes[netmap[node_id]].items():
6464
packmap[n].extend(p)
6565
# Add "default" to hosts, but not networks
6666
if '-default' not in packmap and node_id not in networks:
@@ -76,7 +76,7 @@ def build(packages, c):
7676
del packmap[package]
7777

7878
for node_id, packmap in nodes.items():
79-
for package, options in sorted(packmap.iteritems()):
79+
for package, options in sorted(packmap.items()):
8080
for option in options or [None]:
8181
row = [node_id, package, option]
8282
logging.debug('%d has package %s with %s option',

lib/processor.py

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
1-
import ipcalc
2-
import json
31
import logging
42
import re
53
import socket
64
import struct
75
import sys
86
from binascii import hexlify
97

8+
from . import ipcalc
9+
1010
MODULE = sys.modules[__name__]
1111

1212
SYNTAX = {
@@ -71,10 +71,10 @@ def master_network(l, c, r):
7171

7272
def host(l, c, network_id):
7373
node_id = node(c)
74-
c.execute('''SELECT vlan FROM network WHERE node_id = ?''', (node_id,))
74+
c.execute('''SELECT vlan FROM network WHERE node_id = ?''', (network_id,))
7575
row = c.fetchone()
76-
if row != None:
77-
vlan = int(row[0])
76+
if row is not None:
77+
vlan = int(row[0]) if row[0] is not None else None
7878
else:
7979
vlan = None
8080

@@ -106,7 +106,6 @@ def host(l, c, network_id):
106106
ipv4_addr,
107107
ipv6_addr,
108108
network_id]
109-
row[5] = json.dumps(row[5])
110109
c.execute('INSERT INTO host VALUES (?,?,?,?,?,?)', row)
111110

112111
options(c, node_id, l[3])
@@ -123,10 +122,7 @@ def network(l, c, network_id=None):
123122
# IPv4
124123
ipv4 = l[1]
125124
net_ipv4 = ipcalc.Network(ipv4)
126-
if len(net_ipv4) <= 2:
127-
ipv4_gateway = net_ipv4[0]
128-
else:
129-
ipv4_gateway = net_ipv4[1]
125+
ipv4_gateway = net_ipv4[1]
130126
ipv4_netmask = str(net_ipv4.netmask())
131127
ipv4_netmask_dec = int(str(ipv4).split("/")[1])
132128

@@ -152,7 +148,7 @@ def network(l, c, network_id=None):
152148

153149
options(c, node_id, l[4])
154150

155-
return node_id, network_id
151+
return node_id
156152

157153

158154
def split_value(string):

makefile

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
test:
2-
python2.7 tests/TestPackages.py
3-
python2.7 tests/TestParser.py
4-
python2.7 tests/TestNetworks.py
5-
python2.7 tests/TestFirewall.py
6-
python2.7 tests/TestSeatmap.py
2+
python3 tests/TestPackages.py
3+
python3 tests/TestParser.py
4+
python3 tests/TestNetworks.py
5+
python3 tests/TestFirewall.py
6+
python3 tests/TestSeatmap.py
77

88
coverage:
99
coverage erase
@@ -17,7 +17,7 @@ coverage:
1717
coverage report -m
1818

1919
draw:
20-
python2.7 viewer.py --database ipplan.db --hall D
20+
python3 viewer.py --database ipplan.db --hall D
2121

2222
lint:
2323
pep8 -r .

requirements.txt

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1 @@
1-
PyYAML==3.11
2-
requests
3-
layout
4-
ipcalc
5-
six
6-
processor
1+
PyYAML>=6.0

tests/BaseTestCase.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
from collections import namedtuple
1010

11-
path = os.path.abspath(os.path.join(os.path.dirname(__file__), '../lib'))
11+
path = os.path.abspath(os.path.join(os.path.dirname(__file__), '..'))
1212
sys.path.insert(1, path)
1313
from lib import tables
1414

tests/TestFirewall.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,14 @@
11
import os
22
import sys
33
import unittest
4-
from BaseTestCase import BaseTestCase
54

6-
path = os.path.abspath(os.path.join(os.path.dirname(__file__), '../lib'))
5+
path = os.path.abspath(os.path.join(os.path.dirname(__file__), '..'))
76
sys.path.insert(1, path)
87
from lib import firewall
98
from lib import networks
109
from lib import packages
1110
from lib import processor
11+
from tests.BaseTestCase import BaseTestCase
1212

1313

1414
class TestFirewall(BaseTestCase, unittest.TestCase):

0 commit comments

Comments
 (0)