481 accept date and time as string in ISO 8601 format for ChronoLocalDateTime 2 #552

rkiselev · 2020-07-26T18:33:54Z

Hi @robstoll, thanks for your answers. I think that I have implemented as you said, but I get an error on build:
ChronoLocalDateTimeAssertionSpec.kt: (18, 9): Type inference failed: inline fun <T, A1> fun1(f: KFunction2<Expect, A1, Expect>): Fun1<T, A1> /* = Pair<String, Expect.(A1) -> Expect> /
cannot be applied to
(KFunction2<Expect<ChronoLocalDateTime<>>, ChronoLocalDateTime<>, Expect<ChronoLocalDateTime<>>>)

and others for each method, could you help me?

I confirm that I have read the Contributor Agreements v1.0, agree to be bound on them and confirm that my contribution is compliant.

# Conflicts: # misc/deprecated/domain/robstoll-lib/atrium-domain-robstoll-lib-jvm/src/main/kotlin/ch/tutteli/atrium/domain/robstoll/lib/creating/chronoLocalDateTimeAssertions.kt

…/impl/DefaultChronoLocalDateTimeAssertions.kt

…ll#481-chrono-local-date-time # Conflicts: # apis/fluent-en_GB/atrium-api-fluent-en_GB-jvm/src/test/kotlin/ch/tutteli/atrium/api/fluent/en_GB/ChronoLocalDateTimeAssertionSpec.kt # logic/atrium-logic-jvm/src/main/kotlin/ch/tutteli/atrium/logic/impl/DefaultChronoLocalDateTimeAssertions.kt

robstoll · 2020-07-26T19:40:16Z

@rkiselev can you please base your work on my branch. It already contains all changes you did in the other PR (see instructions in the other PR on how to checkout the branch). This way I see immediately what you have changed since then. thanks.

rkiselev · 2020-07-26T19:55:10Z

@robstoll, I can't change the source branch of PR, but after you have changed PR from master to #481-ChronoLocalDateTime-as-string at file changed tab there are only my last changes from today. Is it enough, or do I need to close this PR and create a new one?

robstoll · 2020-07-26T20:30:02Z

I would expect to see only one commit but it's OK if the changes I see now are the new ones. I'll look at your problem tomorrow

robstoll

@rkiselev the problem is Kotlin, the compiler is not able to infer the types as we have now two functions with the name isBefore in the API. We need to help it.
change

fun1(Expect<ChronoLocalDateTime<*>>::isBefore),

to

fun1<ChronoLocalDateTime<*>, ChronoLocalDateTime<*>>(Expect<ChronoLocalDateTime<*>>::isBefore),

Same same for the other functions. Intellij will tell you that the type arguments are redundant but you need to keep them as the compiler sees it differently (it will work as expected with Kotlin 1.4)

robstoll · 2020-07-27T08:47:04Z

...n_GB-jvm/src/main/kotlin/ch/tutteli/atrium/api/fluent/en_GB/chronoLocalDateTimeAssertions.kt

+
+/**
+ * Expects that the subject of the assertion (a [ChronoLocalDateTime])
+ * is before the [expected] [String].


Please also add to the other KDoc

Suggested change

* is before the [expected] [String].

* is before the [expected] [LocalDateTime] given as [String] in (modified) ISO 8601 format.

*

* The string will be converted to a LocalDateTime according to ISO 8601 but with a slight deviation. The alternative notation (e.g. 20200401120001 instead of 2020-04-01T12:00:01) is not supported and we accept a date without time in which case 00:00:00 is used. Following the supported formats from the longest to the shortest:

* * yyyy-mm-ddThh:mm:ss.sss (up to 9 digits for nanoseconds)

* * yyyy-mm-ddThh:mm:ss

* * yyyy-mm-ddThh:mm

* * yyyy-mm-dd

rkiselev · 2020-07-28T15:21:38Z

thanks, it works.

…stoll#481-chrono-local-date-time # Conflicts: # apis/fluent-en_GB/atrium-api-fluent-en_GB-jvm/src/main/kotlin/ch/tutteli/atrium/api/fluent/en_GB/chronoLocalDateTimeAssertions.kt # apis/fluent-en_GB/atrium-api-fluent-en_GB-jvm/src/test/kotlin/ch/tutteli/atrium/api/fluent/en_GB/ChronoLocalDateTimeAssertionSpec.kt

rkiselev · 2020-08-08T18:44:11Z

hi, @robstoll, could you please check my changes, I have finished implementation.

robstoll

Thanksfor the changes. Following a few suggestions for improvement.

robstoll · 2020-08-08T19:10:56Z

...B-jvm/src/test/kotlin/ch/tutteli/atrium/api/fluent/en_GB/ChronoLocalDateTimeAssertionSpec.kt

+        "2020-01-01T01:01:1" to "wrong string format",
+        "2020-01-01t01:01:01" to "wrong string format"
+    ).forEach{(value, description) ->
+        run {


I would remove this (not necessary)

Suggested change

run {

robstoll · 2020-08-08T19:15:03Z

...B-jvm/src/test/kotlin/ch/tutteli/atrium/api/fluent/en_GB/ChronoLocalDateTimeAssertionSpec.kt

+    ).forEach{(value, description) ->
+        run {
+            val now = expect(LocalDateTime.now())
+            describe(description) {


Since the description is always the same, I would move it outside of the forEach and turn the mapOf into a listOf. Instead I would use the value here.

robstoll · 2020-08-08T19:17:26Z

...B-jvm/src/test/kotlin/ch/tutteli/atrium/api/fluent/en_GB/ChronoLocalDateTimeAssertionSpec.kt

+        run {
+            val now = expect(LocalDateTime.now())
+            describe(description) {
+                it("throws an DateTimeParseException") {


Please add the name of the function (also ain the other places)

Suggested change

it("throws an DateTimeParseException") {

it("isBefore -- throws an DateTimeParseException") {

robstoll · 2020-08-08T19:20:54Z

...B-jvm/src/test/kotlin/ch/tutteli/atrium/api/fluent/en_GB/ChronoLocalDateTimeAssertionSpec.kt

+                    }
+                }
+                it("doesn't throw any Error") {
+                    expect {


remove expect{...} as this creates an Expect of a lambda which is never executed. Alternatively you could use notToThrow to verify it (instead of toThrow) but we haven't use this so far and I prefer the shorter version

…ime' into robstoll#481-chrono-local-date-time

robstoll · 2020-08-19T19:12:16Z

@rkiselev thanks for the update. I am going to adjust a few more things and add the infix API and then merge into master. Thanks for the endurance and your work 🙂 👍
Fancy to tackle the other related issues? I guess they should be easy now.

rkiselev and others added 12 commits July 5, 2020 10:40

robstoll#481 added String parameter instead of a ChronoLocalDateTime

e78f254

robstoll#481 moved implementation after review.

dc044f5

Merge branch 'master' into robstoll#481-chrono-local-date-time

902194e

# Conflicts: # misc/deprecated/domain/robstoll-lib/atrium-domain-robstoll-lib-jvm/src/main/kotlin/ch/tutteli/atrium/domain/robstoll/lib/creating/chronoLocalDateTimeAssertions.kt

robstoll#481 fixed error on MR

c43e808

Update logic/atrium-logic-jvm/src/main/kotlin/ch/tutteli/atrium/logic…

b0fd9df

…/impl/DefaultChronoLocalDateTimeAssertions.kt

Update logic/atrium-logic-jvm/src/main/kotlin/ch/tutteli/atrium/logic…

5571d9a

…/impl/DefaultChronoLocalDateTimeAssertions.kt

Update logic/atrium-logic-jvm/src/main/kotlin/ch/tutteli/atrium/logic…

2db85bd

…/impl/DefaultChronoLocalDateTimeAssertions.kt

Update logic/atrium-logic-jvm/src/main/kotlin/ch/tutteli/atrium/logic…

b1b7580

…/impl/DefaultChronoLocalDateTimeAssertions.kt

Update logic/atrium-logic-jvm/src/main/kotlin/ch/tutteli/atrium/logic…

5e646ee

…/impl/DefaultChronoLocalDateTimeAssertions.kt

robstoll#481 added Companion implementation

0a743da

robstoll#481 added Companion implementation

19e5e32

rkiselev requested a review from robstoll as a code owner July 26, 2020 18:33

robstoll changed the base branch from master to #481-ChronoLocalDateTime-as-string July 26, 2020 19:37

robstoll#481 added Companion implementation

054f718

robstoll requested changes Jul 27, 2020

View reviewed changes

rkiselev added 3 commits August 8, 2020 16:09

robstoll#481 fixed types problem, extended KDoc

ef88590

robstoll#481 tests

71bf82a

rkiselev changed the title ~~481 accept date and time as string in ISO 8601 format for ChronoLocalDateTime 2 [WIP]~~ 481 accept date and time as string in ISO 8601 format for ChronoLocalDateTime 2 Aug 8, 2020

robstoll requested changes Aug 8, 2020

View reviewed changes

Roman Kiselev and others added 3 commits August 19, 2020 17:54

robstoll#481 removed run, replaced map by list, clarified comments.

f9821be

481 removed run, replaced map by list, clarified comments.

19ba491

Merge remote-tracking branch 'origin/robstoll#481-chrono-local-date-t…

2506df6

…ime' into robstoll#481-chrono-local-date-time

robstoll merged commit ae4a36f into robstoll:#481-ChronoLocalDateTime-as-string Aug 19, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

481 accept date and time as string in ISO 8601 format for ChronoLocalDateTime 2 #552

481 accept date and time as string in ISO 8601 format for ChronoLocalDateTime 2 #552

rkiselev commented Jul 26, 2020

robstoll commented Jul 26, 2020

rkiselev commented Jul 26, 2020 •

edited

Loading

robstoll commented Jul 26, 2020

robstoll left a comment

robstoll Jul 27, 2020

rkiselev commented Jul 28, 2020

rkiselev commented Aug 8, 2020

robstoll left a comment

robstoll Aug 8, 2020

robstoll Aug 8, 2020

robstoll Aug 8, 2020

robstoll Aug 8, 2020

robstoll commented Aug 19, 2020

- * is before the [expected] [String].
+ * is before the [expected] [LocalDateTime] given as [String] in (modified) ISO 8601 format.
+ *
+ * The string will be converted to a LocalDateTime according to ISO 8601 but with a slight deviation. The alternative notation (e.g. 20200401120001 instead of 2020-04-01T12:00:01) is not supported and we accept a date without time in which case 00:00:00 is used. Following the supported formats from the longest to the shortest:
+* * yyyy-mm-ddThh:mm:ss.sss (up to 9 digits for nanoseconds)
+* * yyyy-mm-ddThh:mm:ss
+* * yyyy-mm-ddThh:mm
+* * yyyy-mm-dd

	it("throws an DateTimeParseException") {
	it("isBefore -- throws an DateTimeParseException") {

481 accept date and time as string in ISO 8601 format for ChronoLocalDateTime 2 #552

481 accept date and time as string in ISO 8601 format for ChronoLocalDateTime 2 #552

Conversation

rkiselev commented Jul 26, 2020

robstoll commented Jul 26, 2020

rkiselev commented Jul 26, 2020 • edited Loading

robstoll commented Jul 26, 2020

robstoll left a comment

Choose a reason for hiding this comment

robstoll Jul 27, 2020

Choose a reason for hiding this comment

rkiselev commented Jul 28, 2020

rkiselev commented Aug 8, 2020

robstoll left a comment

Choose a reason for hiding this comment

robstoll Aug 8, 2020

Choose a reason for hiding this comment

robstoll Aug 8, 2020

Choose a reason for hiding this comment

robstoll Aug 8, 2020

Choose a reason for hiding this comment

robstoll Aug 8, 2020

Choose a reason for hiding this comment

robstoll commented Aug 19, 2020

rkiselev commented Jul 26, 2020 •

edited

Loading