Conversation
|
@johnny423 Where do you suggest placing a test for this function? |
johnny423
left a comment
johnny423
left a comment
There was a problem hiding this comment.
Looks great, please run scripts/format-imports.sh before pushing again (I should add setup for pre-commit but Im lazy 😓 )
| return dispatcher | ||
|
|
||
|
|
||
| async def _attempt_timestamp(event_builder, timestamps: list[int], should_save: bool): |
There was a problem hiding this comment.
I can't really understand what the function is doing with the attempt_timestamp. can we rename it?
| nip_22_config = nips_config.nip_22 | ||
| if not nip_22_config: | ||
| return | ||
| current_time = time.time() |
There was a problem hiding this comment.
the fun part about not working with a method that uses the actual time it to test it specifically without having the whole dispatcher
| def test_timestamp_validation(self): | ||
| good_timestamps_params = [ | ||
| (1500, 2000, (-600, 0)), | ||
| (1700, 2000, (-600, -200)), | ||
| (190000, 100000, (80000, 100000)), | ||
| (250000, 100000, (-600, 500000)), | ||
| (100000, 100000, (0, 1)), | ||
| (101, 100, (-600, 500000)), | ||
| (8000, 100000, None), | ||
| ] | ||
| bad_timestamps_params = [ | ||
| (1300, 2000, (-600, 0)), | ||
| (1900, 2000, (-600, -200)), | ||
| (150000, 100000, (80000, 100000)), | ||
| (9999999, 100000, (-600, 500000)), | ||
| (100001, 100000, (0, 1)), | ||
| (151, 100, (-600, 50)), | ||
| ] | ||
| for param_list, expected in [ | ||
| (good_timestamps_params, True), | ||
| (bad_timestamps_params, False) | ||
| ]: | ||
| for param_instance in param_list: | ||
| result = is_creation_time_valid(*param_instance) | ||
| assert result == expected |
There was a problem hiding this comment.
first - you probably want to use parameterize here :)
second - its a bit hard to read like that, anyway to improve readability?
|
|
||
| @pytest.mark.asyncio | ||
| async def test_legal_timestamp(self, event_builder): | ||
| nip_22_config = nips_config.nip_22 |
There was a problem hiding this comment.
does this test work based on configuration?
| await repo.delete(keys_to_delete) | ||
|
|
||
|
|
||
| def is_creation_time_valid(event_creation_time: int, current_time: float, nip_22_config: tuple[int, int]) -> bool: |
There was a problem hiding this comment.
can you add a docstring?
| if nips_config.nip_9 and event.kind == EventKind.EventDeletion: # type: ignore | ||
| await _handle_delete_event(repo, event) | ||
|
|
||
| if not is_creation_time_valid(event.created_at, time.time(), nips_config.nip_22): |
There was a problem hiding this comment.
the validation should happen before the delete
Elialka
force-pushed
the
feature/support_nip_22
branch
from
6736792 to
473c19b
Compare
2 participants
No description provided.