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

use the info bullet point for unexpected Exceptions #738

Closed
robstoll opened this issue Dec 23, 2020 · 5 comments · Fixed by #740
Closed

use the info bullet point for unexpected Exceptions #738

robstoll opened this issue Dec 23, 2020 · 5 comments · Fixed by #740
Assignees
Milestone

Comments

@robstoll
Copy link
Owner

Platform (all, jvm, js): all
Extension (none, kotlin 1.3): none

Code related feature

For instance, instead of

expected that subject: () -> kotlin.Nothing        (readme.examples.ReadmeSpec$1$4$1 <1234789>)
◆ ▶ thrown exception when called: java.lang.IllegalArgumentException
    ◾ is instance of type: IllegalStateException (java.lang.IllegalStateException)
      » Properties of the unexpected IllegalArgumentException
        » message: "name is empty"        <1234789>
        » stacktrace: 
          ⚬ readme.examples.ReadmeSpec$1$4$1.invoke(ReadmeSpec.kt:76)
          ⚬ readme.examples.ReadmeSpec$1$4$1.invoke(ReadmeSpec.kt:51)
          ⚬ readme.examples.ReadmeSpec$1$4.invoke(ReadmeSpec.kt:691)
          ⚬ readme.examples.ReadmeSpec$1$4.invoke(ReadmeSpec.kt:51)

It would be nicer if we see:

expected that subject: () -> kotlin.Nothing        (readme.examples.ReadmeSpec$1$4$1 <1234789>)
◆ ▶ thrown exception when called: java.lang.IllegalArgumentException
    ◾ is instance of type: IllegalStateException (java.lang.IllegalStateException)
    ℹ Properties of the unexpected IllegalArgumentException
      » message: "name is empty"        <1234789>
      » stacktrace: 
        ⚬ readme.examples.ReadmeSpec$1$4$1.invoke(ReadmeSpec.kt:76)
        ⚬ readme.examples.ReadmeSpec$1$4$1.invoke(ReadmeSpec.kt:51)
        ⚬ readme.examples.ReadmeSpec$1$4.invoke(ReadmeSpec.kt:691)
        ⚬ readme.examples.ReadmeSpec$1$4.invoke(ReadmeSpec.kt:51)
@robstoll robstoll added this to the 0.15.0 milestone Dec 23, 2020
@robstoll robstoll self-assigned this Dec 23, 2020
@jGleitz
Copy link
Collaborator

jGleitz commented Dec 23, 2020

I think we should discuss whether this is a good use of the new bullet point, taking also into account what we discussed in robstoll/atrium-roadmap#91.

My understanding is that this bullet point is for providing auxiliary information that aides debugging. The properties of a thrown exception are not that, from my point of view.

I think that it is important that Atrium is very consistent in its reporting, as I think the bullet points can be a great help when scanning reports.

@robstoll
Copy link
Owner Author

I basically agree, quickly thought about introducing the magnifier bullet point but came to the conclusion that I don't want to introduce it in 0.15.0. Because then I would want to change the Path assertions as well and I prefer to release 0.15.0 after I have merged the last two issues I have pre-seen for 0.15.0. In this sense it is still an improvement IMO and will be further improved with the magnifier bullet point in a later version.

@jGleitz
Copy link
Collaborator

jGleitz commented Dec 23, 2020

From my point of view, printing the unexpected exception does not fall under any of the use cases we discussed in robstoll/atrium-roadmap#91 at all.

Instead, printing the unexpected exception is for me the same use case as printing the unexpected value in an equals expectation. So I don’t see the need for a special bullet point at all.

For me, this is different from the path expectations, where we give information about related properties. Here, we give information directly about the unexpected value.

@robstoll
Copy link
Owner Author

I would use the magnifier bullet point as it is additional information which helps in debugging the error. A bit like:

» the closest existing parent directory is /usr/bin

which also tells something about the actual context

@jGleitz
Copy link
Collaborator

jGleitz commented Dec 23, 2020

For me, there is a big difference: In the path expectations, the subject is a path, and a wrong value is also a path. Hence, contents on the disk are secondary information for debugging, hence the special bullet point.

For exceptions, the subject and wrong value of the expectations are exceptions, so the wrong exception is the primary information. This does not need special treatment.

However, I recognize that one can see this differently and I won’t split hairs about this for too long. I think the magnifying glass is also acceptable. If it won’t be the in the long run, it’s alright, I guess.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants