incorporate filepeek

[robstoll]

so that we can write:

expect(Person(...))
  .feature { name }
  .toBe("robert")

instead of feature { f(it::name) }

however, it works currently only for JVM.
Thus, either we wait until its implemented for JS or we provide this feature only for JVM.
Moreover, it can currently not deal with the case that the test source is not available. I don't want that tests fail due to this.

[jGleitz]

Wow, I am sensing a lot of trouble and errors that are difficult to understand for users here.

Maybe I am mistaken, but this would almost certainly lead to tests that work in one place and suddenly fail in another. Even if we provided the most helpful of failure messages, I fear that the trouble for users is not justified by the convenience gain.

By the way: Even though I have read through robstoll/atrium#40, I still don’t understand why we cannot provide methods property(x: …) = feature { p(x) } and returnValueOf(x: …) = feature { f(x) }. Is there an easy answer to that?

Edit: To be more precise, I am suggesting something like:

fun <T, R> Expect<T>.property(provider: T.() -> KProperty0<R>): Expect<R> =
    feature { p(provider(it)) }

The way I imagine it, we could then write:

expect(Person(...))
  .property { ::name }
  .toBe("robert")

Which is almost as good as your example, but without Voodoo. I think it solves the ambiguity issues as well as the current solution. What am I missing?

[robstoll]

Wow, I am sensing a lot of trouble and errors that are difficult to understand for users here.

Could you please elaborate in case you are not talking about IO errors?
I have not yet incorporated filepeek into Atrium for three reasons:

1. it should work on all platforms and I am not sure if it will ever work for native (might well be that I we will progress in this direction)
2. it performs IO to read the sources and this can fail in various ways (not only IO but also security wise, the source might not be available etc.)
3. as it performs IO, I expect that it will significantly slow down feature assertions.

By the way: Even though I have read through robstoll/atrium#40, I still don’t understand why we cannot provide methods property(x: …) = feature { p(x) } and returnValueOf(x: …) = feature { f(x) }. Is there an easy answer to that?

There is an easy answer to this, Kotlin's type inference algorithm is not good enough. It cannot distinguish lambdas based on their return type. See the many bugs I created and listed on Atrium's README concerning type inference and overload resolution ambiguities. I hope this will get better. As soon as it gets better I would improve the syntax.

Regarding your imaginations, that's more or less what I planed to do if Kotlin were better. So please upvote all issues regarding type inference and overload resolution ambiguities here https://github.com/robstoll/atrium#kotlin-bugs as well as the following https://youtrack.jetbrains.com/issue/KT-22920 (would allow to write { ::name })

[robstoll]

Another approach to allow feature { name } could be a compiler plugin. However, I don't know how powerful a compiler plugin can be (e.g. change code) and to which level it gets access to the code.

Edit I had a quick glance at compiler plugins, they could modify code, however, as it seems there is quite a bit of overhead involved and it would require an own gradle plugin => looking at https://github.com/robstoll/atrium/wiki/Requirements#r101-little-custom-build-logic this is probably not what we want

[jGleitz]

Could you please elaborate in case you are not talking about IO errors?

Maybe my understanding of your proposed approach is bad. But I understand that you want to access the source code in order to understand “what the user means” at runtime. As you have said, filepeek would rely on the sources being available at runtime. And I feel like this could be a major problem: The tests run successfully, then they are run differently, such that accessing the source code is not possible, and suddentl stop working.

There is an easy answer to this, Kotlin's type inference algorithm is not good enough.

That only means that my proposed approach would fail to compile in some cases, right? It would still be useful to have it for the simple cases, would it not?

For example, with property defined as above, this code compiles:

expect("testString")
    .property { ::length }
    .isGreaterThan(3)

I do not get code completion after ::, but for everything else.

What I am trying to achive here is getting the syntax as good as possible, even if it is just for common and not for all cases, without needing to resort to complicated solutions.

I see that things get a lot complicater with functions and that most code would not compile due to resolution ambiguity.

But can we at least add the property overload for better readability?

Another approach to allow feature { name } could be a compiler plugin.

How likely do you think it that the Kotlin bugs standing in our way will get fixed in the near future? Because if can avoid writing a compiler plugin, maybe we should avoid it.
We also would not get IDE completion without writing an IntelliJ plugin, would we?

[robstoll]

Maybe my understanding of your proposed approach is bad

It's perfectly fine and I share your concerns as outlined above because IO opertions seem not to be a good fit for testing IMO. However, I was in contact with Christoph Sturm, the author of filepeek and asked him what experiences he made and he said so far it was good. That is why I am considering it again.

One thing I tried to simplify with feature was to get rid of the distinction between property and 0 arity functions. Meaning, regardless if length above is a property or a function, we can use feature { i(it::length) } and don't have to make the distinction by either using property or returnValueOf.
I think this is an improvement as people run into this problem. But we can discuss that again but have to settle before we release 0.9.0. I would definitely vote for property { it::length } over property { ::length } as code completion is crucial IMO. In case you want to try out combining property and 0 arity functions, then try to play with WorstCase

---

We wouldn't need to build a IntelliJ plugin as we only inspect code and transform it. For instance, we would expand feature { children.first().name } to feature("children.first().name") { name }

[jGleitz]

I think we can give it a shot if we make sure that it

a) does still supports autocomplete
b) does not fail if the sources are not available, but has only worse reporting at worst

I still doubt that the improvement is worth the effort. Even though the current p & f solution has drawbacks, I find it tolerable. I feel like improvements in other areas of atrium would be more important.

I would definitely vote for property { it::length } over property { ::length } as code completion is crucial IMO.

Counter argument: If property { ::length } is what we aim for—it is what I aim for because I find the it confusing—we would need to change the API again once the code completion bug is fixed. I agree that code completion is important. But if we are sufficiently sure that the bug will be fixed in the forseeable future, I, personally, could tolerate not having autocompletion until then.

[robstoll]

Good, I suggest we wait with filepeek until it supports JS, then we can reassess.

I agree that code completion is important. But if we are sufficiently sure that the bug will be fixed in the forseeable future

Personally I am not sure at all, the issue was created 2 years ago and has still no target version. So most likely it will make it into 1.4.x but not earlier.

If I find time I will look into a compiler plugin but I am with you that this is not really a high prio issue.