Skip to content
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

Raise error on negative usec param to Time.new. #5371

Merged
merged 2 commits into from Oct 15, 2018

Conversation

Projects
None yet
2 participants
@headius
Copy link
Member

commented Oct 15, 2018

Fixes #5370.

I'll add specs momentarily.

@headius headius added this to the JRuby 9.2.1.0 milestone Oct 15, 2018

@til
Copy link

left a comment

Thanks for the quick fix.

Maybe also add a spec to protect against a regression of this specific bug? (No idea how likely that is though).

  # [Bug #5370]
  it "raises ArgumentError if the seconds argument is negative fraction of second" do
    lambda { Time.new(2000, 1, 1, 0, 0, -0.1) }.should raise_error(ArgumentError)
  end
lambda { Time.new(2000, 1, 1, 0, 0, -1) }.should raise_error(ArgumentError)
end

it "raises ArgumentError if the utf_offset argument is greater than or equal to 10e9" do

This comment has been minimized.

Copy link
@til

til Oct 15, 2018

*utc_offset

@headius

This comment has been minimized.

Copy link
Member Author

commented Oct 15, 2018

@til I think the -1 spec is just as good as the -0.1 spec, since it's just a simple test for negative seconds. The fact that it's a fractional amount does not appear to change the logic.

@headius headius force-pushed the headius:negative_usec_error branch from aa8dd44 to 2375ff7 Oct 15, 2018

@headius

This comment has been minimized.

Copy link
Member Author

commented Oct 15, 2018

Force pushed with corrected utf/utc line

@headius headius merged commit 117e9be into jruby:master Oct 15, 2018

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

@headius headius deleted the headius:negative_usec_error branch Oct 15, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.