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
Add+collect tests for tzinfos input types, fix missed case of invalid… #891
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor changes here, and this also needs a changelog (in the end I'm probably going to do a rollup of this in the next release to merge them into one big "Added a ton of tests. Patches by @jbrockmendel (gh prs #x, #y, #z)" so they can be pretty terse.
I'm travelling the next couple days, so may not get around to this until Wednesday. In the interim, can we merge the other two? |
@jbrockmendel I merged #892, but I think #894 is still blocked on changes from you. If you want I can make the changes myself and merge that one. |
I think the requested change here is just adding |
@jbrockmendel Sorry about all the delays. I'll review again today or tomorrow. Assuming any changes I might want are sufficiently simple, I'll just push them directly to your branch and merge rather than bother you with another round of addressing review comments. |
@jbrockmendel I've rebased this and unified the "check if equal + check if def test_valid_tzinfo_int_input(self):
dstr = "2014 January 19 09:00 UTC"
tzinfos = {u"UTC": -28800}
expected = datetime(2014, 1, 19, 9, tzinfo=tz.UTC)
res = parse(dstr, tzinfos=tzinfos)
self.assert_equal_same_tz(res, expected) The current behavior is that I'll note that the fact that If you agree that we should keep the current behavior, I'll adjust the final test appropriately and merge. |
Yes, I agree that if the user explicitly specifies a non-standard UTC, that should take precedence. |
The `tzinfos` argument allows you to add or override the mappings between time zone strings and tzinfo objects, this PR groups the existing tests for this argument together in a class, and expands on it to a few more cases.
Currently, passing an argument to `tzinfos` that cannot be interpreted as a time zone will raise UnboundLocalError, but it should raise TypeError.
This switches the error from UnboundLocalError to TypeError.
For all of these tests, the most important part of the test is ensuring that the correct tzinfo object was attached, so it makes sense to add a convenience function that asserts that they are identical *and* have the same tzinfo object.
Co-authored-by: Brock Mendel <jbrockmendel@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I've tweaked and rewritten the commit history of this PR to my nitpicky satisfaction, ready for merge when CI passes.
Thanks @jbrockmendel for the patience. Now to continue working through the backlog on all my projects... |
Implement tests for types of inputs that can be passed as
tzinfos
(two of these are moved from further down in the file, the rest ported from downstream). Fixes the case where something bizarre is passed liketzinfos={"UTC": something_totally_invalid}
which at the moment raisesUnboundLocalError
instead ofTypeError