add Path.hasDirectoryEntry as shortcut for Path.isDirectory + Path.resolve + Path.exists

[robstoll]

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

Code related feature

expect(path).hasDirectoryEntry("test.txt", "dummy.log")

//instead of

expect(path).isDirectory() and {
  resolve("test.txt") { exists() }
  resolve("dummy.log") { exists() }
}

Following the things you need to do:

api-fluent

• provide a fun contains which expects path: String, vararg otherPaths: String and returns Expect<T> (see resolve with expect an assertionCreator in pathAssertions.kt as a guideline)
• add @since 0.14.0 (adapt to current milestone) to KDOC
• add contains to PathAssertionsSpec in specs-common and extend it in atrium-api-fluent-en_GB-common/src/test

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).

[SylvainGirod]

Hello, can I work on this ?

[robstoll]

Sure

[Xeitor]

Hello, I'll work on this...

[SylvainGirod]

Hello, I'll work on this...

I think you can if @robstoll is ok. I couldn't manage to take the time to work on this.

[robstoll]

@ezesalas7 go ahead and let me know if you need help.
@SylvainGirod come back once you have time again, your help is always appreciated.

[Xeitor]

Thanks @robstoll, I am setting up the project now. First time contributing to open source, pretty exited! Hope I can get It done :)

[jgrgt]

I'll work on this

[robstoll]

@jgrgt there is already someone working on this. Please take another issue unless @ezesalas7 does not have time

[robstoll]

@jgrgt sorry, I have not seen that @ezesalas7 unassigned himself. So go ahead 👍

[robstoll]

@jgrgt unassigning so that others can take over. Let us know in case you still work on it (or if you need help)

[jgrgt]

@robstoll Sorry about the lack of updates, I got stuck. Naming the function contains caused naming conflicts I could not resolve. I have something working with containss as a name for now. And still the build fails at the moment, with:

> Task :atrium-api-infix-en_GB-jvm:compileTestKotlin FAILED
e: GitHub/atrium/apis/infix-en_GB/atrium-api-infix-en_GB-jvm/src/test/kotlin/ch/tutteli/atrium/api/infix/en_GB/PathAssertionsSpec.kt: (3, 30): Unresolved reference: fluent
e: GitHub/atrium/apis/infix-en_GB/atrium-api-infix-en_GB-jvm/src/test/kotlin/ch/tutteli/atrium/api/infix/en_GB/PathAssertionsSpec.kt: (26, 24): Unresolved reference: containss

[robstoll]

No problem, re-assigned you. Please push everything you have, always easier to review and help.
About the particular issue. You try to access api-fluent from api-infix. Check your imports. In api-infix you should not have import ch.tutteli.atrium.api.fluent

[jgrgt]

@robstoll I pushed my code to a branch on my clone. Here's a diff https://github.com/robstoll/atrium/compare/master...jgrgt:path-contains?expand=1 Not sure if I'm allowed to create a 'WIP' pull request for this or not.

[robstoll]

You can always create a WIP pull request, no problem. Please do so, easier to keep track of changes for me.

[jGleitz]

I know I am late to the party, but I have an objection against this method being called “contains”. Path implements Iterable<Path>, which means that there is Expect<Path>.contains(p: Path). This leads to an odd situation:

expect(Path.of("/foo/bar")).contains(Path.of("bar")) // will succeed
expect(Path.of("/foo/bar")).contains("bar") // will or will not succeed, depending on the content of the /foo/bar directory

I think it is bad that we have Expect<Path>.contains(p: Path) and Expect<Path>.contains(s: String) with completely different semantics. Intuitively, I would expect Expect<Path>.contains(s: String) = contains(Path.of(s)).

Since it is not released yet, now would be the time to rename the method. Do you agree that we should? I’d suggest something like hasDirectoryEntry.

[robstoll]

@jGleitz good you spotted this. I forgot that Path implements Iterable. Would you like to do the fix?

[jGleitz]

I am glad to raise a PR. Is hasDirectoryEntry good?

[robstoll]

@jGleitz I think name is OK and better than hasEntry as this would not imply that the path is a directoy

[jGleitz]

How is this assertion supposed to behave for symbolic link entries? I would expect that the assertion resolves a symbolic link if the subject is a symbolic link, but does not follow symbolic links for the directory entries. However, the current implementation also resolves symbolic links for the file system entries.

For example, this assertion:

expect(Path.of("/foo/bar")).hasDirectoryEntry("baz")

would fail with the current implementation if /foo/bar/baz is a symbolic link that points to a non-existent location. However, I intuitively would expect the assertion not to fail in this case.

What do you think?

[robstoll]

nice catch, I agree, I would intuitively also expect it to hold if the link is there regardless where it points to.
So... since the implementation will be more complicated than just a combination of two functions, I suggest you move it to logic in your PR.