enhance interfacing with metadata - #55
Conversation
2a3c77b to
4bb5c82
Compare
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #55 +/- ##
=======================================
Coverage 53.69% 53.69%
=======================================
Files 12 12
Lines 1583 1583
Branches 253 253
=======================================
Hits 850 850
Misses 697 697
Partials 36 36 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| raise ValueError("Start time for session not found!") | ||
| raise ValueError("Start time for session not found! " | ||
| "Please add a `start_time` parameter to sessions for user {}, " | ||
| "e.g. '10 May 2020 12:34:56.789103'" |
There was a problem hiding this comment.
Maybe clarify that the time should be right after the key:
| "e.g. '10 May 2020 12:34:56.789103'" | |
| "e.g. start_time: 10 May 2020 12:34:56" |
(also removing the decimals so as not to imply you need all the accuracy - the date/time parsing is quite flexible, I think)
| expected_error_string = "Start time for session not found! " \ | ||
| "Please add a `start_time` parameter to sessions for user A, " \ | ||
| "e.g. '10 May 2020 12:34:56.789103'" | ||
| assert expected_error_string == str(exc_info.value) |
There was a problem hiding this comment.
As this gets longer, maybe it's better to check only the beginning e.g.
assert str(exc_info.value).startswith(...)or
assert "Start time for session not found!" in str(exc_info.value)or even add a match=... argument to pytest.raises.
We lose some detail with any of these, but that only matters if we care about the exact content of the error message. In this case, maybe we can also assert that it contains start_time, since that is the essence of the change.
addresses some review comments
3 participants
a PR to address the items in #52 not covered in #54.