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

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

Open
tomg246 opened this issue Apr 27, 2023 · 4 comments
Open

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

tomg246 opened this issue Apr 27, 2023 · 4 comments
Labels
enhancement New feature or request

Comments

@tomg246
Copy link

tomg246 commented Apr 27, 2023

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 tomg246 added the enhancement New feature or request label Apr 27, 2023
@tomg246
Copy link
Author

tomg246 commented Apr 27, 2023

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
Copy link
Collaborator

mreiche commented May 7, 2023

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
Copy link
Contributor

arbu commented Jul 11, 2023

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
Copy link
Contributor

martingrossmann commented Aug 22, 2023

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:

  • finalize() is deprecated since Java 9. Alternative should be the Cleaner API (increased the complexity).
  • Some assertion classes like StringAssertion or QuantityAssertion extends from BinaryAssertion. Just a simple getActual of a String attribute "calls" the finalize(). Therefor there is needed a complex check if a is call is needed or not.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants