Refactor All Expectations to Have the Form ‘to + infinitive’

[jGleitz]

As we started to discuss in #92, it might make sense to refactor atrium’s API so that all assertions read like ‘expect (…).to + infinitve’, e.g. ‘expect(x).toBe(y)’, ‘expect(x).toStartWith(y)’, ‘expect(x).all { toBeLessThan(y) }’, etc.

This would mean a massive change to the API, as nearly all assertion functions would need to change. Luckily, the process could be eased using Kotlin’s replacement feature for @Deprecated, but it would still put a huge burden on all users, as they’d need to migrate at some point¹.

In this ticket, I want to evaluate whether the change would be worth it. I see these advantages:

• Grammatical correctness. I think the sentence “I expect x starts with y” is not correct. Correct would be “I expect that x starts with y”. However, all language guides I could find use the form “I expect x to start with y”.
• Better readability in other contexts. The current forms are often in singular form (‘startsWith’, ‘isLessThan’, …), which makes them awkward to use in contexts where the subject is a Collection.
• Using to and notTo as a prefix has some advantages, as you always start with to or notTo to narrow the API surface. Thus, code completion will thus be better.

In this ticket, I will try to collect examples of how different assertions would look like after the change. At the time of writing, I think that the change would only improve the API, without introducing cases that are worse. By collecting many examples, I want to confirm this.

Input from anybody is highly welcome!

¹ we might discuss whether we’d need to remove the old assertion verbs at all. We could probably mark all assertions as @Deprecated, move them into their own module and let them live there forever. Atrium separates API and logic so clearly, that this might not add a significant maintenance burden.

[jGleitz]

To start the discussion, let’s go through assertions that are mentioned in the README and see how they’d change:

Currently	to + infinitive
expect(x).toBe(9)	expect(x).toBe(9)
expect(4 + 6) { isLessThan(5) isGreaterThan(10) }	expect(4 + 6) { toBeLessThan(5) toBeGreaterThan(10) }
expect { throw IllegalArgumentException("name is empty") }.toThrow<IllegalStateException> { message { startsWith("firstName") }	expect { throw IllegalArgumentException("name is empty") }.toThrow<IllegalStateException> { message { toStartWith("firstName") } }
expect(x).isA<SubType1>() .feature { f(it::number) } .toBe(2)	expect(x).toBeA<SubType1>() .feature { f(it::number) } .toBe(2)
expect(listOf(1, 2, 2, 4)).contains(2, 3)	expect(listOf(1, 2, 2, 4)).toContain(2, 3)
expect(listOf(1, 2, 2, 4)).contains( { isLessThan(0) }, { isGreaterThan(2).isLessThan(4) } )	expect(listOf(1, 2, 2, 4)).toContain( { toBeLessThan(0) }, { toBeGreaterThan(2).toBeLessThan(4) } )
expect(listOf(1, 2, 3, 4)).any { isLessThan(0) }	expect(listOf(1, 2, 3, 4)).any { toBeLessThan(0) }
expect(listOf(1, 2, 3, 4)).all { isGreaterThan(2) }	expect(listOf(1, 2, 3, 4)).all { toBeGreaterThan(2) }
expect(Paths.get("/usr/bin/noprogram")).exists()	expect(Paths.get("/usr/bin/noprogram")).toExist()
expect(Paths.get("/root/.ssh/config")).isWritable()	expect(Paths.get("/root/.ssh/config")).toBeWritable()

My takeaway is: Most things are at least as good in the ‘to + infintive’ form as they are today. However, we inconsistently already use ‘to + infinitive’ from time to time (e.g. toBe, toThrow, the example firstToBeDoneWrong), hence, the API would become more consistent. The inconsistency is particularly remarkable with isA vs. toBe.

As expected, assertions using the all operator improve because they become grammatically correct.

Areas where I see trouble:

• expect(listOf(1, 2, 2, 4)).toContain({ toBeLessThan(0) }, { toBeGreaterThan(2).toBeLessThan(4) }) reads a little more awkward than before. I suggest to rename it to toContainElementsFound:

Currently	to + infinitive
expect(listOf(1, 2, 2, 4)).contains( { isLessThan(0) }, { isGreaterThan(2).isLessThan(4) } ) expected that subject: [1, 2, 2, 4] (java.util.Arrays.ArrayList <1234789>) ◆ contains, in any order: ⚬ an entry which: » is less than: 0 (kotlin.Int <1234789>) ⚬ ▶ number of such entries: 0 ◾ is at least: 1 ⚬ an entry which: » is greater than: 2 (kotlin.Int <1234789>) » is less than: 4 (kotlin.Int <1234789>) ⚬ ▶ number of such entries: 0 ◾ is at least: 1	expect(listOf(1, 2, 2, 4)).toContainElementsFound( { toBeLessThan(0) }, { toBeGreaterThan(2).toBeLessThan(4) } ) expected that subject: [1, 2, 2, 4] (java.util.Arrays.ArrayList <1234789>) ◆ to contain, in any order: ⚬ an entry found: » to less than: 0 (kotlin.Int <1234789>) ⚬ ▶ number of such entries: 0 ◾ to be at least: 1 ⚬ an entry found: » to be greater than: 2 (kotlin.Int <1234789>) » to be less than: 4 (kotlin.Int <1234789>) ⚬ ▶ number of such entries: 0 ◾ to be at least: 1

[robstoll]

if we are going to change the functions then I would do it the same way as we did with other assertion functions we deprecated so far. Deprecate it and remove them at some major version, which means they might stay for some time but eventually be removed.

https://www.ldoceonline.com/dictionary/i-expect

I expect...
British English spoken: used to introduce or agree with a statement that you think is probably true

I guess that's the reason why I am used to it and maybe the reason why you aren't? I am pretty sure I have heard sentences like: I expect the food is good. Anyhow, it's better not to use spoken language in writing. Which means, I can give green lights on this level that it would make more sense.

Thanks for preparing all the examples. At a first glace, I think that toBe sometimes takes too much place in a word compared to is but I guess we would quickly get used to it. But maybe worth wile comparing also toBeXy with toBe.xY where we could make toBe mandatory so that we don't have two forms. I am not so sure how good this would be though mainly because code completion will suffer. Looking at it visually:

expect(1).toBeLessThan(2)

expect(1).toBe.lessThan(2)

I am not convinced that toBe.lessThan(2) is really better. What do you think? If you want to play around: https://pl.kotl.in/W0i_cqYS2

On the other hand, I am convinced that it would be a nice match for the infix API:

I expect 1 toBe 2
I expect 2 to be lessThan 2 // instead of
I expect 2 toBeLessThan 2

IMO here it would make more sense, but code completion would also suffer here.

---

Where I am really not with you is toContainElementsFound. Here I would prefer toContain. IMO it is only a matter of knowing the convention of shortcuts in Atrium that they imply I expect embedded into the context. In the example it implies to contain an element that I expect...
I agree that it requires pre-knowledge but I think this is fine for shortcuts. You are going to look up what they mean once and then its fine. And since the assertionCreator-lambda is everywhere the same you quickly now by the signature of the function what is meant. IMO they can be shorter than usual. I also think that we should not make the distinction here between passing e: E and the assertion-creator lambda as we might do in the long version because it would clutter the API to much IMO and imagine the names which would result: toContainInAnyOrderOnlyElementsFound -- nah.. that's a bit too much no?

[robstoll]

Last but not least we should also look at the long forms of iterable.contains and define what name we would use for entry and entries -- I will most likely rename value(s) to element(s) regardless if we make this change or not but it depends on this change if I am going with elementsWhich or something different.

after renaming status quo

expect(list).contains.inAnyOrder.atLeast(2).elementWhich { isLessThan(2) }

a few ideas:

expect(list).toContain.inAnyOrder.atLeast(2).elementThatHas { toBeLessThan(2) }
expect(list).toContain.inAnyOrder.atLeast(2).elementWhichHas { toBeLessThan(2) }
expect(list).toContain.inAnyOrder.atLeast(2).elementWhichNeeds { toBeLessThan(2) }
expect(list).toContain.inAnyOrder.atLeast(2).elementThatNeeds { toBeLessThan(2) }
expect(list).toContain.inAnyOrder.atLeast(2).elementIdentifiedBy { toBeLessThan(2) }
expect(list).toContain.inAnyOrder.atLeast(2).elementFoundBy { toBeLessThan(2) }
expect(list).toContain.inAnyOrder.atLeast(2).elementMatching { toBeLessThan(2) }

I am sure you have different ideas

[jGleitz]

I am not convinced that toBe.lessThan(2) is really better

While I think that toBe.lessThan(2) looks more pleasant and is easier to scan, I don’t think that it is worth it. As you say, it will harm code completion. Users would need to invoke the completion twice instead of once, which is not a price worth paying, imo.

On the other hand, I am convinced that it would be a nice match for the infix API

As I have said many times before, I am not a representative user for the infix API. I expect 2 to be lessThan 2 looks beautiful. However, once again, I don’t think that the price of it being more complicated to write in the IDE is worth paying.

Where I am really not with you is toContainElementsFound. Here I would prefer toContain.

I see your arguments. I would personally still prefer something like toContainElementsWhichNeed for the shortcut, but that is certainly not a hill I am willing to die on.

I agree that it requires pre-knowledge but I think this is fine for shortcuts.

I agree.

---

Regarding elementsWhich: I like elementWhichNeeds / elementsWhichNeed a lot. Because a) it forms a fluent sentence, and b) it is a name that tells me what the assertion does. I think it’s a better proposal than my elementFound.

[robstoll]

I mulled the idea over... I start to make friends with it. Just as notice, we are going to resemble more and more jasmine (https://jasmine.github.io/2.0/introduction). There are still a few things I would like to look at and get your opinion before we move to to + infinitive

1. Rename toBe to toEqual.

Since we resemble more and more jasmine, we should consider to rename toBe to toEqual because toBe has the meaning of isSameAs in jasmine.
I like toBe because it is shorter than toEqual but if everything else is almost like in jasmine, it might be better to use toEqual ? Also because the JVM method is named equals it would be more logical to use toEqual. I was always an opponent of using isEqual because there are JVM functions also named this way. And I always deemd isEqualTo to be too long for a function which you use most of the times.
I would not use toBe as jasmine does though. That behavioural change would be too big and confusing. Instead I would use toBeTheSameAs (instead of the current isSameAS). Likewise I would rename isEqual for LocalDateTime and co. to toBeEqual

I am not sure yet if I want to give up my beloved toBe though.

2. feature assertions

expect(list).size.toBe(3)
expect(list).min().toBe(12.3)

IMO using the property/method name directly, as size and min() in the above example is still good and we don't need to change anything here. Do you have a different opinion? Likewise I think we can stick with feature and don't need a different name:

expect(person).feature { f(it::name) }.toStartWith("Ro")

But there are gray zones I would like to discuss. We provide shortcuts (like size above) for other features which start with is or has. I am not sure if we should keep them because they are very familiar to developers but on the other hand it is kind of unintuitive/and inconsistency that they differ from the rest

expect(list).isEmpty()      
expect(list).isNotEmpty()      
expect(list).isNotBlank()
expect(iterable).hasNext()
expect(iterator).hasNotNext()  // dark gray zone but if we keep hasNext, then the logical thing would be to have hasNotNext()
expect(collection).hasSize(1)  // this one I would rename to `toHaveSize` since the method name does not exist on collection
expect { }.toThrow<..>().messageContains() // this one I would rename to `messageToContain`
expect(string).matches(Regex)   // I would rename this to `toMatch` even though matches is the actual method name

Especially for isEmpty, isNotEmpty and isNotBlank I am unsure. Just from the examples I gave I would rename them as well. If we don't then we should probably keep matches as well (and then logically also mismatches).
Personally, I don't like if we have this inconsistency, so I would definitely rename all functions starting with a verb to the to + infinitive equivalent.

3. unreported subject changes

expect(arrayOf(1,2)).asList().size.toBe(3)

IMO as... can stay as they are.

4. because

It is not yet in Atrium but I already consider to rename it. Why? Jasmine is using withContext maybe it would be better to stick to this? Actually, personally I like because better but I figured we should challenge this decision.

[jGleitz]

I mulled the idea over... I start to make friends with it.

So do I! The more consistent naming in the API that I will lay out it point 2 would also be a significant improvement, from my point of view.

1. Rename toBe to toEqual.

Just as notice, we are going to resemble more and more jasmine (https://jasmine.github.io/2.0/introduction)

Arriving at a point where others have arrived before is usually a good sign. However, resembling Jasmine should never be a goal in and of its own, I think. But seeing that something works well for others is always a good argument.

we should consider to rename toBe to toEqual

I am glad that you bring that up! I support this change fully, albeit mainly for the reason of precision. I don’t really care whether we are similar to another framework or not.

I think toEqual is good because it reminds the reader and writer that equals will be used (and that therefore, the semantics of the operator are completely dependent on the implementation). With toBe, it is not clear: To my mind, toBe could indicate either an identity or an equality check.

I would not use toBe as jasmine does though.

Neither would I! For the same reason I have given above: precision. toBe is simply too ambiguous, toBeTheSameAs is precise. We could also consider toBeTheInstance from Hamcrest, I like both.

2. feature assertions

MO using the property/method name directly, as size and min() in the above example is still good and we don't need to change anything here. Do you have a different opinion? Likewise I think we can stick with feature and don't need a different name

I agree. The shortcuts don’t form a fluent sentence, but we will never get them to: It would require genitives, and I don’t thik we want to go there. So agreed.

I am not sure if we should keep [isEmpty, isNotEmpty, isNotBlank, etc] because they are very familiar to developers but on the other hand it is kind of unintuitive/and inconsistency that they differ from the rest.

I strongly believe that they should be renamed and that renaming them would improve the API. I see your point that isEmpty etc. are familiar names for developers. However, I currently find that to be a disadvantage: Because isEmpty is so familiar to me, I automatically assume to have a function that returns a boolean. So without checking the docs, I’d expect isEmpty to change the subject of the Expect to a boolean. But that’s not the case.

So for my mind, the fact that the functions have the same names as the “real” ones is irritating. After renaming everything, we would have a very clear structure:

• If an Expect extension function has the form to + infinitve, it is an assertion.
• Otherwise, an Expect extension function changes the subject.

I think this new consistency would improve the API. This should also answer your other examples.

3. unreported subject changes

IMO as... can stay as they are.

I agree. It also fits nicely in the new naming rules I wrote above.

4. because

I see no reason not to use because. I think it is a nice and fluent way to give the reason for an assertion. As I wrote above, “Jasmine does it” is not a convincing argument to me in and of its own. It would need to be attached with something like “Jasmine does it and it works very well for them because …”.

[robstoll]

Sweet, we agree on all points :)
One note though, all, none and any fall out of our schema. Yet, I was not able to come up with a better solution for now. Do you have an idea?

[robstoll]

Coming back to robstoll/atrium#689 where you asked that none and all shall not check for an element. Maybe it would be better if we don't provide this functionality under all, none and any at all, i.e. leave this to a feature assertion function returning Expect<Boolean>. Then we would be entirely in the schema of "assertion functions start with to..". If this sounds like a good idea, then I would suggest the following:

expect(list).toHaveElementsAndAll { toBeLessThan(2)  }
expect(list).toHaveElementsAndAny { toBe(2) }
expect(list).toHaveElementsAndNone { toBe(2) }

// and their "supplement"

expect(list).toBeEmptyOrAll { toBeLessThan(2)  }
expect(list).toBeEmptyOrNone { toBe(2) }

Thoughts?

[jGleitz]

One note though, all, none and any fall out of our schema.

Do they? To my mind, they are closer to feature extractors than they are to assertions since they don’t actually do the checking but delegate to a real assertion. It’s a matter of perspective, I guess.

Regarding toHaveElementsAndAll toBeEmptyOrAll: My personal opinion is that the semantics of all and none are very clear. So my favourite solution would be to just have all and none. If I want to assert that the collection is not empty, I just write expect(list).notToBeEmpty.all { toBeLessThan(2) }.

However, I already know that you are concerned about developers programming subtle bugs into their tests. I agree that this can happen. Given that you want to help developers avoid such bugs, I think that toHaveElementsAndAll and toBeEmptyOrAll are a better solution than what atrium does currently. I think so because it makes the behaviour explicit and people like me are not surprised by atrium’s behaviour.

Having said that, I’d find the following names more intuitive: notToBeEmptyAndAllElements, notToBeEmptyAndAnyElement, notToBeEmptyAndNoElement, toBeEmptyOrAllElements, toBeEmptyOrNoElement.

These names are even longer, though. I think I feel more strongly about using notToBeEmpty instead of toHaveElements than I feel about using Elements at the end. So if the verbosity bothers us, those would also be fine, albeit a little less precise, from my point of view: notToBeEmptyAndAll, notToBeEmptyAndAny, notToBeEmptyAndNone, toBeEmptyOrAll, toBeEmptyOrNone

[robstoll]

Do they? To my mind, they are closer to feature extractors than they are to assertions since they don’t actually do the checking but delegate to a real assertion. It’s a matter of perspective, I guess.

According to your own statement:

Because isEmpty is so familiar to me, I automatically assume to have a function that returns a boolean. So without checking the docs, I’d expect isEmpty to change the subject of the Expect to a boolean. But that’s not the case.

The same applies to all, none and any, no? 😉

[jGleitz]

The same applies to all, none and any, no? 😉

I somehow never looked at it this way, but you are right. Goes to show how subjective these things can be 🤷

[jGleitz]

Since we decided to do it: What is the timeline and plan for this change? If we coordinate ahead of time, I am happy to help realise it. I suspect it will be quite an endeavour and once started, should be finished quickly to avoid conflicts.

[robstoll]

Nice to hear. Planning is quite hard if you don't know the time you have available. So I cannot tell you when I would do it. Therefore, do you already know when it would be suitable for you?

[jGleitz]

do you already know when it would be suitable for you

There won’t be any good time for at least the next half year, I’ll have to squeeze it in any way. So there’s no particular time constraint from my side.

In general I’d suggest something like:

1. Identify non-breaking, low-hanging fruits that should be done before the change so people can adopt them.
2. Communicate the planned change.
3. Issue a change freeze while we get this ticket through as fast as possible.
4. Publish a breaking release.

[robstoll]

I don't see any breaking change happening on the API-level. Do you see one?

Since you don't have a particular time slot, I suggest we don't stress ourselves and do it as fast as times allow it.

I suggest the following:

• I am going to finish 0.15.0: https://github.com/robstoll/atrium/milestone/12
• I'll announce in the release notes, that we are going to shift to to + infinitive in api-fluent and that we will deprecate the existing functions and remove them with 1.0.0
• I already planned to announce, that we will no longer maintain the deprecated APIs, i.e. I will remove them from the project
• I plan to announce a few more deprecations

You can already start with the rewrite in an own branch. Once 0.15.0 is released, we can merge the branch into master and we will release 0.16.0 as soon as all functions are rewritten for api-fluent. I would not touch api-infix at the moment and wait a bit to see how well it plays out for api-fluent

[jGleitz]

I am on board, but I am a bit worried about

You can already start with the rewrite in an own branch.

Don’t you think this will cause a lot of merge conflicts?

[robstoll]

I doubt it, just don't start with mapAssertions. There we could run into conflicts. In the end I want to move all deprecated functionality to files called deprecated... but let's do this after the merge.

[robstoll]

With rename I meant: duplicate, rename and deprecate the old version. It surely makes sense if you notify me after the first change so that I can give feedback before you continue with further changes.

[jGleitz]

@robstoll Let’s go!

[jGleitz]

@robstoll I just noticed that we never discussed whether this change should affect reporting as well. I gave it some thought and think it should, for two reasons:

1. You have stated multiple times that you like the code to reflect the reporting, and vice versa, as far as possible. As I have said, I don’t find that particularly important, but still, there’s that.
2. More importantly, I sometimes become confused in atrium’s reports regarding whether a line states an expectation or a result. For example, in this block:

◾ an element which: 
   » ▶ key: 
       ◾ equals: "a"        <1234789>

is Atrium telling me that the key was expected to be a or that it was a?

From my point of view, changing the reporting also to the ‘to+infinitive’ style will make the reports easier to read because it will be clear grammatically that lines state expectations:

◾ an element which needs: 
   » ▶ key: 
       ◾ to equal: "a"        <1234789>

I am not sure whether I am the only one having this problem, but I’d find it helpful. Plus, we could probably revert the assertion verb back to ‘expect’.

[robstoll]

Good input. By reverting to expect you mean change expected the subject back to expect right?

For me, both is fine. Your's and some else's argument to change it to expected was that we should report what happened and thus past tense

[jGleitz]

By reverting to expect you mean change expected the subject back to expect right?

Yes, although I am still sympathetic to use ‘expected’ instead of ‘expect’ since we are talking about the past (I miswrote above). But that is certainly not something I will pick fights about.

[robstoll]

Good with me as well, so we will change the reporting from

expected that subject: 10        (kotlin.Int <1234789>)
◆ is less than: 5        (kotlin.Int <1234789>)

to

expected subject: 10        (kotlin.Int <1234789>)
◆ to be less than: 5        (kotlin.Int <1234789>)

Do you agree?

[jGleitz]

Yes.

[robstoll]

@jGleitz so, to + infitive will finally happen in 0.17.0 in the API. I'll start soon with the rewrite of anyAssertions to anyExpectations based on your open PR (I'll close it once I start) and hope you might find time do review it and then maybe rewrite some files as well. One thing which I muled over and I am not sure any more is regarding including articles in the expectation function name. For instance, the rewrite of isDirectory to toBeADirectory (vs. toBeDirectory). The reason why I hesitate or rather why I am currently against to use an article is:

1. I ran into several situations lately where people have trouble to chose a vs. an (that's the decisive factor).
2. as far as I know, Russian does not have articles and I see regularly that Russian speaking people are using this library. So articles might be a barrier for those people.
3. in case of A as in toBeADirectory my brain needs a split second to separate A from D - the problem is not there if it is An. But for A there are combinations which are always "hard" for my brain. For instance toBeASymbolicLink, I always connect AS first and then only realise it is a Sym...
4. last but not least, developers are not used to use articles in function names.

I even re-consider to remove articles from the infix API once we switch to to + infinitive there as well but I can imagine that I keep them there as the separation issue is not the same, e.g. toBe aDirectory is not a problem thought the first two points (mentioned above) remain.

Do you have any counter-arguments why we should include them (next to it would be grammatically correct)?

[jGleitz]

and hope you might find time do review it

I'll be happy to review if it's after April 19th. Otherwise, there will, unfortunately be little chance.

---

Regarding articles:

I ran into several situations lately where people have trouble to chose a vs. an (that's the decisive factor).

I don't get this argument. We would catch a/an errors in PRs just like the myriad of other minor mistakes everybody makes when writing code. Sure, I agree, we will probably see some of these errors, but I don't see why that should pose any problem.

as far as I know, Russian does not have articles and I see regularly that Russian speaking people are using this library. So articles might be a barrier for those people.

I also don't quite understand this argument. I don't think you want to argue that foreign speakers whose mother tongue doesn't have articles will have troubles reading code with articles. So I guess you must be arguing that they will have troubles writing it.

But if that's your argument, this could only be a concern if people are guessing function names. I highly doubt that anybody is doing that. My typical modus operandi is using content completion. Sometimes I read the docs. That's how I find the available functions. Both methods are entirely unaffected by the presence or absence of articles.

So in conclusion, I don't understand how not being used to articles should influence how well one can use a library that uses articles.

my brain needs a split second to separate A from D

I understand this argument. I would judge the effect very minor, but I agree that it's there.

last but not least, developers are not used to use articles in function names

I'd argue that developers are mostly not used to it because most APIs are not fluent. If we followed this logic, we should probably not build a fluent API.

---

Like all developers, I am, of course, quite used to code not being grammatically correct. I usually try to pick names that convey all important information with the shortest amount of letters possible.

Once I use a fluent API, however, I switch modes and try to build correct sentences. I read the code a lot more like a book. Now imagine you'd read a book in English (or an language with articles, for that matter) and all of the articles were missing. It would just feel ... wrong. Sure, you'll get all of the meaning, articles are usually redundant after all. A lot of languages manage perfectly fine without them. But the whole time it would just feel wrong. It would make you uncomfortable.

That's my issue with the articles in a fluent API. I am in ‘book reading mode’, thus, my brain expects articles and is always a little irritated if they are missing.

Sure, that is a subjective, aesthetic argument. However, as I see it, there is no objective, and surely not non-aesthetic argument against articles. The best way to decide would probably be a study on a representative sample of users. Without having that, we (that is, ultimately you) will have to make the decision. I am in favour of articles.

[robstoll]

We would catch a/an errors in PRs just like the myriad of other minor mistakes everybody makes

I totally agree but that's not my concern. My concern is in using Atrium, i.e. writing expectations in tests. I for one write many times faster than Intellij is able to suggest things. Which means, if I had trouble knowing if its ADirectory or AnDirectory then I would be slowed down by Atrium because I would need to wait for Intellij for the suggestion. What I saw lately (which raised this concern) is that people which have trouble to know if it's a or an don't suddenly get the distinction. They repeatedly wrote it wrong even though they were corrected many times. What I don't know, maybe they have some sort of dyslexia... but it made me think that the improved readability might be outweighed by the decreased writeability.

The best way to decide would probably be a study on a representative sample of users.

I agree, the more opinions we get of people using Atrium the better. I have started a poll in the atrium channel, I don't expect a lot of responses as there is normally not much going on in the channel but it's worth a try.

[jGleitz]

They repeatedly wrote it wrong even though they were corrected many times.

This is an unexpected result for me. I interpret it such that they actually form a sentence in their mind, think of how it’s written in English and then type that out. I almost never do that. I have two ways major ways of writing identifiers:

• For functions I use often, I just know by heart how exactly they are called. I also know how many letters I have to type until I can hit Ctrl+Space+Enter and IntelliJ will do the rest for me.
• For functions I do not use that often, I search by keyword. In your example, I’d write ‘directory’, look at IntelliJ’s suggestions and pick toBeADirectory.

---

Please note that the argument is two-edged: If there are people that form sentences and write those out, then there will also people who think like me that will be harmed if the articles are missing. That is, if we accept the argument that having articles irritates some when using functions (because they are irritated by how articles work in English), we must also accept the argument that not having articles irritates others (because they expect the articles to be there).

The only way I can see this argument working is if we knew that one group greatly outnumbers the other group.

[robstoll]

I guess leaving out articles during writing will most likely not irritate someone who is expecting articles during reading, just because it's so common to leave them out during writing. But that's the same for people which do not expect an article during writing, it will surely not be a problem for them to read them (just the minor A+Capital letter problem which one probably gets used to over time).

I totally get that missing articles are an annoyance during reading and I share this view. So I'll wait a bit and gather answers. So far, there is 1 vote more in favour of articles than for without articles. Maybe you can do the same, if you know someone which would potentially use Atrium, ask them what they would prefer.

[jGleitz]

So far, there is 1 vote more in favour of articles than for without articles.

Please note that this is my vote!

[robstoll]

Thanks for your hint, I know. I did not mean the votes in the Atrium Channel, the votes of people I asked directly.

[jGleitz]

Ah, I see. I just wanted to make the vote fair, since I expect a pretty small sample size.

[robstoll]

@jGleitz another topic but related, renaming isA. toBeA would be straight forward but reads odd in some cases as well, e.g. toBeA<Entry> (toBeAn<Entry> would read nicer IMO). What is your preference:

• toBeA
• toBeA + toBeAn and let the developer choose
• toBeAnInstanceOf
• other ideas?

[robstoll]

@jGleitz I made up my mind regarding articles, we will include them as initially thought.

[jGleitz]

Regarding toBeA:

I would exclude the option

toBeA + toBeAn and let the developer choose

I have used libraries that offer synonyms and it usually ends in chaos. For example, imagine that Entry is renamed to Record. Theoretically, you would need to change every toBeAn to toBeA, just for aesthetics.

I, personally, like toBeAnInstanceOf best. Mainly because it is more explicit. But I would also be okay with toBeA. The fact that sometimes, an ‘n’ would be required, can be tolerated, from my point of view.

[CfGit12]

Very late to the discussion here but having raised my issue against the main project it's nice to see essentially exactly what I wanted is going ahead :)

A couple of votes to later discussion points:

Prefer toBeAnInstanceOf over toBeA/An. Similarly prefer toBeEqualTo over toBe. Both make their use more explicit.

No API functions should end in A/An to avoid the grammar problems it creates. But their use inside a function name is fine - e.g. toBeADirectory over toBeDirectory.

And whilst it's not my preference dropping articles altogether is also fine.

[CfGit12]

Also would drop assert and assertThat if expect is preferred (which due to the typing in the API I would assume it is). Have a single consistent experience and a cleaner API.

[robstoll]

Good, I'll modify the PR robstoll/atrium#866 and change toBeA to toBeAnInstanceOf 👍

@CfGit12 one thing to note, I'll go with toEqual instead of toBeEqual because the underlying method name is equals and not isEqual. Following the same logic, I will rename Expect<LocalDateTime>.isEqual (and co). to toBeEqual because the underlying method is named isEqual

---

Also would drop assert and assertThat if expect is preferred (which due to the typing in the API I would assume it is). Have a single consistent experience and a cleaner API.

@jGleitz I would definitely drop assertThat as well and I think dropping assert makes sense as well since we want to move away from the assertion wording. What do you think?

[jGleitz]

I'll go with toEqual instead of toBeEqual because the underlying method name is equals and not isEqual.

That makes a lot of sense. It is also better grammatically: expect(x).toEqual(y) is correct, while expect(x).toBeEqual(y) is not (should be expect(x).toBeEqualTo(y)).

Following the same logic, I will rename Expect<LocalDateTime>.isEqual (and co). to toBeEqual because the underlying method is named isEqual

I wonder whether this is the best approach. toEqual and toBeEqual will both be defined for LocalDateTime and it is not clear at all from the names what the difference is. Sure, the Jave API developers are to blame for naming a method isEqual when there already is equals. Nevertheless, we should consider whether clarity when reading tests might be more important than consistency with production code. I tend to answer ‘yes’. And then we should name the method something like toBeTheSamePointInTimeAs.

I would definitely drop assertThat as well and I think dropping assert makes sense as well since we want to move away from the assertion wording. What do you think?

I agree completely.

[robstoll]

And then we should name the method something like toBeTheSamePointInTimeAs.

Good idea 👍

[CfGit12]

I did suggest toBeEqualTo rather than toBeEqual but also don't think I have any issues with toEqual.

I would however caution against trying to match underlying production APIs too much. Whether or not the underlying function is equals or isEqual should IMO have no bearing on the test API. A consistent toBeEqualTo or toEqual is all I need. I agree with @jGleitz in this regard.

[robstoll]

@jGleitz / @CfGit12 I am almost through with the transition 🎉
The last thing to change is resultAssertion (and probably some additional clean-up).
Here I am not yet sure how I should rewrite it and would like to know what your preference is. Result provides two methods isSuccess and isFailure and we currently use the same names. Following some ideas:

• expect(result).isSuccess() => toBeSuccess / toBeASuccess / toBeSuccessful / toSucceed / notToBeAFailure, notToHaveFailed
• expect(result).isFailure<Exception>() => toBeFailure<Exception> / toBeAFailure<Exception> / toFailWith<Exception> / toHaveFailedWith<Exception>

As additional information, there is only the inner class Result.Failure but not Result.Success and Result.Failure is defined internal in Kotlin.

[CfGit12]

My initial thoughts would be:

toBeSuccess
toBeFailure
toBeFailureOfType<Exception>

Worth splitting out the two failure cases. There'll be times when you do and don't care about the type of failure coming out.

[jGleitz]

I am almost through with the transition 🎉

Great! This is a big improvement for the API, from my point of view! (also: sorry for the long silence).

Regarding the Result functions (sorry if this is already merged & done, I am still following up on everything I missed):

I think it should be toBeASuccess / toBeAFailure / toBeAFailureOfType<>. This would be the option that is most consistent with the rest of the API. For example, we also have toBeADirectory, and not just toBeDirectory.

[robstoll]

@jGleitz I lately don't have much more time than reviewing things. I hope I will come around to implement the last bits during this summer. I have not yet created the change for Result. I am also in favour of toBeASuccess for basically the same reasons as you stated.
Yet, I don't intend to introduce two functions for the Failure case but stick to one which expects a type parameter. If one does not care which exception occurred then one can still write toBeAFailureOfType<Throwable>. I prefer to have a cleaner API than an additional shortcut which will not be used that often (and I don't want to encourage people writing assertions which are not specific enough so that potential bugs are hidden).

I am not yet sure if we should use OfType or something else like Wrapping / WithType -- because the Failure is not of type xy looking at it through type theory glasses but is a wrapper and the wrapped throwable is of type xy. Thoughts?

[jGleitz]

Yet, I don't intend to introduce two functions for the Failure case

Makes sense from my point of view.

because the Failure is not of type xy

I think this is a good argument against toBeAFailureOfType<xy>.

Thoughts?

I’d be fine with just having toBeAFailure<NoSuchElementException>. Yes, it is not 100% explicit because the name does not indicate the type check. But on the other hand, I can’t see any way how this could mislead anybody. And this is the shortest meaningful name.

Another reason why toBeAFailure makes sense: Success and Failure are very similar. The first is a wrapper for some data, the latter is a wrapper for some error. toBeASuccess checks that the subject is a Success and then changes the subject to the data. Hence, it makes sense to my mind that the function that checks that the subject is a Failure and then changes the subject to the error is calelled ‘toBeAFailure. The only difference is that we already know the type for toBeASuccess, but have to provide it for toBeAFailure`.

---

Should the above not convince you, and you want to make the type check somewhat explicit in the name, I’d suggest toBeAFailureOf. It reads well enough, but is shorter: toBeAFailureOf<NoSuchElementException>. of is also consistent with the typical name of the creation function for Monads in Java, like Optional.of.

[robstoll]

I am totally fine with toBeAFailure. We currently have isFailure and I never had the feeling it should include an Of or similar, so I am glad you see it the same way

[robstoll]

expectation functions have been renamed in 0.17.0 open are the message in error reporting

[robstoll]

messages in error reporting were adopted with 0.18.0, closing this