typelevel / scalaz-contrib

Interoperability libraries & additional data structures and instances for Scalaz
http://typelevel.org
MIT License
54 stars 16 forks source link

Broken nscala-time instances #21

Closed larsrh closed 10 years ago

larsrh commented 10 years ago

In 35b3436e661355f462e2e062c378d62cdcb1c9a7, I updated to 2.11 and bumped the nscala-time version to 1.0.0. However, the compound types (like DateTime) violate the laws.

/cc @folone @adelbertc

folone commented 10 years ago

Looks like we're hitting jodatime's custom overflow detection here. We can avoid overflows by tuning the arbitraty instances for those types to have smaller range:

diff --git a/nscala-time/test/scala/NScalaTimeArbitrary.scala b/nscala-time/test/scala/NScalaTimeArbitrary.scala
index 197977b..da1dbf8 100644
--- a/nscala-time/test/scala/NScalaTimeArbitrary.scala
+++ b/nscala-time/test/scala/NScalaTimeArbitrary.scala
@@ -25,16 +25,16 @@ object NScalaTimeArbitrary {
     smallIntArb map { Period.millis(_) }

   implicit val DateTimeArbitrary: Arbitrary[DateTime] =
-    arb[Long] map { new DateTime(_) }
+    arb[Int] map { new DateTime(_) }

   implicit val LocalDateArbitrary: Arbitrary[LocalDate] =
-    arb[Long] map { new LocalDate(_) }
+    arb[Int] map { new LocalDate(_) }

   implicit val LocalTimeArbitrary: Arbitrary[LocalTime] =
-    arb[Long] map { new LocalTime(_) }
+    arb[Int] map { new LocalTime(_) }

   implicit val LocalDateTimeArbitrary: Arbitrary[LocalDateTime] =
-    arb[Long] map { new LocalDateTime(_) }
+    arb[Int] map { new LocalDateTime(_) }

   implicit val DaysArbitrary: Arbitrary[Days] =
     smallIntArb map Days.days
@@ -61,10 +61,10 @@ object NScalaTimeArbitrary {
     arb[Long] map { new Instant(_) }

   implicit val YearMonthArbitrary: Arbitrary[YearMonth] =
-    arb[Long] map { new YearMonth(_) }
+    arb[Int] map { new YearMonth(_) }

   implicit val MonthDayArbitrary: Arbitrary[MonthDay] =
-    arb[Long] map { new MonthDay(_) }
+    arb[Int] map { new MonthDay(_) }

   implicit val IntervalArbitrary: Arbitrary[Interval] =
     Apply[Arbitrary].apply2(arb[Int], arb[Int])((a, b) =>
diff --git a/nscala-time/test/scala/NScalaTimeTest.scala b/nscala-time/test/scala/NScalaTimeTest.scala
index 75d4ca2..511c208 100644
--- a/nscala-time/test/scala/NScalaTimeTest.scala
+++ b/nscala-time/test/scala/NScalaTimeTest.scala
@@ -11,7 +11,7 @@ class NScalaTimeTest extends Spec {

   import NScalaTimeArbitrary._

-  //checkAll("DateTime", enum.laws[DateTime])
+  checkAll("DateTime", enum.laws[DateTime])

   checkAll("Days", monoid.laws[Days])
   checkAll("Days", order.laws[Days])
@@ -26,16 +26,16 @@ class NScalaTimeTest extends Spec {

   checkAll("Interval", equal.laws[Interval])

-  //checkAll("LocalDate", enum.laws[LocalDate])
+  checkAll("LocalDate", enum.laws[LocalDate])

-  //checkAll("LocalDateTime", enum.laws[LocalDateTime])
+  checkAll("LocalDateTime", enum.laws[LocalDateTime])

-  //checkAll("LocalTime", order.laws[LocalTime])
+  checkAll("LocalTime", order.laws[LocalTime])

   checkAll("Minutes", monoid.laws[Minutes])
   checkAll("Minutes", order.laws[Minutes])

-  //checkAll("MonthDay", order.laws[MonthDay])
+  checkAll("MonthDay", order.laws[MonthDay])

   checkAll("Months", monoid.laws[Months])
   checkAll("Months", order.laws[Months])
@@ -49,7 +49,7 @@ class NScalaTimeTest extends Spec {
   checkAll("Weeks", monoid.laws[Weeks])
   checkAll("Weeks", order.laws[Weeks])

-  //checkAll("YearMonth", order.laws[YearMonth])
+  checkAll("YearMonth", order.laws[YearMonth])

   checkAll("Years", monoid.laws[Years])
   checkAll("Years", order.laws[Years])

Not sure how this worked before though (the checks are there since 2005, as far as I can see). Also not sure if this is a satisfactory solution to the problem.

larsrh commented 10 years ago

Thank you, @folone!

folone commented 10 years ago

Glad to help!