removeAssertJRelatedElementsFromStackTrace does not remove all the JDK elements triggered by AssertJ
#3449
Comments
|
Good catch, @almondtools! I would also consider the bug quite obvious but yours seems to be the first feedback on a code that never changed since the forking from Fest Assert (66f11f8) 🙂 According to the Javadoc example of If you would run the following: @Test
void testStacktrace() {
Assertions.setRemoveAssertJRelatedElementsFromStackTrace(false);
assertThat(0).isEqualTo(1);
}you would see how the stack trace really looks like: I believe any element that is a consequence of Referring to the original example, what users should see is only: @joel-costigliola how do you see it? How it currently looks without and with the removal of AssertJ elements:
|
scordio
changed the title
removeAssertJRelatedElementsFromStackTrace does not remove all the JDK elements triggered by AssertJ
|
I completely agree to remove the stack before the user's code as the aim is to clear the clutter from the stack. Let's do it for 3.26.0 |
|
Also check to keep the handling of this code consistent Here the superfluous lines from the example above appear in the middle of the stack trace. I have no preference how to handle this, but just wanted to point out that this test is similar but different. |
How it currently looks without and with the removal of AssertJ elements:
|
|
The lines prefixed with Yet, I think both stacktraces are not optimal. I think there are two "correct" views on this stacktrace:
And "artifacts" are not only code prefixed with
|
|
🤦 I swear I knew I should execute the examples in a separate package to avoid such cases, and still... screenshots updated. |
Starting from 5.10.0, JUnit has:
|
|
I think we should remove all stack trace elements starting from the first assertj element. Example 1: assertThat(0).isEqualTo(1);instead of we would have Note: the extra Example 2: assertThat(0).satisfies(x -> assertThat(x).isEqualTo(1));instead of we would have |
Look at your last example: You did not remove all stack trace elements starting from the first assertj element. You removed all stack trace elements between the user code entries and after the user code entry. My Alternative 1. This would be more obvious if you put the second assert on a new line like this: |
|
I'm not sure I understand what you mean @almondtools, here's the test output with your code snippet
If you expand the folded elements you indeed have non assertj elements between assertj elements but the goal of this feature was to easily find the line of the test where the assertion fails and that is I think the case. Could you show on an example what you would like to have (I did not really understand when you talked about infix and suffix artefacts, what does that mean concretely ?) |
|
Maybe it is just the fact that I read your statement to literal, In your stacktrace you find two (!) lines concerning the test : one at line 67 ( You described this as:
But you kept the user-code line for Concerning suffix artifacts and infix artifacts - maybe this is a wrong translation of my thoughts. I mean the following: In the general case we can have following types of stackelements to be removed
In your last stacktrace there are no suffix lines - but in mine there were. I vote for removing both, but keeping the user-code statements. In the concrete case we would find a stracktrace like this: |
|
I'll give a try to remove also assertj lines between user test code (infix as you call them), they are not relevant to understand the stack trace. |
Assertj stack trace elements were already removed but not the elements coming indirectly from assertj, for example java.lang.reflect.Constructor.newInstance when an assertion error is built dynamically. Fix #3449
Assertj stack trace elements were already removed but not the elements coming indirectly from assertj, for example java.lang.reflect.Constructor.newInstance when an assertion error is built dynamically. Fix assertj#3449
If I call
assertThat(0).isEqualTo(1);, I get an exception with a stacktraceThe first two lines are superfluous and not understandable (
WorldTest:13does not callnewInstanceWithCaller). I think that some bug fix removing assertj internals from the stack now does not remove these lines any more.This bug seems so obvious to me, that I am uncertain whether my configuration is the reason, so I added my IDE and my build system to the context below. Feel free to ask more, if you have a suspect.
Test case reproducing the bug
The text was updated successfully, but these errors were encountered: