telekom / testerra

Testerra is an integrated framework for automating tests for (web) applications.
https://docs.testerra.io/
Apache License 2.0
27 stars 15 forks source link

Risk of Assertions doing nothing when .is(true) is missing #320

Open tomg246 opened 1 year ago

tomg246 commented 1 year ago

Problem. This does an assertion: uiElement.assertThat().text().is("text"); This reads and feels the same but does nothing: uiElement.assertThat().text().contains("sisdfn");

The main problem is that i won't notice my mistake which leads to tests that don't do the assertions i expect them to do.

Solution Best would be an implicit .is(true) when no assertion is done. Second best seems the addition of a compiler warning or something alike, to increase my chance of noticing my mistake, see e.g. grafik

tomg246 commented 1 year ago

For the second best solution: the annotation @org.jetbrains.annotations.Contract(pure=true) at a method seems to activate the "result of ... is ignored" warning

mreiche commented 1 year ago

Yes, this is a design flaw of the inline assertion API. I think a good solution would be, to warn for unfulfilled BinaryAssertions in the finalize() method.

public void finalize() {
    if (!_fullfilled) {
        log().warn(createFailMessage(" was not fulfilled"));
    }
}
arbu commented 1 year ago

I've stumbled across this situation as well. Another possible annotation I would suggest, is @CheckReturnValue as provided by jsr305, SpotBugs or Error Prone. While jsr305 is not actively maintained anymore, it is already pulled in as implicit dependency by TestNG and Guava. It also has out-of-the-box-support in IntelliJ .

martingrossmann commented 1 year ago

Yes, this is a design flaw of the inline assertion API. I think a good solution would be, to warn for unfulfilled BinaryAssertions in the finalize() method.

public void finalize() {
    if (!_fullfilled) {
        log().warn(createFailMessage(" was not fulfilled"));
    }
}

Do you mean a finalize()in DefaultBinaryAssertion? Some notes for that:

The best way but with the biggest pain is to clean the Assertion API, maybe the removal of is.

Other helpers like @org.jetbrains.annotations.Contract are vendor based.