Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change Wording from ‘Assertion’ to ‘Expectation’ #746

Open
jGleitz opened this issue Dec 23, 2020 · 26 comments
Open

Change Wording from ‘Assertion’ to ‘Expectation’ #746

jGleitz opened this issue Dec 23, 2020 · 26 comments
Assignees
Milestone

Comments

@jGleitz
Copy link
Collaborator

jGleitz commented Dec 23, 2020

In #714, we found that it makes sense to talk about ‘expectations’ instead of ‘assertions’. Hence, we want to refactor the whole code base to reflect that naming.

@jGleitz
Copy link
Collaborator Author

jGleitz commented Dec 23, 2020

@robstoll: I’d also like to change the wording from ‘feature assertion’ to ‘feature extractor’ because they don’t really assert (or expect) anything, they only change the subject.

This would also mean that it might make sense to separate feature extractors from expectations. If I remember correctly, we had that in the past but removed the split. From a naming standpoint, it would make sense though. What do you think?

@robstoll
Copy link
Owner

I am not sure where you want to change the wording. In the readme? the file name in the API?

We actually have already a FeatureExtractor on the logic level, it's the implementation behind feature assertions in the end.

So far I made the following distinction between the two (KDoc of FeatureExtractor)

/**
 * Defines the contract for sophisticated `safe feature extractions` including assertion creation for the feature.
 *
 * It is similar to [FeatureAssertions] but differs in the intended usage.
 * [FeatureAssertions] are intended to make assertions about a return value of a method call or a property,
 * assuming that the call as such always succeeds (no exception is thrown).
 * The [FeatureExtractor] on the other hand should be used if it is already known,
 * that the call/access fails depending on given arguments.
 * For instance, [List.get] is a good example where it fails if the given index is out of bounds.
 */

@jGleitz
Copy link
Collaborator Author

jGleitz commented Dec 23, 2020

I am not sure where you want to change the wording. In the readme? the file name in the API?

I am mainly concerned about how we communicate, so I’d mainly change the readme. However, I thought it might make sense to reflect that distinction also in the API and the logic files. That could also make it easier to find certain functions, since so files are quite big.

We actually have already a FeatureExtractor on the logic level

I know :). However, I hadn’t read that piece of KDoc yet.

I don’t find the distinction you make in the KDoc all too convincing. I think the distinction should be about intend. Let’s take List.get: True, using it is also some kind of expectation, however, that should not be the intend. If I want to make sure that there are three elements, I should write something like expect(list).size.toBeGreaterThan(2). I’d, personally, consider it bad style to only use get(2) if there being three elements is an important expectation for my test.

From my point of view, there are no ‘feature expectations’, but only ‘feature extractors’ and ‘expectations’. Something like Throwable.messageToContain is only a shortcut for a feature extractor plus an expectation. This also ties in nicely with the naming convention we found in robstoll/atrium-roadmap#93. Expectations have the form ‘to+infinitive’, feature extractors do not and are most of the time called like their corresponding method/value on the subject type.

As I said, I am mainly concerned about the wording we use in the documentation. Consistent wording that builds a clear mental model always helps me a lot when reading docs and getting to know projects.

Whether we split the files is another question. I am in favour of it, but much less concerned about it than I am about the docs.

@robstoll
Copy link
Owner

Those are good points and make sense. So I am in favour of renaming featureAssertions.kt in the APIs to featureExtractors.kt and we can also name it that way in the README.

Also I agree that the distinction is no longer really up-to-date as we now also catch exceptions caused in feature { } since 0.15.0 and report it. So I am in favour of merging the FeatureAssertions and FeatureExtractor on the logic level. I would still name it FeatureExtractor and not FeatureExtractors in contrast to featureExtractors.kt on the API level.

However, I think you take an end-user perspective on the logic level where the logic level targets expectation function writers. Thus, I would still include in the KDoc of property, f0 and co. + manualFeature that they should not be used if the extraction is not safe and instead use extract

@jGleitz
Copy link
Collaborator Author

jGleitz commented Dec 23, 2020

So I am in favour of renaming featureAssertions.kt in the APIs to featureExtractors.kt and we can also name it that way in the README.

So I am in favour of merging the FeatureAssertions and FeatureExtractor on the logic level. I would still name it FeatureExtractor and not FeatureExtractors in contrast to featureExtractors.kt on the API level.

Great 👍

However, I think you take an end-user perspective on the logic level where the logic level targets expectation function writers. Thus, I would still include in the KDoc of property, f0 and co. + manualFeature that they should not be used if the extraction is not safe and instead use extract

I am not sure what you are hinting at with this paragraph. I did not want to imply changing any KDoc. I absolutely see that distinguishing safe from unsafe extractions on the logic level makes sense. As you say, I was only concerned about the user’s perspective.

Last question: Should we separate expectations from feature extractions in the logic and API files? Should we, for example, have pathExpectations.kt (containing stuff like toExist) and pathFeatureExtractors.kt (containing stuff like `fileName)? On first look, it seems that this would no even lead to a lot more files, since most files contain only expectations or only feature extractors.

@robstoll
Copy link
Owner

I am not sure what you are hinting at with this paragraph, I did not want to imply changing any KDoc

That on this level we should first serve the expectation function writers and then end-users.
I actually understood that you would want to remove the distinction, i.e. remove the extract or at least the text in the KDoc. So, a misunderstanding on my part. Thanks for your clarification 👍

Should we separate expectations from feature extractions in the logic and API files?

I would not, we had and many times still have the split in the specs and so far it was more confusing than helpful.

@robstoll robstoll added this to the 0.16.0 milestone Dec 23, 2020
@jGleitz
Copy link
Collaborator Author

jGleitz commented Dec 23, 2020

Fair enough.

@jGleitz
Copy link
Collaborator Author

jGleitz commented Jan 10, 2021

@robstoll how will we proceed here? My current understanding is that you approve the change in principle, but don’t want it to affect the code too much, which leaves us in a weird half-baked situation, from my point of view.

  1. Why don’t we rename the API files? My understanding is that this will be a source compatible change, which should be fine, right?
  2. In 🚧 Rename ‘Assertions’ to ‘Expectations’ #748 you mentioned that you want to leave the ‘Assertion’ class, and all related classes, named ‘Assertion’. Why is that?

Regarding 2, I don’t think that it would be wise to talk about both ‘Assertions’ and ‘Expectations’ in Atrium. Because:

  • I think ‘Assertion’ is not a fitting name at all. I am not sure why it is so common in the Java world when talking about tests. Maybe it is because of the assert keyword and the corresponding AssertionError. In production code, it makes sense to use assert and the term assertion, because you use it to assert a certain property of the code. In test code, I don’t think it makes much sense. Test code checks or makes expectations, it does not assert.
  • I think in 🚧 Rename ‘Assertions’ to ‘Expectations’ #748 you imagined ‘assertion’ to be the domain & logic term, and ‘expectation’ to be the API term. However, I think can not be separated clearly. It starts with assertionCreator lambdas: They create Assertions, so calling them anything else, would not make sense. But then again, the user would provide an assertionCreator lambda to make expectations – that’s also odd.

All in all, I proposed this change because I think it would make it easier to talk about Atrium’s concept and to write clear documentation. I still think that pulling it through would improve the understandability of this project. I also think that now is a good point in time, once Atrium hits 1.0.0, such a change is much worse to pull off.

However, if we are going to mix terms throughout the code base and documentation, I think we are worse off than if we went just with ‘Assertion’. So I think we should do the change, but do it all the way.

What are your thoughts @robstoll ?

@robstoll
Copy link
Owner

robstoll commented Jan 10, 2021

  1. We will. I first figured that renaming the files now makes sense and we can later on move the deprecated stuff out of the files into a file called xyExpectationsDeprecated. But that's work which is unnecessary as it is also perfectly fine to have the old deprecated functions in xyAssertions.kt and the new in xyExpectations.kt

  2. because we will talk about expectation functions in the README and on the API level. To me, they sound like they are creating expectations. Which they do but not in a technical term in the sense of, if we rename Assertion to Expectation, then they don't create Expectations only, they have much more side effects (also depending on the context) -- a bit further outlined below when I talk about assertionCreator-lambda

But... I agree that Assertion can be quickly mixed up with assert from production and that this is not an ideal term. Maybe we can come up with something else which also has less overlapping with expectation? I am open for a new term and I agree that now would be the perfect timing.

It starts with assertionCreator lambdas: They create Assertions, so calling them anything else, would not make sense.

They create Assertions but also add them to the Expect. And this can cause that an exception is thrown depending on the Expect to which they add the Assertion. A lot of side effects. I would like to distinguish this from the mere creation of Assertions.

Which also means, I would still rename it to expectationCreator-lambda


  • I still think talking about expectations on the API level (and hence in README and most of the docs) makes a bit more sense than about assertions, especially because we use now the verb expect and the interface is named Expect.
  • I am also happy to rename Assertions (assertionBuilder etc.) to something else. Right now I don't have a better fitting name in my head but I will think about it

@robstoll
Copy link
Owner

Maybe it makes sense to look at it from a reporting perspective. We wanted to introduce Reportable with subtypes: Text, Translatable, something which replaces ExplanatoryAssertionGroup and Assertion/AssertionGroup.

@jGleitz
Copy link
Collaborator Author

jGleitz commented Jan 11, 2021

Okay, I understand from what you write that you don’t insist on the term ‘Assertion’, but rather want to differentiate between the logic object and the general term ‘expectation’. I’ll write my interpretation of the terms, maybe that convinces you.

First, the functions we offer through the API are currently called ‘assertion functions’. Going forward, I’d just call them ‘expectation’, not ‘expectation function’. It makes perfect sense to say that toBe(4) is an expectation. In this context ‘expectation’ is used in its broad meaning that it has in the everyday English language.

If I understood you correctly, you hesitate to call Assertion Expectation because an expectation (lowercase ‘e’) on the API level includes more (side effects, etc.) than an Expectation on the logic level. This is correct. However, I don’t think that this is an issue at all, because this is also a very common case in programming: There is the use case on the API level, and there is an object representing that use case on the logic level. And of course, the object encompasses much less than the use case.

In my experience, naming the logic object according to the use case is helpful, not irritating. The use case and logic object can be clearly separated: the former is an ‘expectation’, the latter an Expectation. On the other hand, it is helpful, especially for newcomers, to use a name that resembles the use case, because you can just go ‘ah, this is called Expectation, this will most likely be the thing that contains the actual check’.


We wanted to introduce Reportable with subtypes: Text, Translatable, something which replaces ExplanatoryAssertionGroup and Assertion/AssertionGroup.

Yes, I am aware. And I still think we should do it, it will improve the API! But I am not sure how this is related to this discussion.

@robstoll
Copy link
Owner

robstoll commented Jan 11, 2021

I understand from what you write that you don’t insist on the term ‘Assertion’,

Exactly, I don't insist on the term. I am basically with you that Assertion can be mixed-up with assert and I am happy to change this. IMO Assertion is better if we talk only about language because for me, an Assertion is stronger than an Expectation but maybe that's a personal preferences. In the context of Atrium, I still think Expectation fits better because we have expect as verb.

Going forward, I’d just call them ‘expectation’, not ‘expectation function’.

I don't see yet how this will work out. In the README we will have sections like Your first expectation and then Write your own expectations. I think we need expectation function for the distinction. But I don't see a problem with using expectation function when we only talk about the function as such. I am with you that we can use the term expectation for an expectation function call, e.g. for toEqual(4).

However, I don’t think that this is an issue at all, because this is also a very common case in programming

I don't think that it is a real problem, we had assertion function and Assertion before and no one complained. It's more a subtlety which I would like to improve because it does not really ad up in the end to my mind and now is the perfect time to change something like that. As already outlined, if we have an expectation function, it should create expectations. If we call Assertion, Expectation, then why does the expecationBuilder create a different kind of expectation than the expectation function. It's not terrible but it can be improved IMO.

But I am not sure how this is related to this discussion.

Was just meant as input, maybe you had a good idea looking at Assertion from the reporting perspective to come up with something different. I thought about it a bit and I think my gut feeling was not wrong. Looking at it from a reporting perspective might make more sense. I try to outline two possibilities.

An AssertionGroup is currently: a description + representation, a list of Assertions + the information how they should be displayed in reporting. We can see from this that an AssertionGroup is more than just a grouping and it is related to reporting. A better fitting name could be ReportGroup and an Assertion would be a Report instead. The AssertionBuilder would become the ReportBuilder. We have different kind of reports, like a RepresentationOnlyReport, a DescriptiveReport (I don't like the name as it is too general so maybe we can come up with a better name as well, maybe simply DescriptionRepresentationReport). A ReportGroup will in the end be reported by a Reporter to the user.
AnyAssertions on the logic level would become AnyReports (or maybe better AnyReportFactory?).

Although I find report quite fitting because we are using it for reporting, the term might be too loaded by devs with the idea that a Report already needs to be everything and not a single unit. And we would again overload the term once we produce a HTML report next to the terminal report. Therefore, I am considering to use the term Record instead. At first glance I like it because it emphasises that we take a snapshot in time and create a record from it. The user has an expectation, we record the actual state and a Reporter in the end reports the aggregated records to the user to let him know about the outcome.

Following the three possibilities (with status quo) next to each other (I also made a few further adjustments)

Current Expectation Report Record
toEqual(4) = assertion expectation expectation expectation
toEqual = assertion function expectation function expectation function expectation function
contains.atLeast(…) = sophisticated assertion builder sophisticated expectation builder sophisticated expectation builder sophisticated expectation builder
assertion verb verb verb verb
anyAssertions.kt (on API level) anyExpectations.kt anyExpectations.kt anyExpectations.kt
AnyAssertions (on logic level) AnyExpectations AnyReportFactory AnyRecordFactory
Assertion Expectation Report Record
AssertionGroup ExpectationGroup ReportGroup RecordGroup
AssertionGroupType ExpectationGroupType ReportGroupType RecordGroupType
AssertionBuilder ExpectationBuilder ReportBuilder RecordBuilder
AssertionCollector ExpectationCollector ReportCollector RecordCollector
AssertionContainer ExpectationContainer ReportContainer RecordContainer
AssertionContainer.addAssertion ExpectationContainer.appendExpectation ReportContainer.appendReport RecordContainer.appendRecord
AssertionContainer.createAndAddAssertion ExpectationContainer.createAndAppendExpectation ReportContainer.createAndAppendReport RecordContainer.createAndAppendRecord
AssertionContainer.addAssertionsCreatedBy ExpectationContainer.appendExpectationsCreatedBy ReportContainer.appendExpectationsCreatedBy RecordContainer.appendExpectationsCreatedBy
AssertionFormatter ExpectationFormatter ReportFormatter RecordFormatter
AssertionFormatterFacade can most likely be removed can most likely be removed can most likely be removed
TextListAssertionGroupFormatter TextListExpectationGroupFormatter TextListReportGroupFormatter TextListRecordGroupFormatter
HtmlListAssertionGroupFormatter (does not yet exist) HtmlListExpectationGroupFormatter HtmlListExpectationGroupFormatter HtmlListRecordGroupFormatter
DescriptionAnyAssertion DescriptionOfAnyExpectations DescriptionOfAnyReports DescriptionOfAnyRecords

@robstoll
Copy link
Owner

Oho... I figured I am going to start rewording assertion to expectation in the readme. The following would be bad I think:
Atrium is an open-source multiplatform expectation library for Kotlin with support for JVM, JS and Android. The term assertion library is so common whereas expectation library is not known at all 🤔
I guess I will keep assertion library but replace assertion with exception in other places

@jGleitz
Copy link
Collaborator Author

jGleitz commented Jan 14, 2021

I’ll answer your comment above at a later point because I want to give it careful consideration.

Regarding your latest comment: I don’t think that anybody will have trouble understanding the term ‘expectation library’. If you are worried about SEO, maybe having ‘assertion library’ in the tags is enough? I don’t have experience with that, though.

@robstoll
Copy link
Owner

I have started rewriting README and other md files from assertion to expectation (and feature assertion to feature extractor):
https://github.com/robstoll/atrium/blob/reword-assertion-to-expectation-in-md/README.md

@jGleitz
Copy link
Collaborator Author

jGleitz commented Jan 16, 2021

Sorry for the long silence.


I think we need expectation function for the distinction. But I don't see a problem with using expectation function when we only talk about the function as such.

I agree: I was too rigorous in my previous statement. I think the following would be a good policy: Talk about ‘expectations’ per default, and only use the term ‘expectation function’ when it is relevant that we are talking about the actual function.


It's more a subtlety which I would like to improve because it does not really ad up in the end to my mind.

Okay. So let’s try to find something that makes better sense to your mind. However, I would be glad if it was not Assertion, simply because how overloaded that term is from all its different uses throughout programming.


Regarding the name Report: I agree that this is not a good choice. Just like the ‘other developers’ you mention, I would expect the report to be the whole thing. This is why I suggested Reportable: A Reportable is something that can be reported, hence a part of a report, and not the report itself.

However, I think that Reportable should be the name of the yet-to-be-created superinterface of everything that can be reported. This still leaves the question of how the Assertion (the thing that has the holds() function) should be called. The same line of argument can be applied for the name Record, although I think that Record is an altogether inferior term.

If we go back to the basics, what an Assertion does is check a certain condition. So maybe we should just call it Check, and its function passed()?

@robstoll
Copy link
Owner

robstoll commented Jan 16, 2021

Sorry for the long silence.

No problem at all

However, I think that Reportable should be the name of the yet-to-be-created superinterface of everything that can be reported.

I am still with you on this one and I think I am going to add it in 0.16.0 if we do such a bigger renaming.

If we go back to the basics, what an Assertion does is check a certain condition. So maybe we should just call it Check, and its function passed()?

I thought about your suggestion and I prefer Record. IMO Check has more or less the same weakness as Assertion. The check as such is just one part of the Assertion the other parts are related to reporting. Also, Kotlin has a prominent function check. So checkBuilder could quickly be confused with building a Check which will throw an IllegalStateException if it fails.

I guess Java will in the end get Record types and thus Record might be confused with it as well but I am way less concerned about this one. Also, record is used in the context of databases where it corresponds more or less to a row of a table. Likewise Record in Atrium is one data record of something bigger (the report). So for now, I would go with Record. Do you have a strong opinion against Record?

That being said, I am open for alternatives to Record. Preferably a term which also accounts for the reporting aspect. Following a few but I find them all inferior to Record (but maybe they inspire you to come up with something better):

  • ReportEntry
  • LogEntry
  • ExpectationUnit

@jGleitz
Copy link
Collaborator Author

jGleitz commented Jan 17, 2021

Do you have a strong opinion against Record?

Unfortunately, yes. Mainly because I do not agree with the statement

Preferably a term which also accounts for the reporting aspect.

Consider this: I do fully agree with you that the reporting aspect is important. No disagreement here. However, the reporting aspect will be conveyed by the fact that Check/Assertion/… will implement Reportable. If we wanted to, we could also call it ReportableCheck, however, I don’t think that that is necessary. Implementing Reportable should be sufficient to convey this important aspect, from my point of view.

My main objection against Record is that it does not in the least convey the checking functionality (am I missing something here?). In your current proposal, Record would extend Reportable. I, as a developer, would directly ask the question: ‘What is the difference between a Record and a Reportable?’. Now the answer would be: ‘A Record, unlike a Reportable, has a passes function’. I guess my response to that would be a profound ‘Come again?’.

In other words: The interesting thing about Assertion, when compared to its parent interface Reportable, is that it checks things. The name should convey that.


Check has more or less the same weakness as Assertion.

How so? My main objection against Assertion is that it is wrong when you think about it. In tests, we are not ‘asserting’ something, to the contrary, we are ‘checking’ or ‘verifying’ or ‘testing’ something. In that sense, Check is better.

As I said above: If you insist on having the reporting aspect in the name, ReportableCheck would convey both important aspects: the checking and the reporting.


Also, Kotlin has a prominent function check. So checkBuilder could quickly be confused with building a Check which will throw an IllegalStateException if it fails.

I guess Java will in the end get Record types and thus Record might be confused with it

I am not convinced by either of those arguments. The English language just has so many words, and one cannot stop using some of them just because they have already been used by another prominent library. I think developers are very used to the fact that the same term refers to different things in different libraries.


After my arguments above, it should not surprise you that the only one of your proposal that I can agree with is ExpectationUnit.

In fact, I think we should give ExpectationUnit serious considerations. It has quite some things going for it:

  • The name conveys the direct relation to the term ‘expectation’ that we use in the documentation
  • The name conveys the fact that it checks something
  • The name conveys that it is the smallest part from which all other elements will be built
  • Groups will be called ExpectationGroup (or ReportableGroup, depending on what their contents can be), which is short and also a speaking name

@robstoll
Copy link
Owner

robstoll commented Jan 17, 2021

However, the reporting aspect will be conveyed by the fact that Check/Assertion/… will implement Reportable

That's something you need to look up. Ideally you don't need to and the name tells you enough. But yeah... it's quite often not easy to come up with good names...

The English language just has so many words, and one cannot stop using some of them just because they have already been used by another prominent library.

I agree that this is certainly not a killer-argument but something to consider nonetheless. I also meant it as argument on top of why I would prefer to use something different than Expecation or Assertion on the logic level. But I see that it was not clear at all why I don't like Expecation or Assertion on the logic level and I think I see it now clearer since you used verification, so thanks for that 🙂 I hope I will be able to explain it better now...

My main objection against Record is that it does not in the least convey the checking functionality (am I missing something here?).

I think that's kind of what I like about Record. The fact that it does not imply that it contains itself the checking part. I do think though, that it conveys verification functionality. Maybe for you, checking and verification is the same but I connect the checking part with throwing the AssertionError in the end. Same same for Expectation and Assertion. If we take google's definition for record (https://www.google.com/search?q=record+definition)

a thing constituting a piece of evidence about the past, especially an account kept in writing or some other permanent form.

That's what I think about when I read Record in the context of Atrium. For me, Record is only data which happens to provide a holds/passed function which is kind of just a summary of the Record. If you call an assertion function on the logic level, then this function also just creates data, it's not up to the function to decide what happens with it. It records what the verification has revealed and to a certain degree how it should be reported. It can include extra information such as failure hints, reasons why the verification was done, additional debug information etc. But that's it...

If we take the analogy of a car check. Overall, you expect that the car is still allowed to be driven on street, you will have different sub expectation like the break works, steering works etc. (for me this is when you write expect(...)). Then the verification as such is happening (execution of the code) where the car mechanic will verify one of the expectation -- maybe even different sub-aspects for it -- and write down a record with the actual number(s) / result. If you imagine that different mechanic (record functions on the logic-level) verify different things, then you get multiple records which are most likely combined in the end to one report with a lot of detail and maybe one overall result. Now it is up to the reader (the function caller) of this report what he does with it. Here I leave the analogy, and talk only in terms of code. Currently there are more or less two cases: the function caller uses the record as part of another record or it appends it to a RecordContainer where the RecordContainer then decides what to do (throw an AssertionError, do nothing in addition, just collect and remember it, pass it on to another RecordContainer etc.).

I am not sure if I can convey my point with this analogy, but now that I have wrote it down, I keep it.
AnyAssertions and co. on the logic level mainly gather data and thus should have a name which convey this and should be less focused on things related to expectation / checking which to my mind are connected with fail the test if the criteria are not met.

Does this change your mind or do you have an even better name?

@jGleitz
Copy link
Collaborator Author

jGleitz commented Jan 18, 2021

I love discussions about naming. When people are willing to talk it through (like you are), these discussions can reveal a lot about how people think about the software and its requirements. I am glad that you don’t think ‘it’s just a name’, but are willing to discuss everything to its conclusion. I am learning a lot here!

I’ll proceed as follows:

  1. Describe what I understood from your comment
  2. Give more arguments against Record
  3. Suggest another name
  4. Give miscellaneous remarks

There are different aspects in the description and the analogy you provided. I’ll report what I have understood below. Please correct me if I am wrong or anything is missing. For the sake of simplicity, I will refer to the type we are searching a name for (the one that holds a boolean result and extends Reportable) as TypeX. I don’t want to use any of the proposed names because I think this would add bias to the discussion.

1. You want to convey that TypeX holds the result of a computation and does not represent the computation itself.

I am not sure whether or not I agree that this distinction is important. However, if the author of the library says that the distinction is important to him, I will just assume that it is. My takeaway is: We need a name that conveys this distinction.

2. Some of the terms ‘verifying’, ‘checking’, ‘testing’ are related to throwing an exception for you

Here I strongly disagree: To me, ‘checking’, or ‘testing’, or ‘verifying’ all is ‘the act of looking at the state of affairs and seeing if an expected property holds’. Throwing the exception is merely one way to report the result.

Let me support my point of view:

Let’s go back to your car check analogy. Different mechanics have looked at the car and wrote down for every expected property whether it is satisfied or not. In Atrium, the result of these actions would be stored in TypeX. Throwing the exception is akin to telling the car owner: ‘Your car is not road safe, you must repair it before driving it’. Let’s look at your statement: ‘things related to expectation / checking which to my mind are connected with fail the test if the criteria are not met.’ and take it at face value. If your statement was correct, then if we simply forget to throw an exception (or forget to tell the car owner that they must not drive), then we would not have checked. To me, this is absurd. Of course have we checked! We checked everything and verified all the expectations. The only thing that was missing is telling the car owner / telling the user. Because of this, I think throwing an exception is a matter of reporting, not a matter of verifying/checking.

I think that having the language clear here is important. Such wording will definitely sneak into the documentation. And I suspect that most users will share my view–but then again, I am biased in this assessment 😉.

What I will concede, though, is that after all the simple checks have happened, there will be another check: Whether all the sub-checks have passed. This is, of course, a check in and of itself. But I would strongly disagree to use different terms for ‘checking a property’ and ‘checking whether multiple other checks have passed’. Those two things are the same. A check is a composite.


Having written the above, I still don’t think Record does the trick. The killer argument for me is this: Record and Reportable are nearly interchangeable.

I stated this before, and I am honestly surprised that it does not bother you (please do not take offence! I am just genuinely surprised!). Record and Reportable are so similar that for me, the obvious question will be: What’s the difference? And the answer ‘Record holds the result of a check’ will surely not satisfy me.

The problem is that Record conveys that is some record of some data, but not the kind of data was recorded. All Reportables hold some kind of descriptive data. The important thing about TypeX is that it additionally holds the result of an expectation. We are writing an expectation library, so TypeX is the central data class: It stores the most relevant results from which the most relevant results will be derived. This warrants representation in the name.


My suggestion: ExpectationResult

When I take all the arguments we exchanged so far into account, I currently feel that ExpectationResult is the best name I can come up with.

I anticipate that you will object that this name puts too much focus on the ‘expectation’ part. I want to reply with the arguments I have given above:
a) I disagree with your interpretation ‘expectation is related to throwing the exception’
b) I think that name should include the fact that it holds the result of a check.

All in all, I think the name ExpectationResult includes everything that we deemed important:

  • It has the ‘Record’-aspect in it. ‘Result’ conveys that this type holds data about something that happened beforehand. I think ‘Result’ is even more precise than ‘Record’ here.
  • It connects the type directly to the term ‘expectation’. And it does so in a meaningful way: The user expected, for example, that a file exists. And the ExpectationResult type holds the information whether or not that expectation was fulfilled.

Ideally you don't need to and the name tells you enough.

It’s not important to this discussion, but I disagree with this sentiment. This is what gives you those long, long names that are so common in JVM programming. People try to make their type name explain everything about the type. I don’t think that this is helpful. I think effectively, it makes code harder than easier to read. The longer the word, the higher the chance that people don’t really read it through carefully. Because of that, I think short, catchy, names are to be prefered. Such names will be better understood and remembered. I think this advantage outweighs the small disadvantage of having to read docs or looking at the type hierarchy to understand more about its purpose.


Did I understand your arguments correctly? What do you say to my counter-arguments, where I have given them? What do you think of ExpectationResult, given all that we have discussed?

@robstoll
Copy link
Owner

robstoll commented Jan 18, 2021

Thanks for you input.
First of all, I think we are not that far away from each other. I will try to outline why. Maybe we just use different terms, look from it from a different angle and thus there is still a confusion/misconception of the logic level.

You want to convey that TypeX holds the result of a computation and does not represent the computation itself.

Not quite. First, I prefer to use TermX because I think we don't talk about the type only (see Table above). It influences also other things we are going to name after it (not necessarily but I think it makes sense).
Second, I want to convey that TermX stands for the result of the verification but does not include the side effect of throwing an AssertionError. This also means that a function creating TermX which I would call a termX function should not have a side effect. I think you totally misunderstood me on this point and that's most likely the reason for the confusion. And yeah... that's where we need to talk about checking again.

To me, ‘checking’, or ‘testing’, or ‘verifying’ all is ‘the act of looking at the state of affairs and seeing if an expected property holds’. Throwing the exception is merely one way to report the result.

In theory I agree, if you would ask some random guy on the street, then they would probably answer the same thing, there might be subtleties between the term but they are more or less interchangeable. But... we are embedding the term in a context and if you talk about checking in the context of writing tests, then I would say most expect that checking involves that the test fails if the check fails and that's done by throwing an AssertionError in most libraries. Of course, we could say, but wait, check and Check are not the same thing (which they are not) and that is done a lot in Java libraries but IMO it is a bad idea, better use different terms for each because they are not directly related to each other.

In other words, when I talked about checking in my previous comment, then I always meant the act of looking at a property and perform the side effect of throwing an AssertionError. For instance:

The fact that it does not imply that it contains itself the checking part.

What I meant is that it does not contain this side effects (throwing an Exception). Instead, it only models the effect of checking. I guess my comment will make more sense now if you re-read it and I guess you will see that we are not that far from each other.

Throwing the exception is akin to telling the car owner: ‘Your car is not road safe, you must repair it before driving it’. Let’s look at your statement: ‘things related to expectation / checking which to my mind are connected with fail the test if the criteria are not met.’

There you have it again:

things related to expectation / checking which to my mind are connected with fail the test if the criteria are not met.

I talk about the side effect.

... and take it at face value. If your statement was correct, then if we simply forget to throw an exception (or forget to tell the car owner that they must not drive), then we would not have checked. To me, this is absurd. Of course have we checked! We checked everything and verified all the expectations.

I agree that it would be absurd if I would have said we did not check 😆 but that's not what I wrote nor what I meant as already outlined.

I try to explain it again differently. I stopped the analogy last time once the TermX was created but I guess I have to go on to make it clearer. Everything I described was happening on the logic level. The mechanics have written down their TermX and passed on the result to the person asking for it. The logic department has done its work, the actual verification. Now their paper-work is passed on and it depends on this person what happens next. If it's someone in the logic department who has asked for it, then it is most likely used as part of another TermX (yes, that's composition). But if it is a guy from the core department (oho... a core guy), then maybe the person just throws the paper work into the next bin 😱. Did the verification/work of the mechanics not happen due to this (as you figured I wanted to convey)? Of course it did, but no effect resulted out of it, because the core guy was not interested in the modelled effect. As side notice, throwing the work into the bin is currently (until we get HTML reports) the desired step of action when using Atrium, it happens in OnlyFailureReporter and implies that your test passed).

See, I would like to come up with a term which does not imply that it has side effects but instead search for a term which conveys that it only models the effect of checking.

I disagree with your interpretation ‘expectation is related to throwing the exception’

Here we have clearly a different view and that's fine... if we would have the same view, then we could use Check or Expectation or Verification. I would probably vote for Verification because a) Check is too loaded with throwing IMO, b) Expectation is too close to expect(...) and Expect. But as outlined, I think we should use something which makes it really obvious that the function creating the TermX does not have side effects.

IMO a function on the API level is allowed/encouraged to imply that it contains the effect of throwing. To my mind, expect(...) implies that the next method called on Expect will cause the effect of throwing an AssertionError if the expectation is not met.
As side notice, that's also not always true and I remember the thread about improving @throws AssertionError where you figured you only need to add one more sentence to cover the actual behaviour. We eventually decided to remove @throws because the actual behaviour is way more complex than what you thought. I hope that we can make it a bit clearer with this distinction that the functions on the logic level are effectful (in the sense of functional design) but don't incorporate side effects.

Last but not least, I would like to underline the difference with code:

expect(2).toBe(3)

You expect that this fails the test:

expect(2)._logic.toBe(3)

As it is currently, you might still think this will fail the test. Which means, I came to the conclusion that we have to rename those functions to something like

expect(2)._logic.createTermXForToBe(3)

Having written the above, I still don’t think Record does the trick. The killer argument for me is this: Record and Reportable are nearly interchangeable.

For me there is a clear distinction. Reportable is just a marker interface, we could also use Any instead. Because basically everything can be a Reportable, we just use it reduce the potential for mistakes. Record on the other hand provides a functionality.
If you would ask me, what is the difference between a Report and a Record then I would, due to the fact that I connect record with databases, tell you that record is probably just a row in the report. Which has some analogy in Atrium.

Ideally you don't need to and the name tells you enough.

It’s not important to this discussion, but I disagree with this sentiment. This is what gives you those long, long names that are so common in JVM programming. People try to make their type name explain everything about the type

I agree with you and I meant to come up with a good catchy name as you called it which still tells you enough so that you don't have to look up the definition for its main usage, is hard.
And I think it is actually also important to this discussion because it is one of the reasons why I still prefer Record over ExpectationResult. I thought about Result instead of Recover but dropped it because kotlin has a Result type and using the same term just calls for trouble. I prefer Record over ExpectationResult because it is shorter, easier to remember and also does not result in long names all over the place (see the table above). And this, even though I have to admit that ExpectationResult conveys better what it is.

The problem is that Record conveys that is some record of some data, but not the kind of data was recorded.

That's a fair point but I would say that it's clear what it records because we are using it in the context of an assertion library.
As you wrote:

We are writing an expectation library, so TypeX is the central data class: It stores the most relevant results from which the most relevant results will be derived.

So for me it is clear that a Record is data about the evidence of a specified expectation. Yet, I see from all the counter-arguments you brought up that you seem to have a hard time to think about Record in this way. I don't know why but I am still open for other terms 🙂

So what about Evidence or Proof instead of Record? Personally, I prefer Record because, as outlined in previous comments, such types also model to a certain degree how they should be displayed in reporting.

edit I reconsidered Proof and I think it is the better term than Record. It fits better although it does not imply anything about reporting.

So my new suggestion is to use Proof. Thoughts?

@robstoll
Copy link
Owner

@jGleitz I'll probably start this or next week with the work on the logic level. So if you have something to say against Proof then please raise your voice now

@jGleitz
Copy link
Collaborator Author

jGleitz commented Jan 26, 2021

Once again: I am sorry that I am currently not able to participate more in atrium’s development in general, and this discussion in specific.


I think we have come a long way in this discussion. And I agree with your statement that

we are not that far away from each other.

For me, Proof is better than Record because it is more specific. It gives me a better notion of what is being stored in it. Having said that, I am still inclined to ask “proof of what?”. Hence, I’d prefer ExpectationResult (or ExpectationProof).

Regarding your argument that

I prefer Record over ExpectationResult because it is shorter, easier to remember and also does not result in long names all over the place (see the table above).

I disagree that ExpectationResult would be the prefix of many types. Most groups can contain not only TermXs, but rather any Reportable. For example, it should be InvisibleReportableGroup instead of InvisibleExpectationResultGroup.

In conclusion, I think we agree that ExpectationResult is more precise, while Proof is shorter. We also agree that both precision and shortness are important. We only disagree on which side of the trade-off we should settle on.

Maybe the argument above sways you in favour of ExpectationResult. If not, I am happy that our discussion lead us to using Proof which is still the second-best proposal, from my point of view.

@jGleitz
Copy link
Collaborator Author

jGleitz commented Jan 26, 2021

Regarding the groups: We could probably find even better names. Maybe the group’s purpose will always be clear from the context, then InvisibleGroup would be enough. Or, if not, InvisibleReportGroup would point out both the purpose and that it only affects reporting, which is both good.

So a) The name for TermX does not necessarily affect how other classes are called and b) if we are renaming, we should carefully think about whether we can find better names than the most obvious ones, just like in this instance.

@robstoll
Copy link
Owner

robstoll commented Jan 26, 2021

Good, I'll go with Proof. I am not yet sure how I rename Assertion and co. but I was also thinking about an InvisibleGroup - so I'll keep that one in mind. The other things will be more or less like shown above in the table where I will replace Record with Proof.

@robstoll robstoll modified the milestones: 0.16.0, 0.17.0 Mar 16, 2021
@robstoll robstoll removed this from the 0.17.0 milestone Mar 30, 2021
@robstoll robstoll added this to the 0.18.0 milestone Mar 30, 2021
@robstoll robstoll self-assigned this Jan 6, 2022
@robstoll robstoll modified the milestones: 0.18.0, 0.20.0 Jan 6, 2022
@robstoll robstoll modified the milestones: 0.20.0, 0.19.0 Apr 27, 2022
robstoll added a commit that referenced this issue May 4, 2022
#746 rename assertion to expectation in README and samples
@robstoll
Copy link
Owner

robstoll commented Dec 29, 2022

the api is free from assertion in 0.18.0 but logic level has still the Assertion type. Going to introduce Proof in 1.3.0

@robstoll robstoll modified the milestones: 0.19.0, 0.21.0 Dec 29, 2022
@robstoll robstoll modified the milestones: 1.2.0, 1.3.0 Aug 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants