LGTM.

Hm.. I'm noticing there's not a codecov hook anymore, but actually this seems to be introducing some uncovered lines see here. @jbrockmendel Do you think you can come up with some strings that will hit those other cases?

I haven't brainstormed any particularly good examples for the other cases. Got any in mind?

I couldn't think of anything so I tried fuzzing the parser a bit with python-afl, and in fact found a bug in those lines!

>>> from dateutil.parser import parse
>>> parse('02:17NOV2017CE')
...
AttributeError: '_ymd' has no attribute 'month'

Good catch. I'll have to take a look at that fuzzer.

Pushing a fix shortly.

Though the trailing "BCE" is a whole other can of worms.

It also works with a trailing CE, which would be a valid date in Python. I haven't found any examples yet where it would actually parse to a real date, though.

Yah, I'm running the fuzzer now to get a feel for how it works.

I vote for closing out this bug fix and not cargo-culting coverage.

@jbrockmendel I wouldn't worry too much about the fuzzing, but I think that my insistence on coverage was not at all cargo culting, since in this case the code you've written is fundamentally broken. Anything that reaches the uncovered branches will fail with AttributeError. I'm not sure if there's an obvious fix to it, since _ymd no longer actually stores year, month or day, so either the approach to could_be_day needs to be fixed or _ymd needs to take a different approach as a container.

Worst case scenario, we can add tests that directly call _ymd (and maybe that's a good start when doing the development), but I'd be more comfortable with that if we could come up with some reasoning to explain why no valid datetime strings would ever reach those branches in the current implementation.

since in this case the code you've written is fundamentally broken

Fair point. (Huh. The fix I pushed isn't showing up here... gotta see where that ended up)

[...] if we could come up with some reasoning to explain why no valid datetime strings would ever reach those branches in the current implementation.

I have no such reasoning, and I like the idea of using fuzzing to try to find them. All I'm getting at is that if we can't find such cases after [whatever the appropriate amount of effort is] we shouldn't let that stop us from fixing the bug we know about.

I like this approach with test_internals.py. Fuzzing with 5 parallel tasks for most of a day didn't find anything useful here, so we'll replace the relevant test_internals.py calls if we come up with anything that naturally hits these lines (I'm guessing that's more likely to happen due to refactoring than fuzzing or something).