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

Filter away timestamps from diffs, increase testing. #199

Merged
merged 2 commits into from
Jan 15, 2024
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
36 changes: 31 additions & 5 deletions ci/diff.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,39 @@
import difflib
import json
import re
import sys
from typing import Any, Dict, List


def group_objects(json_file_path):
def replace_timestamps(obj: Any) -> Any:
Copy link
Member

@pederhan pederhan Jan 15, 2024

Choose a reason for hiding this comment

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

Only consider implementing this if you think replace_timestamps() could actually be called with typed values in the future (currently only Any is passed to it).


Suggested change
def replace_timestamps(obj: Any) -> Any:
from typing import TypeVar
TSType = TypeVar("TSType", str, list, dict)
def replace_timestamps(obj: TSType) -> TSType:

This is not perfect, but provides some very rudimentary type checking:

reveal_type(replace_timestamps("2020-01-01T00:00:00.000Z"))
reveal_type(replace_timestamps(["2020-01-01T00:00:00.000Z", "2020-01-01T00:00:00.000Z"]))
reveal_type(replace_timestamps({"foo": "2020-01-01T00:00:00.000Z"}))
% mypy ci
ci/diff.py:27: note: Revealed type is "builtins.str"
ci/diff.py:28: note: Revealed type is "builtins.list[Any]"
ci/diff.py:29: note: Revealed type is "builtins.dict[Any, Any]"

With a more complicated TypeVar we could get rid of the Any from the returned list and dict type annotations, but that's overkill for something that isn't even part of the CLI itself.

"""Recursively replace timestamp values in a JSON object.

:param obj: A JSON object (dict, list, or primitive type).
:returns: A new object with timestamps replaced.
"""
timestamp_pattern = re.compile(
r"\d{4}-\d{2}-\d{2}[ T]\d{2}:\d{2}:\d{2}(?:\.\d+)?(?:[+-]\d{2}:\d{2})?|\d{4}-\d{2}-\d{2}"
)
if isinstance(obj, dict):
return {k: replace_timestamps(v) for k, v in obj.items()}
elif isinstance(obj, list):
return [replace_timestamps(elem) for elem in obj]
elif isinstance(obj, str):
return timestamp_pattern.sub("<TIME>", obj)
return obj


def group_objects(json_file_path: str) -> List[List[Dict[str, Any]]]:
"""Group objects in a JSON file by a specific criterion.

:param json_file_path: Path to the JSON file.
:returns: A list of grouped objects.
"""
with open(json_file_path, "r") as f:
data = json.load(f)

data = [replace_timestamps(obj) for obj in data]

grouped_objects = []
temp = []

Expand All @@ -23,7 +50,8 @@ def group_objects(json_file_path):
return grouped_objects


def main():
def main() -> None:
"""Compare two JSON files."""
if len(sys.argv) != 3:
print("Usage: diff.py <file1> <file2>")
sys.exit(1)
Expand All @@ -40,9 +68,7 @@ def main():
cmdlist2.append(a[0]["command"].rstrip())
differ = difflib.Differ()
diff = differ.compare(cmdlist1, cmdlist2)
differences = [
line for line in diff if line.startswith("-") or line.startswith("+")
]
differences = [line for line in diff if line.startswith("-") or line.startswith("+")]
if differences:
print(
"Diff between what commands were run in the recorded result and the current testsuite:"
Expand Down
14 changes: 10 additions & 4 deletions ci/testsuite
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ network list_used_addresses 2001:db8::/64
network list_unused_addresses 2001:db8::/64
network set_reserved 2001:db8::/64 50
host add tinyhost -ip 2001:db8::/64 -contact [email protected]
host info tinyhost | 2001
host info tinyhost |! example.org
host remove tinyhost
network remove 2001:db8::/64 -f

Expand All @@ -56,6 +58,7 @@ network set_description 10.0.1.0/24 "Frozen but has one host"
network set_vlan 10.0.1.0/24 1234
network info 10.0.1.0/24
network find -network 10.0.1.0/24 -description '*one host*' -vlan 1234 -frozen 1 -reserved 6 -dns_delegated 0 -category Yellow -location Somewhere
host history somehost
host remove somehost
network unset_frozen 10.0.1.0/24
host add otherhost -ip 10.0.1.20 -contact [email protected] # fails because reserved range
Expand Down Expand Up @@ -95,6 +98,7 @@ group info mygroup
group group_remove mygroup yourgroup
group owner_add mygroup anotherowner
group owner_remove mygroup myself
group history mygroup
group delete mygroup # fails because the group contains testhost1, must force
group delete mygroup -force
group delete yourgroup
Expand Down Expand Up @@ -130,9 +134,10 @@ policy list_roles *
policy list_roles fru
policy add_atom fruit apple
policy add_atom fruit orange
policy info orange |! Created # Created dates will differ
policy info fruit |! Created # Created dates will differ
policy list_members fruit |! Created # Created dates will differ
policy info orange
policy info fruit
policy list_members fruit
policy atom_history apple
policy atom_delete apple # Should fail because 'apple' is used in role 'fruit'
policy remove_atom fruit apple
policy atom_delete apple
Expand All @@ -147,6 +152,7 @@ policy host_remove fruit foo
policy remove_atom fruit banana # should fail
policy remove_atom fruit tangerine
policy role_delete vegetables # fails, that role doesn't exist
policy role_history fruit
policy role_delete fruit
policy atom_delete tangerine
host remove foo
Expand Down Expand Up @@ -285,7 +291,7 @@ label add postit 'A label again'
policy role_create myrole 'This is the description'
policy label_add postit myrole
policy list_roles *
policy info myrole |! Created # Created dates will differ
policy info myrole
label info postit
policy label_remove postit myrole
policy role_delete myrole
Expand Down
Loading