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

accept date and time as string in ISO 8601 format for ChronoZonedDateTime #482

Closed
12 tasks
robstoll opened this issue Apr 28, 2020 · 10 comments · Fixed by #625
Closed
12 tasks

accept date and time as string in ISO 8601 format for ChronoZonedDateTime #482

robstoll opened this issue Apr 28, 2020 · 10 comments · Fixed by #625
Assignees
Labels
Milestone

Comments

@robstoll
Copy link
Owner

robstoll commented Apr 28, 2020

Platform (jvm, js, android): jvm
Extension (none, kotlin 1.3, jdk8): jdk8

Code related feature

expect(ZonedDateTime.now()).isAfter("2020-04-28T12:00Z")

//instead of

expect(ZonedDateTime.now()).isAfter(ZonedDateTime.of(2020,4,28,12,0,0,0, ZoneId.of("Z")))

Following the things you need to do:

atrium-logic:

  • duplicate each method in ChronoZonedDateTimeAssertions and:
    • accept a String instead of a ChronoZonedDateTime and parse it to ZonedDateTime and pass it to the existing overload where the following formats shall be supported. This is a slight deviation of ISO 8601, e.g. we do not want to support the alternative notation e.g. 20200401120001Z instead of 2020-04-01T12:00:01Z
      The format is composed of {DateTime}{ZoneDesignator}, first the spec for DateTime:

      • yyyy-mm-ddThh:mm:ss.SSS (up to 9 digits for nanoseconds)
      • yyyy-mm-ddThh:mm:ss
      • yyyy-mm-ddThh:mm
      • yyyy-mm-dd

      And for ZoneDesignator:

      • Z
      • Z+hh
      • Z+hh:mm
      • Z-hh
      • Z-hh:mm

      Following a few examples how it looks combined:

      • yyyy-mm-ddThh:mm:ssZ
      • yyyy-mm-ddThh:mmZ+01
      • yyyy-mm-ddZ-04:30

      Right now we don't intend to support things like Europe/Zurich as ZoneDesignators (would be supported by jdk). So do a check upfront if in correct format and throw a DateTimeParseException if wrong and only then use ZonedDateTime.parse
      Note that accept date and time as string in ISO 8601 format for ChronoLocalDateTime #481 deals also with parsing DateTime. You should reuse the logic from there.

api-fluent

  • duplicate each method in chronoZonedDateTimeAssertions.kt and:
    • accept a String instead of a ChronoZonedDateTime - call the corresponding logic function
    • change since to 0.14.0 (adopt to current milestone)
  • add the string version to ChronoZonedDateTimeAssertionsSpec:
    • add an object StringSpec, create functions in the companion object which corresponds to the signature of the existing functions (i.e. accept a ChronoZonedDateTime) where inside you turn the ChronoZonedDateTime into a string in ISO format and pass it to your new function
  • create ChronoZonedDateTimeAsStringAssertionSpec by copying ch.tutteli.atrium.specs.integration.ChronoLocalDateTimeAsStringAssertionSpec and adapt it, it should cover the following cases for a wrong format:
    • one passes a ZonedDateTime in format yyyymmddhhmmssSSSZ
    • one passes a ZonedDateTime in format yyyymmddhhmmssZ
    • one passes a ZonedDateTime in format yyyymmddhhmmZ
    • one passes a ZonedDateTime in format yyyymmddZ
    • one passes a ZonedDateTime in format yyyy-m-ddThh:mmZ
    • one passes a ZonedDateTime in format yyyy-mm-dThh:mmZ
    • one passes a ZonedDateTime in format yy-mm-ddThh:mmZ
    • one passes a ZonedDateTime in format yyyy-mm-ddTh:mmZ
    • one passes a ZonedDateTime in format yyyy-mm-ddThh:mZ
    • one passes a ZonedDateTime in format yyyy-mm-ddThh:mm:sZ
    • one passes a ZonedDateTime in format yyyy-mm-ddthh:mmZ (note lower case t)
    • one passes a ZonedDateTime in format yyyy-mm-ddThh:mmZ+h
    • one passes a ZonedDateTime in format yyyy-mm-ddThh:mmZ+h:mm
    • one passes a ZonedDateTime in format yyyy-mm-ddThh:mmZ+hh:m
    • one passes a ZonedDateTime in format yyyy-mm-ddThh:mmZ-h
    • one passes a ZonedDateTime in format yyyy-mm-ddThh:mmZ-h:mm
    • one passes a ZonedDateTime in format yyyy-mm-ddThh:mmZ-hh:m
  • add describe("allowed shortcuts") with at least the following test cases:
    • one passes a ZonedDateTime in format yyyy-mm-ddThh:mm:ssZ
    • one passes a ZonedDateTime in format yyyy-mm-ddThh:mmZ
    • one passes a ZonedDateTime in format yyyy-mm-ddZ
  • adopt ChronoZonedDateTimeAssertionsSpec (see ch.tutteli.atrium.api.fluent.en_GB.ChronoLocalDateTimeAssertionSpec as a guideline)
    • add an object ChronoZonedDateTimeAsStringSpec which extends the above copied spec
    • add the string version to the ambiguityTests

api-infix

  • same same but different

Your first contribution?

  • Write a comment I'll work on this if you would like to take this issue over.
    This way we get the chance to revise the description in case things have changed in the meantime, we might give you additional hints and we can assign the task to you, so that others do not start as well.
  • See Your first code contribution for guidelines.
  • Do not hesitate to ask questions here or to contact us via Atrium's slack channel if you need help
    (Invite yourself in case you do not have an account yet).

Hint
Have a look at the following PR for ChronoLocalDateTime:
https://github.com/robstoll/atrium/pull/556/files

assaflei pushed a commit to assaflei/atrium that referenced this issue May 15, 2020
introduce typealias CharSequenceOrNumberOrChar for Any

move CharSequenceOrNumberOrChar to domain-api and use in domain-impl

update setup instructions for the new infix API

include the new infix API to be published

preparation for accepting String instead of Zoned/LocalDate(Time) object

preparation for robstoll#480, robstoll#481 and robstoll#482

refer to current milestone in good_first_issue template

fix link for nullableContainer

return empty message in case callee is org.spekframework.ide

reduce code duplication also for jvm specs, add more cases to PathSpec

fix wrong replacements and workaround KT-38721 (add import next to star)

api-fluent work done

commiting requested changes

updated with comments

adding problematic classes which break the build

adding problemtic code to server to reproduce error

reproducing problematic build on CI

fix to build problem

addressing latest comment, build should pass

adjust the pseudo-keyword from exist to existing

moved infix jdk8 extension to jvm module

rebasing
assaflei pushed a commit to assaflei/atrium that referenced this issue May 15, 2020
introduce typealias CharSequenceOrNumberOrChar for Any

move CharSequenceOrNumberOrChar to domain-api and use in domain-impl

update setup instructions for the new infix API

include the new infix API to be published

preparation for accepting String instead of Zoned/LocalDate(Time) object

preparation for robstoll#480, robstoll#481 and robstoll#482

refer to current milestone in good_first_issue template

fix link for nullableContainer

return empty message in case callee is org.spekframework.ide

reduce code duplication also for jvm specs, add more cases to PathSpec

fix wrong replacements and workaround KT-38721 (add import next to star)

api-fluent work done

commiting requested changes

updated with comments

adding problematic classes which break the build

adding problemtic code to server to reproduce error

reproducing problematic build on CI

fix to build problem

addressing latest comment, build should pass

adjust the pseudo-keyword from exist to existing

moved infix jdk8 extension to jvm module

rebasing
robstoll added a commit that referenced this issue May 15, 2020
* build windows also against JDK 11

introduce typealias CharSequenceOrNumberOrChar for Any

move CharSequenceOrNumberOrChar to domain-api and use in domain-impl

update setup instructions for the new infix API

include the new infix API to be published

preparation for accepting String instead of Zoned/LocalDate(Time) object

preparation for #480, #481 and #482

refer to current milestone in good_first_issue template

fix link for nullableContainer

return empty message in case callee is org.spekframework.ide

reduce code duplication also for jvm specs, add more cases to PathSpec

fix wrong replacements and workaround KT-38721 (add import next to star)

api-fluent work done

commiting requested changes

updated with comments

adding problematic classes which break the build

adding problemtic code to server to reproduce error

reproducing problematic build on CI

fix to build problem

addressing latest comment, build should pass

adjust the pseudo-keyword from exist to existing

moved infix jdk8 extension to jvm module

rebasing

* added file deprecation suppress message + removed niok and kbox references

* resolving comments in re
@Valefant
Copy link
Collaborator

Valefant commented Sep 7, 2020

You can assign me this one :-)

@robstoll
Copy link
Owner Author

robstoll commented Sep 7, 2020

Sure 🙂👍
I have sent you a collaborator invitation. Once accepted you can assign task yourself

@Valefant
Copy link
Collaborator

Valefant commented Sep 7, 2020

Sure 🙂👍
I have sent you a collaborator invitation. Once accepted you can assign task yourself

I didn't get the mail, because I haven't updated my Profile Email in a long time. If you don't mind, can you send the invitation again? Thank you 🙂

@robstoll
Copy link
Owner Author

robstoll commented Sep 8, 2020

@Valefant I have re-sent it yesterday, did you get it? Forget it, I just saw that you have accepted the invitation.

@Valefant
Copy link
Collaborator

Valefant commented Sep 8, 2020

@Valefant I have re-sent it yesterday, did you get it? Forget it, I just saw that you have accepted the invitation.

I got it. Thank you @robstoll 🙂

@robstoll robstoll added this to the 0.14.0 milestone Sep 8, 2020
@robstoll
Copy link
Owner Author

@Valefant do you need help with this one? No stress, I just want to offer my help

@khliwa
Copy link

khliwa commented Sep 29, 2020

@robstoll I could takeover if @Valefant doesn't want to continue it. Or I'll gladly take some other entry-level issue if you have something available.

@robstoll
Copy link
Owner Author

robstoll commented Sep 29, 2020

@khliwa feel free to start, in the worst case you both do the same and you can compare the two solutions

@Valefant
Copy link
Collaborator

@robstoll Sry for replying so late. I would start working on it now.
@khliwa What's your status? :-)

Valefant pushed a commit to Valefant/atrium that referenced this issue Oct 11, 2020
Valefant pushed a commit to Valefant/atrium that referenced this issue Oct 14, 2020
@Valefant
Copy link
Collaborator

This issue can be closed as the pull request was accepted

@robstoll robstoll linked a pull request Oct 15, 2020 that will close this issue
jakubriegel pushed a commit to jakubriegel/atrium that referenced this issue Oct 16, 2020
# This is the 1st commit message:

robstoll#630 add Path.isRelative method

# This is the commit message robstoll#2:

robstoll#482 accept date as string for chronozoneddatetime (robstoll#625)
jakubriegel pushed a commit to jakubriegel/atrium that referenced this issue Oct 16, 2020
# This is the 1st commit message:

robstoll#630 add Path.isRelative method

# This is the commit message robstoll#2:

robstoll#482 accept date as string for chronozoneddatetime (robstoll#625)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants