Skip to content

Commit 74d9fa8

Browse files
tobixenclaude
andcommitted
Fix breakpoint() in production code, clean up stale TODO comments
- Replace bare breakpoint() in CheckMakeDeleteCalendar with a proper warning log and set create-calendar.set-displayname to False when the server returns a calendar for a display name that cannot exist - Remove duplicate stale "collector framework" TODO comment from checks.py and checks_base.py (feature-to-check mapping is now implemented in the CLI via _feature_to_check_name) - Remove stale "record what features are checked" TODO from checks_base.py run_check (already implemented via keys_before/after) - Mark TODO item as done; add note about orphaned caldav_server_tester_old.py Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 314aea4 commit 74d9fa8

4 files changed

Lines changed: 34 additions & 42 deletions

File tree

docs/TODO.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1-
* [x] Currently there is some differences in the logic dependent on if it can find a caldav test directory or not. If it can find it, then it seems to disregard the ordinary servers in the caldav configuration. I don't want this - the only thing I want is to have extra servers avaiable through the `--name` option if caldav test servers are found.
1+
* [x] Currently there is some differences in the logic dependent on if it can find a caldav test directory or not. If it can find it, then it seems to disregard the ordinary servers in the caldav configuration. I don't want this - the only thing I want is to have extra servers available through the `--name` option if caldav test servers are found.
22
* [x] The default action now is "run towards all servers". I think it should default to "don't run anything", with a `--all` flag that can be used if one wants this kind of behaviour. Change of mind: we skip `--all` and the default is to connect to the default calendar server.
33
* [x] Currently, by default it shows all "non-full features". It should rather show all features deviating from the default.
44
* [x] New TODO-items have been added to USAGE.md. They should be fixed and removed from this file prior to the v1.0-release
5-
* [] Search for other TODO-notes in the project
5+
* [x] Search for other TODO-notes in the project
6+
* [x] `src/caldav_server_tester/caldav_server_tester_old.py` is 1028 lines, not imported anywhere - consider deleting it
67
* [] It's needed to test it towards a server that does not allow calendars to be created
78
* [] The text-based description should be prettified

src/caldav_server_tester/checker.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ def report(self, verbose=False, show_diff=False, return_what=str):
188188
f"Server: {ret['name']} ({ret['url']})",
189189
f"caldav library version: {ret['caldav_version']}",
190190
"",
191-
"Feature compatibility (non-verbose: showing only deviations from expected):"
191+
"Feature compatibility (non-verbose: showing only deviations from the standard):"
192192
if not verbose
193193
else "Feature compatibility:",
194194
]

src/caldav_server_tester/checks.py

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,13 +44,6 @@ def d(obj):
4444
return (x for x in objects if date(2000, 1, 1) <= d(x) <= date(2001, 1, 1))
4545

4646

47-
## WORK IN PROGRESS
48-
49-
## TODO: We need some collector framework that can collect all checks,
50-
## build a dependency graph and mapping from a feature to the relevant
51-
## check.
52-
53-
5447
class CheckGetCurrentUserPrincipal(Check):
5548
"""
5649
Checks support for get-current-user-principal
@@ -112,7 +105,15 @@ def _try_make_calendar(self, cal_id, **kwargs):
112105
try:
113106
name = "A calendar with this name should not exist"
114107
self.checker.principal.calendar(name=name).events()
115-
breakpoint() ## TODO - do something better here
108+
## Server returned a calendar for a name that cannot exist;
109+
## display-name lookup is unreliable on this server.
110+
import logging
111+
112+
logging.warning(
113+
"Server returned a calendar for a display name that should not exist; "
114+
"cannot verify create-calendar.set-displayname"
115+
)
116+
self.set_feature("create-calendar.set-displayname", False)
116117
except:
117118
## This is not the exception, this is the normal
118119
try:

src/caldav_server_tester/checks_base.py

Lines changed: 21 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,6 @@
11
import copy
22
import logging
33

4-
## WORK IN PROGRESS
5-
6-
## TODO: We need some collector framework that can collect all checks,
7-
## build a dependency graph and mapping from a feature to the relevant
8-
## check.
9-
104

115
class Check:
126
"""
@@ -36,20 +30,23 @@ def set_feature(self, feature, value=True):
3630
return
3731

3832
feat_def = self.checker._features_checked.find_feature(feature)
39-
feat_type = feat_def.get('type', 'server-feature')
33+
feat_type = feat_def.get("type", "server-feature")
4034

41-
if feat_type not in ('server-peculiarity', 'server-feature'):
35+
if feat_type not in ("server-peculiarity", "server-feature"):
4236
## client-behaviour, tests-behaviour or client-feature
4337
## cannot be checked for reliably (and is not supposed to
4438
## be checked by the script). server-observation is unreliable.
45-
assert(feat_type in ('server-observation',))
39+
assert feat_type in ("server-observation",)
4640
return
4741

4842
value_str = fs.is_supported(feature, str)
4943

5044
## Fragile support is ... fragile and should be ignored
5145
## same with unknown
52-
if value_str in ('fragile', 'unknown') or self.expected_features.is_supported(feature, str) in ('fragile', 'unknown'):
46+
if value_str in ("fragile", "unknown") or self.expected_features.is_supported(feature, str) in (
47+
"fragile",
48+
"unknown",
49+
):
5350
return
5451

5552
expected_ = self.expected_features.is_supported(feature, dict)
@@ -59,21 +56,23 @@ def set_feature(self, feature, value=True):
5956

6057
## Strip all free-text information from both observed and expected
6158
for stripdict in observed, expected:
62-
for y in ("behaviour", "description"):
63-
if y in stripdict:
64-
stripdict.pop(y)
59+
for y in ("behaviour", "description"):
60+
if y in stripdict:
61+
stripdict.pop(y)
6562

66-
if self.checker.debug_mode == 'assert':
67-
assert(observed == expected)
63+
if self.checker.debug_mode == "assert":
64+
assert observed == expected
6865
return
6966

7067
if observed != expected:
71-
if self.checker.debug_mode == 'logging':
72-
logging.error(f"Server checker found something unexpected for {feature}. Expected: {expected_}, observed: {observed_}")
73-
elif self.checker.debug_mode == 'pdb':
68+
if self.checker.debug_mode == "logging":
69+
logging.error(
70+
f"Server checker found something unexpected for {feature}. Expected: {expected_}, observed: {observed_}"
71+
)
72+
elif self.checker.debug_mode == "pdb":
7473
breakpoint()
7574
else:
76-
assert(False)
75+
assert False
7776

7877
def feature_checked(self, feature, return_type=bool):
7978
return self.checker._features_checked.is_supported(feature, return_type)
@@ -85,12 +84,7 @@ def run_check(self, only_once=True):
8584
for foo in self.depends_on:
8685
foo(self.checker).run_check(only_once=only_once)
8786

88-
## TODO: record what new features are checked. Everything in
89-
## self.features_checked should be covered, everyhing not in
90-
## self.features_checked should not be checked.
91-
keys_before = set(
92-
self.checker._features_checked.dotted_feature_set_list().keys()
93-
)
87+
keys_before = set(self.checker._features_checked.dotted_feature_set_list().keys())
9488

9589
## expected_features is the preconfigured feature set for this server.
9690
self.expected_features = self.checker._client_obj.features
@@ -104,9 +98,7 @@ def run_check(self, only_once=True):
10498
self.checker._client_obj.features = self.expected_features
10599

106100
## Check that all the declared checking has been done
107-
keys_after = set(
108-
self.checker._features_checked.dotted_feature_set_list().keys()
109-
)
101+
keys_after = set(self.checker._features_checked.dotted_feature_set_list().keys())
110102
new_keys = keys_after - keys_before
111103
missing_keys = self.features_to_be_checked - new_keys
112104
parent_keys = ()
@@ -135,6 +127,4 @@ def run_check(self, only_once=True):
135127
self.checker._checks_run.add(self.__class__)
136128

137129
def _run_check(self):
138-
raise NotImplementedError(
139-
f"A subclass {self.__class__} hasn't implemented the _run_check method"
140-
)
130+
raise NotImplementedError(f"A subclass {self.__class__} hasn't implemented the _run_check method")

0 commit comments

Comments
 (0)