Add Time.new specs for strings ending where a colon is expected - #1393
Open
aminmansuri wants to merge 1 commit into
Open
Add Time.new specs for strings ending where a colon is expected#1393aminmansuri wants to merge 1 commit into
aminmansuri wants to merge 1 commit into
Conversation
Every other error-case string in core/time/new_spec.rb carries a trailing zone suffix, so no example ended at the byte where the parser expects a ':'. That is the one position where the message stops short of the offending byte, and it was unexercised. The gap turned up while investigating jruby/jruby#9623, where these three inputs read past the end of the string instead of raising ArgumentError.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
core/time/new_spec.rbcovers theTime.new(String)parse errors thoroughly, butevery error-case string in it ends with a trailing zone suffix —
+09:00,+0900or
Z. CRuby's ownTestTime#test_new_from_stringdoes the same. So no testexercises a string that ends exactly at the byte where the parser expects a
:.That byte matters. These messages are formatted with
%.*sand a length of one pastthe consumed region, so they normally include the byte that failed the check —
"missing sec part: 00:56 "keeps its trailing space,"fraction min is not supported: 00:56."keeps its dot. When the string ends at that position there is nosuch byte, the precision runs into the string's terminator, and the message stops
short:
"missing sec part: 00:56". That form was untested.Three examples for it:
"2020-12-25 00:56"missing sec part: 00:56"2020-12-25T00:56"missing sec part: 00:56"2020-12-25 00"missing min part: 00One byte shorter (
"2020-12-25 00:5") and the two-digit check fires first; one bytelonger (
"2020-12-25 00:56Z") and there is a byte to report. The boundary is exact.The expectations are anchored with
\A...\z, as several neighbouring examples in thefile already are, so they genuinely distinguish the truncated message from the form
that carries a trailing byte. They keep the usual
|can't parse:alternative forimplementations that raise the generic error.
CRuby raises exactly these messages today. JRuby 10 does not — it reads one byte past
the end of the string while building the message, and an
ArrayIndexOutOfBoundsExceptionescapes in place of theArgumentError. Reported asjruby/jruby#9623, fix in jruby/jruby#9624; until that ships, JRuby will need to tag
this example.
The omission was spotted by @sampokuokkanen while reviewing jruby/jruby#9624 — thanks
for noticing that everything in the existing set carries a zone suffix.