Now that 1.4 is out, what's the status of feature testing new syntax?

[StragaSevera]

The Readme says:

"We are sorry that the syntax is not yet the nicest one. We admit that one has to get used to it first and that is a pity. Yet, it is due to many Kotlin Bugs standing in the way -- we hope we can provide a better API once Kotlin 1.4 is out (the new type inference respectively)."

Now that 1.4 is out, what's the status of an improved syntax? If it is still not possible, then maybe the Readme should be improved?..

[robstoll]

Thanks for bringing that one up.
I did not have the time yet to look into it fully but I did a quick glance when Kotlin 1.4 came out and I was rather disappointed in this regard, the type inference system is still not able to differentiate based on the inner definitions of a lambda -- see for instance this example

I saw somewhere, that an internal Kotlin annotation (I don't remember the name, I think it was something with Builder in it) might help to trigger an experimental feature of the type inference system but I don't know if it would really help here.
Also there are still some issues open I have in the context of inference and overload resolution problems, unfortunately also after Kotlin 1.4.

Besides, we are currently not yet able to provide an improved syntax as we first need to migrate from the old MPP plugin to the new MPP plugin (the old is no longer supported in Kotlin 1.4). See also #744. So I already agree, that we should update the Readme to reflect this.

Nevertheless, I was lately again thinking about this issue, since also other people reached out to me that the syntax of feature assertions is not easy. Since you opened this issue, I am interested in your opinion. I plan to introduce a new function (name yet to be defined, in the following named its) which allows that one extracts a feature without using reflection. Like this:

expect(person).its { firstName }.toBe("Ilya")

The reason why I have not done that so far (I thought about this many times) is that the reporting will look bad, as I don't know what feature was extracted:

expected that the subject: Person(firstName="Robert", lastName="Stoll")
* -> feature: "Robert"
      * to be : "Ilya"

Maybe this isn't as bad as I think because the stacktrace will still point to the correct line? Yet, this changes if one defines multiple features in an assertion group block as follows:

expect(person) { 
  its { firstName }.toBe("Ilya")
  its { lastName }.toBe("Fedorinov")
}

Because the stacktrace would point to expect and one would not immediately see the correct line. I think one can still see what went wrong but requires a bit more thinking - a re-read of the code:

expected that the subject: Person(firstName="Robert", lastName="Stoll")
* -> feature: "Robert"
      * to be : "Ilya"
* -> feature: "Stoll"
      * to be : "Fedorinov"      

Therefore, I consider to add at least a counter so that one gets a little hint in addition

expected that the subject: Person(firstName="Robert", lastName="Stoll")
* -> feature 1: "Robert"
      * to be : "Ilya"
* -> feature 2: "Stoll"
      * to be : "Fedorinov"      

Not nearly as nice as

expected that the subject: Person(firstName="Robert", lastName="Stoll")
* -> firstName: "Robert"
      * to be : "Ilya"
* -> lastName: "Stoll"
      * to be : "Fedorinov"      

but maybe the improved readability of the code outweighs the poorer readability of the report?

So, what I would like to know from you (and any other reading this):

• would you use the new syntax even if the reporting is worse? And if yes, do you think the counter would help?
• do you have a suggestion for the name of the function?

[StragaSevera]

Hmm, an interesting concept, I never thought about this aspect, I always thought that some reflection magic can extract the feature name from the lambda.

Personally, I would be glad to have a readable syntax even at the cost of worse reporting - the indexing will give enough info for me to use this syntax.

And I think the its name is perfectly fine - it is human-readable and short. For me the current syntax is too verbose partly because of the long and attention-grabbing feature function name - seven characters instead of three! - and if there would be a shorter overload like its, it would be readable in case I want improved reporting =-)

By the way, maybe there would be a way for a stack-trace to point to the correct line? For example, if I don't need the "compounding" behaviour, and I'm fine with the test stopping at the first failing assetion, I could use this:

expect(person).all { 
  its { firstName }.toBe("Ilya")
  its { lastName }.toBe("Fedorinov")
}

, and this syntax (instead of just expect(person) {}) will tell the Atrium to just throw error on a first its line, and the stack trace will show the correct line with failing assertion.

[robstoll]

Thanks for your feedback. Although I like its for extracting properties and the like, it does not work that well for methods. E.g.

expect(list) {
  its { last }... // reads nice
  its { filter { it > 2 } }... // less nice
}

In case you come up with a better suggestion, then please share it with us.

[robstoll]

Updated the README in the meantime. Keeping this issue open because I think it could be of interest for others to know the status quo and as a reminder that I want to look into it at some point.

[StragaSevera]

I think that it works fine for methods too - because we are testing its filter ;-)

Seriously, though, the main benefit of this method is the length - only 3 symbols =-) I don't think there is a concise word that can replace it.

[robstoll]

Not entirely wrong that we are testings its filter 🙂
OK let's go with its. I'll probably add it in v0.16.0

[StragaSevera]

Thank you, will wait for it eagerly! =-)

And what do you think about the all syntax (with compounding behaviour disabled)? This may be another way to fix the "which assertion is failing" problem.

[jGleitz]

@StragaSevera I think this idea needs its own ticket. I agree with the feature you want to see (jumping directly to the first failing assertion from the stack trace) but disagree with the solution you propose. So let’s discuss in a separate ticket, shall we?

[StragaSevera]

Ok, will open it =-)

[robstoll]

I am closing this issue as I opened a new one for its

[robstoll]

@StragaSevera did you have chance to try out its? Let me know if you think it should be improved in any way.