Use status severity and codes in JDT model test assertions

[fedejeanne]

An alternative to #5005 .

Do the checks/assertions based on the status and the error code instead of the text.

[fedejeanne]

@trancexpress I think these kind of assertions would make the tests less brittle.

In general, I try to write tests in a way that they do not check for (error) messages unless of course the sole purpose of the test is to validate the message. These tests are not interested in the message itself but in the kind of error produced by the operations.

WDYT?

[trancexpress]

From my POV its OK for these tests to check with the status error code instead of the message.

[fedejeanne]

In that case, if you decide to merge this one then #5005 may be dropped👍

[trancexpress]

Check warning on line 1823 in org.eclipse.jdt.core.tests.model/src/org/eclipse/jdt/core/tests/model/JavaProjectTests.java

@jenkins-eclipse-jdt jenkins-eclipse-jdt / Compiler

Unnecessary Code

NORMAL:
The value of the local variable rootPath is not used

Should we just merge #5005 ?

[trancexpress]

#5005 got merged, do we continue here?

[fedejeanne]

@trancexpress I am at the OCX this week so I won't be able to look at it. I can continue this PR next week so please don't close it :-)

[fedejeanne]

I rebased on master and I also added a 2nd commit that removes unnecessary code (discovered by the checks in this PR)

[fedejeanne]

Still interested on this one, @trancexpress ? :-)

[trancexpress]

Still interested on this one, @trancexpress ? :-)

I don't think ClasspathInitializerTests cares about the actual message, but the actual message is not tested anywhere else. That does make me slightly uncomfortable with the change.

So one more item from me: Can you check if JavaModelStatus can be used to create the expected status, from the error code you are already providing? Then the message check can also be retained. That will shift the responsibility to JavaModelStatus to create the proper status.

[fedejeanne]

Then the message check can also be retained.

You mean checking the text of the error message in the tests too like #5005 did? The purpose of this PR was the exact opposite: to not check the messages since the most important thing is the kind of generated error. Or did I misunderstood your comment?