Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test vectors #37

Merged
merged 1 commit into from
Jan 21, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 20 additions & 0 deletions tests/data/base_file.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
network,is_valid,is_subnet_of_extra
127.0.0.0/17,FALSE,FALSE
0.128.0.0/10,TRUE,FALSE
206.0.0.0/8,TRUE,FALSE
105.231.36.0/23,TRUE,FALSE
108.232.0.0/13,TRUE,FALSE
110.204.32.0/21,TRUE,FALSE
113.53.128.0/17,TRUE,FALSE
121.128.0.0/9,TRUE,FALSE
122.64.0.0/11,TRUE,FALSE
131.128.0.0/9,TRUE,FALSE
138.232.0.0/13,TRUE,FALSE
140.210.168.0/22,TRUE,FALSE
145.224.0.0/13,TRUE,FALSE
152.128.0.0/9,TRUE,FALSE
162.55.64.0/18,TRUE,FALSE
165.128.0.0/13,TRUE,FALSE
174.160.0.0/11,TRUE,FALSE
109.92.213.0/24,TRUE,FALSE
244.20.0.0/14,TRUE,FALSE
21 changes: 21 additions & 0 deletions tests/data/extra_file.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
network,is_valid,is_subnet_of_base
2.11.76.0/22,TRUE,FALSE
115.7.192.0/19,TRUE,FALSE
12.132.80.0/20,TRUE,FALSE
82.64.0.0/11,TRUE,FALSE
194.48.0.0/17,TRUE,FALSE
197.216.136.0/21,TRUE,FALSE
249.0.0.0/9,TRUE,FALSE
32.17.64.0/20,TRUE,FALSE
38.66.0.0/17,TRUE,FALSE
40.240.124.0/22,TRUE,FALSE
44.26.0.0/19,TRUE,FALSE
46.99.64.0/20,TRUE,FALSE
55.48.0.0/15,TRUE,FALSE
74.0.0.0/10,TRUE,FALSE
77.120.136.0/22,TRUE,FALSE
131.128.58.0/23,TRUE,TRUE
244.20.240.0/20,TRUE,TRUE
108.232.0.0/16,TRUE,TRUE
122.68.0.0/14,TRUE,TRUE
121.128.0.0/10,TRUE,TRUE
44 changes: 43 additions & 1 deletion tests/merge_test.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
"""
Test merging multiple sets of networks, as if they were independent AS files.
"""
from kartograf.merge import general_merge

from .generate_data import (
from .util.generate_data import (
build_file_lines,
generate_ip_file,
generate_file_items,
Expand All @@ -13,6 +16,45 @@
def __tmp_paths(tmp_path):
return [tmp_path / p for p in ["rpki_final.txt", "irr_final.txt", "out.txt"]]

def __read_test_vectors(filepath):
"""
Fixtures for IP networks are under tests/data.
Read them and return the list of valid networks and the count of individual subnets expected in the result of the merge.
"""
networks = []
subnet_count = 0
with open(filepath, "r") as f:
lines = f.readlines()[1:]
for line in lines:
network, is_valid, is_subnet = line.split(',')
if is_valid == "TRUE":
networks.append(network)
if is_subnet.strip() == "TRUE":
subnet_count += 1
return networks, subnet_count

def test_merge_from_fixtures(tmp_path):
'''
Assert that general_merge merges subnets correctly.
'''
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

formatting-nit: A bit inconsistent to use ''' here and """ above


base_nets, base_subnet_count = __read_test_vectors("tests/data/base_file.csv")
base_path = tmp_path / "base.txt"
extra_nets, extra_subnet_count = __read_test_vectors("tests/data/extra_file.csv")
extra_path = tmp_path / "extra.txt"
# write the networks to disk, generating ASNs for each network
generate_ip_file(base_path, build_file_lines(base_nets, generate_asns(len(base_nets))))
generate_ip_file(extra_path, build_file_lines(extra_nets, generate_asns(len(extra_nets))))

outpath = tmp_path / "final_result.txt"
general_merge(base_path, extra_path, None, outpath)
with open(outpath, "r") as f:
l = f.readlines()
final_network_count = len(l)
expected_count = (len(base_nets) + len(extra_nets)) - (base_subnet_count + extra_subnet_count)
assert final_network_count == expected_count
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's fine to check the count but it leaves the possibility of uncaught bugs open if the count is offset to the correct number again. Imagine we add two networks, one should go into final and the other not (simple one valid case + one invalid case). If we don't check here which one goes in the behavior could be the opposite of what we expect (invalid in, valid out) and the test would still pass.

So as a follow-up I think it would be great if this is switched from checking the count to checking the actual content matches. That shouldn't be too bigh of a change, instead of counting in read_test_vectors, build a list of included ones and then compare the list to a readout of the final_result. I hope it works as simple as I imagine it :)



def test_merge(tmp_path):
"""
Assert that merging two identical files is a no-op.
Expand Down
File renamed without changes.
File renamed without changes.
Loading