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

Add samples for pathExpectations of api-infix (#999) #1003

Merged

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.

@rhushikesh rhushikesh mentioned this pull request Oct 9, 2021
11 tasks
@robstoll robstoll linked an issue Oct 9, 2021 that may be closed by this pull request
2 tasks
@codecov
Copy link

codecov bot commented Oct 9, 2021

Codecov Report

Merging #1003 (1ebfa0e) into main (b1b8b6e) will not change coverage.
The diff coverage is n/a.

❗ Current head 1ebfa0e differs from pull request most recent head 37233cd. Consider uploading reports for the commit 37233cd to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1003   +/-   ##
=======================================
  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 Δ
...tutteli/atrium/api/infix/en_GB/pathExpectations.kt 100.00% <ø> (ø)
...teli/atrium/api/infix/en_GB/throwableAssertions.kt 100.00% <0.00%> (ø)
...teli/atrium/api/fluent/en_GB/sequenceAssertions.kt 100.00% <0.00%> (ø)
...li/atrium/api/infix/en_GB/throwableExpectations.kt 100.00% <0.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...37233cd. Read the comment docs.

@rhushikesh
Copy link
Contributor Author

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

Comment on lines 144 to 147
expect(dir) resolve "test_file.ttt" toEqual fileInDir toEndWith Paths.get("ttt")
// | not reported `toEqual(fileInDir)` already fails
// | fails because resolve returns *test_file.ttt and fileInDir equals *test_file.txt
// | use `resolve other { ... }` if you want that all expectations are evaluated
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
expect(dir) resolve "test_file.ttt" toEqual fileInDir toEndWith Paths.get("ttt")
// | not reported `toEqual(fileInDir)` already fails
// | fails because resolve returns *test_file.ttt and fileInDir equals *test_file.txt
// | use `resolve other { ... }` if you want that all expectations are evaluated
expect(dir) resolve "test_file.ttt" toEqual fileInDir toEndWith Paths.get("ttt")
// | | | not reported `toEqual(fileInDir)` already fails
// | | fails because resolve returns *test_file.ttt and fileInDir equals *test_file.txt
// | use `resolve other { ... }` if you want that all expectations are evaluated

@robstoll
Copy link
Owner

@rhushikesh Thanks for the adjustments. I suggested one small improvement for the comments.
On the other hand, the PR is missing the samples for the functions in pathAssertions.kt (e.g. toBeASymbolicLink) -- I guess you plan to add them as well to this PR. I suggest you go trough apis/fluent-en_GB/atrium-api-fluent-en_GB-jvm/src/test/kotlin/ch/tutteli/atrium/api/fluent/en_GB/samples/PathExpectationSamples.kt and check that all functions there have an equivalent in your samples class

@rhushikesh
Copy link
Contributor Author

Hi @robstoll, I have updated PR with your review comments. Also I have added samples for pathAssertions. Please have a look

Copy link
Owner

@robstoll robstoll left a comment

Choose a reason for hiding this comment

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

@ratkayandras we are nearly there, some small adjustments and we can merge

val dir = tempDir.newDirectory("test.$extension")


expect(dir) extension { // subject is now of type String (actually "txt")
Copy link
Owner

Choose a reason for hiding this comment

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

more precise would be (I know you copied it from api-fluent, should be improved there as well)

Suggested change
expect(dir) extension { // subject is now of type String (actually "txt")
expect(dir) extension { // subject inside this block is of type String (actually "txt")

}

fails {
expect(dir) extension { // subject is now of type String (actually "txt")
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
expect(dir) extension { // subject is now of type String (actually "txt")
expect(dir) extension { // subject inside this block is of type String (actually "txt")

Comment on lines +279 to +283
expect(dir).extension toEqual "txtt" toEndWith "jpg"
// | subject is now of type String (actually "txt")
// | | fails because it doesn't equal to "txtt"
// | | not reported
// | use `.extension` if you want that all expectations are evaluated
Copy link
Owner

Choose a reason for hiding this comment

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

I suggest you style it the same way you did it further below in fileNameFeature

Suggested change
expect(dir).extension toEqual "txtt" toEndWith "jpg"
// | subject is now of type String (actually "txt")
// | | fails because it doesn't equal to "txtt"
// | | not reported
// | use `.extension` if you want that all expectations are evaluated
expect(dir).extension toEqual "txtt" toEndWith "jpg"
// | | | not reported
// | | fails because it doesn't equal to "txtt"
// | subject is now of type String (actually "txt")
// | use ` extension { ... }` if you want that all expectations are evaluated

Note the change for extension { ... } Not your fault, it's actually also wrong in api-fluent. If you don't mind, would be great, if you could fix it there as well => use .extension { ... } there.

fun fileName() {
val dir = tempDir.newDirectory("test_dir")

expect(dir) fileName { // subject is now of type String (actually "test_dir")
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
expect(dir) fileName { // subject is now of type String (actually "test_dir")
expect(dir) fileName { // subject inside this block is of type String (actually "test_dir")

same error in api-fluent

fun fileNameWithoutExtension() {
val dir = tempDir.newDirectory("test_dir")

expect(dir) fileNameWithoutExtension { // subject is now of type String (actually "test_dir")
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
expect(dir) fileNameWithoutExtension { // subject is now of type String (actually "test_dir")
expect(dir) fileNameWithoutExtension { // subject inside this block is of type String (actually "test_dir")

same same api-fluent

}

fails { // because fileNameWithoutExtension equals `test_dir`
expect(dir) fileNameWithoutExtension { // subject is now of type String (actually "test_dir")
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
expect(dir) fileNameWithoutExtension { // subject is now of type String (actually "test_dir")
expect(dir) fileNameWithoutExtension { // subject inside this block is of type String (actually "test_dir")

Comment on lines +405 to +407
it toEqual dir3 // fails because dir3 and dir does not have same parents
it notToBe existing // still evaluated even though `toEqual(dir3)` already fails
// use `.parent` if you want a fail fast behaviour
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
it toEqual dir3 // fails because dir3 and dir does not have same parents
it notToBe existing // still evaluated even though `toEqual(dir3)` already fails
// use `.parent` if you want a fail fast behaviour
it toEqual dir3 // fails because dir3 and dir does not have same parents
it notToBe existing // still evaluated even though `toEqual(dir3)` already fails
// use `.parent` if you want a fail fast behaviour

expect(dir) resolve "test_file.ttt" toEqual fileInDir toEndWith Paths.get("ttt")
// | | | not reported `toEqual(fileInDir)` already fails
// | | fails because resolve returns *test_file.ttt and fileInDir equals *test_file.txt
// | use `resolve other { ... }` if you want that all expectations are evaluated
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
// | use `resolve other { ... }` if you want that all expectations are evaluated
// | use `resolve path(other) { ... }` if you want that all expectations are evaluated

expect(dir) resolve path("test_file.txt") {
it toEqual fileInDir // fails
it toBe existing // still evaluated even though `toEqual(fileInDir)` already fails
// use `.resolve(other).` if you want a fail fast behaviour
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
// use `.resolve(other).` if you want a fail fast behaviour
// use `resolve other` if you want a fail fast behaviour

…/tutteli/atrium/api/infix/en_GB/samples/PathExpectationSamples.kt
@robstoll robstoll changed the base branch from main to samples-localDate-localDateTime October 12, 2021 20:37
@robstoll robstoll merged commit f766284 into robstoll:samples-localDate-localDateTime Oct 12, 2021
@robstoll
Copy link
Owner

@rhushikesh thanks for your 3 contribution to Atrium, you're on 🔥

I am going to fix the remaining things myself. I won't have time to look at 1004 tonight but will do tomorrow

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

Successfully merging this pull request may close these issues.

add samples for pathExpectations of api-infix
2 participants