Conversation
|
This branch is working on a ticket in the NHS England VED JIRA Project. Here's a handy link to the ticket: VED-307 |
| "2000-02-30", # Invalid combination of month and day | ||
| ] | ||
|
|
||
| for_future_dates = [ |
There was a problem hiding this comment.
Not essential, but the future-proof way of doing this would be to generate date strings with strptime() based on offsets from the time now. That would mean we can have reliable test cases for dates which were only months or days in the future as well
There was a problem hiding this comment.
I'm sorry, only managed to get to review this now due to some internal calls.
This is absolutely mandatory. Even if we are deferring until the year 3000, you never write unit tests that are going to fail at some point and need bumping.
Typically, I would advocate mocking datetime.now(), but the validation tests are such spaghetti, that James' suggestion to use time offsets so we can guarantee it is always a future date would be sufficient.
|
| now = datetime.now(parsed_value.tzinfo) if parsed_value.tzinfo else datetime.now() | ||
| elif isinstance(parsed_value, date): | ||
| now = datetime.now().date() | ||
| if parsed_value > now: |
There was a problem hiding this comment.
This code will crash if parsed_value is not a datetime or date and now will not be assigned a value. I have checked and your code is always called in a safe way - i.e. it is only ever provided a datetime or date.
However, it might be worth stipulating this can only be called by functions which guarantee the input is date | datetime. Or handling the case where it isn't. Probably the first option is simplest and okay.
| now = datetime.now().date() | ||
| if parsed_value > now: | ||
| return True | ||
| return False No newline at end of file |
There was a problem hiding this comment.
Minor, but could use a newline.
* Datetime must not be in the future
4 participants
Summary
Add any other relevant notes or explanations here. Remove this line if you have nothing to add.
Reviews Required
Review Checklist
ℹ️ This section is to be filled in by the reviewer.