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.

Sign up for GitHub

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

Jump to bottom

add samples for localDateTimeAssertions (#997) #1005

Merged
merged 6 commits into from
Oct 12, 2021
Merged

add samples for localDateTimeAssertions (#997) #1005

merged 6 commits into from
Oct 12, 2021

Conversation

rhushikesh
Copy link
Contributor

PR for this issue

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

@robstoll robstoll linked an issue Oct 9, 2021 that may be closed by this pull request
add samples for localDateTimeAssertions of api-fluent #997
Closed
3 tasks
@codecov
Copy link

codecov bot commented Oct 9, 2021 •
edited
Loading

Codecov Report

Merging #1005 (04a0e50) into main (b1b8b6e) will not change coverage.
The diff coverage is n/a.

❗ Current head 04a0e50 differs from pull request most recent head 95830d2. Consider uploading reports for the commit 95830d2 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1005   +/-   ##
=======================================
  Coverage   90.86%   90.86%           
=======================================
  Files         433      433           
  Lines        4359     4359           
  Branches      219      219           
=======================================
  Hits         3961     3961           
  Misses        349      349           
  Partials       49       49
Flag Coverage Δ
bbc 80.17% <ø> (ø)
bc 80.04% <ø> (ø)
current 87.13% <ø> (ø)
current_windows 86.21% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...atrium/api/fluent/en_GB/localDateTimeAssertions.kt 100.00% <ø> (ø)
.../atrium/api/infix/en_GB/localDateTimeAssertions.kt 100.00% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b1b8b6e...95830d2. Read the comment docs.

@rhushikesh
Copy link
Contributor Author

Hi @robstoll, I have updated PR with your review comments. Please have a look

@rhushikesh rhushikesh mentioned this pull request Oct 10, 2021
Closed
3 tasks
robstoll
robstoll requested changes Oct 10, 2021
View reviewed changes
...rc/test/kotlin/ch/tutteli/atrium/api/fluent/en_GB/samples/LocalDateTimeExpectationSamples.kt Show resolved Hide resolved
...rc/test/kotlin/ch/tutteli/atrium/api/fluent/en_GB/samples/LocalDateTimeExpectationSamples.kt Outdated
// subject inside this block is of type Int (actually 2021)
toEqual(2021)
toBeGreaterThan(2020)
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
}
} // subject here is back to type LocalDateTime

same same in other places

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, will make that change

...src/test/kotlin/ch/tutteli/atrium/api/infix/en_GB/samples/LocalDateTimeExpectationSamples.kt

@Test
fun yearFeature() {
expect(LocalDateTime.of(2021, Month.OCTOBER, 9, 11, 56)).year toEqual 2021
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you please also add comment about the subject change. See e.g.apis/infix-en_GB/atrium-api-infix-en_GB-common/src/test/kotlin/ch/tutteli/atrium/api/infix/en_GB/samples/AnyExpectationSamples.kt => notToEqualNullFeature

Suggested change
expect(LocalDateTime.of(2021, Month.OCTOBER, 9, 11, 56)).year toEqual 2021
expect(LocalDateTime.of(2021, Month.OCTOBER, 9, 11, 56)).year toEqual 2021
// | subject is now of type Int

same same further below

@rhushikesh
Copy link
Contributor Author

Hi @robstoll, I have updated PR with your review comments. Please have a look

@rhushikesh rhushikesh mentioned this pull request Oct 12, 2021
Merged
robstoll
robstoll approved these changes Oct 12, 2021
View reviewed changes
Copy link
Owner

@robstoll robstoll left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rhushikesh thanks for adding the comments. I am going to address the remaining details myself

...rc/test/kotlin/ch/tutteli/atrium/api/fluent/en_GB/samples/LocalDateTimeExpectationSamples.kt
Comment on lines +28 to +29
.year {
// subject inside this block is of type Int (actually 2021)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
.year {
// subject inside this block is of type Int (actually 2021)
.year { // subject inside this block is of type Int (actually 2021)

@robstoll robstoll changed the base branch from main to samples-localDate-localDateTime October 12, 2021 19:40
@robstoll robstoll merged commit a6b0bea into robstoll:samples-localDate-localDateTime Oct 12, 2021
@robstoll
Copy link
Owner

robstoll commented Oct 12, 2021 •
edited
Loading

@rhushikesh thanks for you first contribution to Atrium 👍
I know that further are in the pipeline, that's great. Continue like that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@robstoll robstoll robstoll approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

add samples for localDateTimeAssertions of api-fluent
2 participants
@rhushikesh @robstoll