ExplanatoryAssertionGroupType Naming

[jGleitz]

We currently have ExplanatoryAssertionGroupType, as supertype for all group types that add some kind of additional information to an assertion. As of atrium#666, We have these subtypes:

• ExplanatoryAssertionGroupType
  • WarningAssertionGroupType
  • InformationAssertionGroupType

I want to suggest to redo the naming of these group types based on the use cases we found while discussing #9 and #15.

These use cases are:

1. tell the user about something that went wrong while checking the assertions, e.g.
   • additional entries in an iterable’s contains check
   • a symbolic link loop while checking a path assertion
   • situations where we cannot extract a feature
2. give the user context information about a failed assertion. Subject of #9, e.g.
   • giving more information about a file system entry
   • telling the user that although two numbers are not strictly equal, they are numerically equal
3. tell the user the reason for why the programmer expects an assertion to hold
   • meant for cases where it is not obvious while the assertions where made the way they were

Currently, we map the use cases as follows:

1 -> WarningAssertionGroupType 2 -> ExplanatoryAssertionGroupType 3 -> InformationAssertionGroupType

I see three issues with the current structure:

i. From my point of view, the names of the types are not a good indication for the use case of the types. ii. I think that it is odd that ExplanatoryAssertionGroupType is a supertype of the other two, although its use case does not really subsum the other two use cases. iii. I think “warning” is a not a good word choice for WarningAssertionGroupType, as it is always used in a context where the “warning” is not recoverable, i.e. the assertion will not succeed without fixing the warning. To my mind, a “warning” is something bad, but recoverable.

I think we should address these issues. To start the discussion, I suggest the following hierarchy:

• InformationAssertionGroupType (instead of ExplanatoryAssertionGroupType)
  • RelatedErrorAssertionGroupType (instead of WarningAssertionGroupType)
  • ReasonAssertionGroupType (instead of InformationAssertionGroupType)
  • DebugDetailAssertionGroupType (new type for use case 2)

Most cases where we currently use ExplanatoryAssertionGroupType would use DebugDetailAssertionGroupType. The (new) InformationAssertionGroupType would only be used for cases that are not covered by the uses cases 1, 2 or 3. Ideally, InformationAssertionGroupType would only serve as a fallback type that is never used.

If we decide to rename the group types, we need to make sure to rename all related helpers as well, like ExplanatoryGroup, withExplanatoryAssertion, withExplanation, etc. pp.

[jGleitz]

Given that we currently plan to use the name withHelpOnFailure for use case 2, it might make sense to either call the group type HelpAssertionGroupType (instead of DebugDetailAssertionGroupType) or the function withDebugDetailsOnFailure.

[robstoll]

Just some food for thought: https://github.com/robstoll/atrium-roadmap/issues/1#issuecomment-532208654

[robstoll]

I agree that we should change the names. They represent the currently used symbols and this is actually configurable.

However, renaming ExplanatoryAssertionGroupType to InformationAssertionGroupType only makes sense if we rename ExplanatoryAssertionGroup to InformationAssertionGroup as well. And here I am not sure if this a good name. I have chosen ExplanatoryAssertionGroup because it is used to explain things, like:

• explain a defined assertionCreator-lambda in reporting instead of actually evaluating it (used e.g. in expect(listOf(1,2)).all { ... }
• explain what mismatches where found
• explain that using BigDecimal.isEqualIncludingScale might not be a good fit
• explain that path.exists failed because of missing permissions etc.

InformationAssertionGroup does not tell me what it is for, its too generic IMO.

[jGleitz]

Just some food for thought: #1 (comment)

You have once again proven how bad my memory is :smile:

Unsurprisingly, I still like ReportableGroup. Calling the supertype ReportableGroupType would also make sense if its main job is being a fallback supertype that is seldom used directly.

I think this would also address your second comment, wouldn’t it? Removing the word Assertion from all of those groups and group types makes sense, because they are not actually assertions, they’re just something that can be reported.

Most of your examples wouldn’t use ReportableGroupType anyway, they would use the more specific HelpReportableGroupType or DebugDetailReportableGroupType.

It makes sense to my mind.

[jGleitz]

To clarify: I think dropping the word “Assertion” from everything that is not actually an assertion already makes sense, even if they still inherit from Assertion. It conveys the intent. We can still realize #1 (i.e. introduce a Renderable interface above Assertion) at a later point.

[robstoll]

To clarify: I think dropping the word “Assertion” from everything that is not actually an assertion already makes sense, even if they still inherit from Assertion. It conveys the intent.

I agree

they would use the more specific HelpReportableGroupType or DebugDetailReportableGroupType.

I prefer HelpReportableGroupType over DebugDetailReportableGroupType

Unsurprisingly, I still like ReportableGroup

I am not 100% sure but I think it might be we need ReportableGroup as a super type but I also think that we still need ExplanatoryGroup. I'll give this some thoughts later on

[jGleitz]

I am not 100% sure but I think it might be we need ReportableGroup as a super type but I also think that we still need ExplanatoryGroup. I'll give this some thoughts later on.

We definitely need a supertype for the group types, because we use when on them. I currently see no use case that is not covered and hence don’t see what ExplanatoryGroup would be need for. I am looking forward to your thoughts!

[robstoll]

I guess we need to re-iterate this as it seems I don't have the same intuition about the change as you do. I am interested in how you would like to see things. Following a few more examples. In the end, I would like to see the following reports (I introduced a new bullet point :bulb:)

• :information_source: => for additional information provided by the user; currently only via because in the API

• :bulb: failure hints about the usage of the chosen function or rules in Atrium:

  expected subject:             10          (java.math.BigDecimal <1234789>)
  ◆ to equal (including scale): 10.0        (java.math.BigDecimal <1234789>)
  💡 notice, if you used isNumericallyEqualTo then the assertion would have hold.

  //

  expected subject: [1]        (java.util.Collections.SingletonList <1234789>)
  ◆ ▶ get(0): 1                (kotlin.Int <1234789>)
    ◾ at least one assertion defined: false
       💡 You forgot to define assertions in the assertionCreator-lambda
          Sometimes you can use an alternative to `{ }` 
          For instance, instead of `toThrow<..> { }` you should use `toThrow<..>()

  //

  expected subject: MyClass
  ◆ to equal:       MyClass
    💡 notice, MyClass has not overridden equals
       better use toBeSameInstanceAs or implement equals/hashCode

• 🔎 additional information to help debugging the failure

  expected subject: /usr/bin/noprogram        (sun.nio.fs.UnixPath <1234789>)
  ◆ to: exist
  🔎 the closest existing parent directory is /usr/bin

  //

  expected that subject: /tmp/atrium-path/directory/subfolder/file        (sun.nio.fs.UnixPath <1234789>)
  ◆ is: a file
  🔎 followed the symbolic link /tmp/atrium-path/directory to /tmp/atrium-path/file
    ❗❗ failure at parent path: /tmp/atrium-path/file
      ⚬ was a file instead of a directory

• ❗❗to indicate an error or further specify the error

  expected that subject: {a=1, b=2}        (java.util.LinkedHashMap <1234789>)
  ◆ contains, in any order:
  ⚬ ▶ entry "c": ❗❗ key does not exist
      » to equal: 2                     (kotlin.Int <1234789>)
  ⚬ ▶ entry "b":  2                     (kotlin.Int <1234789>)
      ◾ equals:  1                     (kotlin.Int <1234789>)

  //

  expected that subject: {a=1, b=2}        (java.util.LinkedHashMap <1234789>)
  ◆ ▶ size:      2                         (kotlin.Int <1234789>)
    ◾ equals: 1                         (kotlin.Int <1234789>)
  ◆ contains only, in any order:
  ✔ ▶ entry "b":  2                      (kotlin.Int <1234789>)
      ◾to equal: 2                       (kotlin.Int <1234789>)
    ❗❗ additional entries detected:
       ⚬ entry "a": 1                    (kotlin.Int <1234789>)

  // (also note how I would use 🔎 in this scenario)

  expected subject: /root/.ssh/config        (sun.nio.fs.UnixPath <1234789>)
  ◆ to be: writable
  ❗❗ failure at parent path: /root        (sun.nio.fs.UnixPath <1234789>)
     ⚬ access was denied
    🔎 the owner is root, the group is root
    🔎 the permissions are u=rwx g= o=

• » to indicate an expectation the user defined but could not be evaluated as a previous subject transformation failed

  expected that subject: {a=1, b=2}        (java.util.LinkedHashMap <1234789>)
  ◆ contains, in any order:
  ⚬ ▶ entry "c": ❗❗ key does not exist
      » to equal: 2                     (kotlin.Int <1234789>)
  ⚬ ▶ entry "b":  2                     (kotlin.Int <1234789>)
      ◾ equals:  1                     (kotlin.Int <1234789>)

[robstoll]

Once we tackle the alignment in reporting, we will need to look for bullet points with a standard width as it really looks ugly if the numbers or parentheses are slightly shifted.

[jGleitz]

Thanks for writing down all those examples! I like your idea of differentiating between messages from the library and messages from the programmer by using :information_source: or :bulb:, respectively.

I agree with all your examples. My only disagreement is with the properties of exceptions, but as I stated in #738, I can live with it, as long as it is not :information_source:. With the new meaning of :information_source:, this because even more important.

I think it is important that we chose good names for the groups representing these bullet points. We should make it as easy as possible for developers to understand what the groups are good for.

[jGleitz]

we will need to look for bullet points with a standard width

is that something we can influence? I’d expect that the character width is determined by the user’s font :confused: .

We could use tabs, which would give as reliable alignment. We would lose control over the width of spacings, though.

[robstoll]

is that something we can influence? I’d expect that the character width is determined by the user’s font confused .

At least a bit. I am only talking about the real bullet points: ◆⚬◾ and not about the emojis; there we are definitely lost. The ones I have chosen now are sometimes a bit wider and I used a smaller space to compensate it. It worked quite fine I guess (because no one ever complained) but it would not once we align it.

[vlsi]

How do all these emojis look on Windows console that does not support UTF-8?

[robstoll]

Good point, so far we did not target windows consoles and as there was never a complaint, I guess most use linux runners in CI? I am aware of that even the current bullet points we use don't look nice in CMD or similar. Personally, I just ignored it in the CI windows built of Atrium.

I guess we never got a complaint because most users use intellij and it looks all good on their console.

Nevertheless, it's something we should tackle if we have more users who use Windows (but IMO it's not a priority). We plan to introduce colours via ANSI and there we will have a fallback for terminals not supporting ANSI (i.e. also all the Windows consoles). I already started with a draft once and as far as I remember, I used asci symbols instead of emoij if there was no support for ANSI.

Do you know if this is coupled, in other words, is it enough if we base the emoji fallback on the support for ANSI or should we somehow figure out in another way the (proper) support for UTF-8?

I would pre-seen the following ascii icons:

• :information_source: => (i)
• :bulb: => (u)
• :mag_right: => (d)
• ❗❗=> (!)

[vlsi]

CI windows

That is my case :-/

is it enough if we base the emoji fallback on the support for ANSI or should we somehow figure out in another way the (proper) support for UTF-8?

I guess the only workaround, for now, is to use ASCII