Adding Path.contains #640
Adding Path.contains #640
Conversation
There was a problem hiding this comment.
@jgrgt I don't get the naming conflict. Please try again, address all the review comments respectively. Maybe only the error in infix confused you.
Edit as you can see, you need to rebase your commit on master. Let me know in case you have never done that and the following does not help:
https://github.com/robstoll/atrium/blob/master/.github/CONTRIBUTING.md#git
...s/atrium-specs-jvm/src/main/kotlin/ch/tutteli/atrium/specs/integration/PathAssertionsSpec.kt
Show resolved
Hide resolved
...s/atrium-specs-jvm/src/main/kotlin/ch/tutteli/atrium/specs/integration/PathAssertionsSpec.kt
Show resolved
Hide resolved
| expect { | ||
| expect(folder).containsFun("a", arrayOf("b", "c")) | ||
| }.toThrow<AssertionError>().message { | ||
| contains("${TO.getDefault()}: ${EXIST.getDefault()}") |
There was a problem hiding this comment.
same here as above.
I would like to see the use case in addition where the 1. and the 3rd. don't exist to be sure that both are reported.
...en_GB-jdk8-jvm/src/test/kotlin/ch/tutteli/atrium/api/fluent/en_GB/jdk8/PathAssertionsSpec.kt
Outdated
Show resolved
Hide resolved
...-api-infix-en_GB-jvm/src/test/kotlin/ch/tutteli/atrium/api/infix/en_GB/PathAssertionsSpec.kt
Outdated
Show resolved
Hide resolved
...-api-infix-en_GB-jvm/src/test/kotlin/ch/tutteli/atrium/api/infix/en_GB/PathAssertionsSpec.kt
Outdated
Show resolved
Hide resolved
...pi-fluent-en_GB-jvm/src/test/kotlin/ch/tutteli/atrium/api/fluent/en_GB/PathAssertionsSpec.kt
Outdated
Show resolved
Hide resolved
...um-api-fluent-en_GB-jvm/src/main/kotlin/ch/tutteli/atrium/api/fluent/en_GB/pathAssertions.kt
Outdated
Show resolved
Hide resolved
|
@robstoll Thanks for the thorough review and the many hints. 😄 I pushed with most, if not all, of your remarks processed. The build will fail in |
|
@jgrgt I'll take a look for the problem. Could you please rebase in the meantime? |
|
@jgrgt you still have the following in the spec: should be |
….api.fluent.en_GB.jdk8.PathAssertionsSpec
|
@robstoll Rebased, fixed the leftover |
|
👍 good, now we are almost there, there are three/four things open (I already did some of them): ✔️ 1. Concerning the spec, it seems like we need to help the Kotlin compiler as it is not able to figure out the types by its own (it's basically a bug in the type inference algorithm IMO). rewrite to: ✔️ 2. The spec fails, try to fix it (OK, might be hard, so I left a review comment)
bonus |
...-api-infix-en_GB-jvm/src/test/kotlin/ch/tutteli/atrium/api/infix/en_GB/PathAssertionsSpec.kt
Outdated
Show resolved
Hide resolved
…/tutteli/atrium/api/infix/en_GB/PathAssertionsSpec.kt
...en_GB-jdk8-jvm/src/test/kotlin/ch/tutteli/atrium/api/fluent/en_GB/jdk8/PathAssertionsSpec.kt
Outdated
Show resolved
Hide resolved
...pi-fluent-en_GB-jvm/src/test/kotlin/ch/tutteli/atrium/api/fluent/en_GB/PathAssertionsSpec.kt
Outdated
Show resolved
Hide resolved
…ch/tutteli/atrium/api/fluent/en_GB/PathAssertionsSpec.kt
…-jdk8-jvm/src/test/kotlin/ch/tutteli/atrium/api/fluent/en_GB/jdk8/PathAssertionsSpec.kt
|
@jgrgt Let me know in case you need more information regarding the last open point, point 3 in #640 (comment) |
|
@robstoll Your changes made everything compile, so the hard part is done now. I should be able to finish the other steps next week. |
|
@jgrgt IMO it is good to merge, please remove the WIP prefix of your PR in case you don't intend to do further things. |
|
@jgrgt many thanks for your 2nd contribution to Atrium 🎉 how about a 3rd one? |
Current issues:
Should fix #590
I confirm that I have read the Contributor Agreements v1.0, agree to be bound on them and confirm that my contribution is compliant.