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

TimeZone support #16

Closed
nrinaudo opened this issue Oct 27, 2016 · 4 comments
Closed

TimeZone support #16

nrinaudo opened this issue Oct 27, 2016 · 4 comments
Milestone

Comments

@nrinaudo
Copy link

I often have use cases where I need to test dates within a specific time zone. It's probably fairly simple to adapt scalacheck-datetime's Gen instances to my need - just map into it and set the desired timezone - but then I need to make sure this still falls within the expected date boundaries.

Would it be possible to, say, add a new time zone parameter to genDateTimeWithinRange? Might be implicit (probably not a great idea), might have a default value of UTC if you don't want to break backwards compatibility...

@noelmarkham noelmarkham added this to the v1.0 milestone Jan 18, 2017
@sullivan-
Copy link
Contributor

I needed UTC myself, and wrote a little wrapper like so:

import com.fortysevendeg.scalacheck.datetime.joda.GenJoda.genDateTime                                     
import org.joda.time.DateTimeZone.UTC                                                                     
import org.scalacheck.Arbitrary                                                                           
                                                                                                                   
implicit val arbJodaZ = Arbitrary(genDateTime.map(_.withZone(UTC)))

I was thinking of contributing it, probably make it into a def with a DateTimeZone parameter with default value of DateTimeZone.getDefault(), which would match current behavior. E.g. maybe replace existing

  implicit val arbJoda: Arbitrary[DateTime] = Arbitrary(genDateTime)

with

  implicit def arbJoda(implicit zone: DateTimeZone = DateTimeZone.getDefault): Arbitrary[DateTime] = Arbitrary(genDateTime.map(_.withZone(zone)))

Now users can override the default zone by supplying their own implicit zone.

If you want I can write it up with a test and send a PR. Not sure how much binary compat matters to you; I'm fairly certain this would break it.

@sullivan-
Copy link
Contributor

Probably better as

  implicit def defaultTimeZone = DateTimeZone.getDefault
  implicit def arbJoda(implicit zone: DateTimeZone): Arbitrary[DateTime] =
    Arbitrary(genDateTime.map(_.withZone(zone)))

@sullivan-
Copy link
Contributor

here's a PR, please take it if you like it. sorry I didn't wait for your feedback before writing it.

@nrinaudo it seems to me you shouldn't have to make changes to GenDateTime.genDateTimeWithinRange. this def takes in a DateTime as argument, and it looks like the operations in JodaInstances.jodaForPeriod will respect the time zone of the original date time passed in.

#58

@juanpedromoreno
Copy link
Member

Fixed by #58

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants