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

AssertJ (SoftAssertions) for Android #1493

Closed
aartiPl opened this issue Apr 26, 2019 · 8 comments
Closed

AssertJ (SoftAssertions) for Android #1493

aartiPl opened this issue Apr 26, 2019 · 8 comments

Comments

@aartiPl
Copy link

@aartiPl aartiPl commented Apr 26, 2019

There is a version of ByteBuddy for Android: byte-buddy-android. Couldn't it be used instead of regular bytebuddy for AssertJ? Lack of SoftAssertions is really painful for Android. I have to wrap normal assertions, catch errors and then at the end present user results. Please check if Android version of ByteBuddy can be applied for AssertJ.

Originally posted by @aartiPl in #1322 (comment)

@joel-costigliola
Copy link
Member

@joel-costigliola joel-costigliola commented Apr 27, 2019

Even without soft assertions, AssertJ core 3.x is not fully Android compliant (for example it uses Stream api that is not yet available in Android AFAIK).

At the moment, I would be more inclined to have a separate project for Android forked from assertj-core and updated to be Android compatible, it could moreover provide Android specific assertions. I'm wary of changing byte buddy and even if we do so, we don't have tests to verify Android compatibility.

I haven't the time not the knowledge to maintain assertj-android if we choose to go that way so somebody from the community will have to do so. I'm happy to provide the help to welcome the project to the assertj family though.

@PascalSchumacher @epeee thoughts on the topic ?

@aartiPl
Copy link
Author

@aartiPl aartiPl commented Apr 27, 2019

Fortunately, it is very simple to emulate soft assertions. Not perfect, but works. Below you can find my code (Kotlin version):

SoftAssertiojns.kt

class SoftAssertions {

    private val sb = StringBuilder()
    private var counter = 1
    private var isValid = true

    init {
        sb.append("\n")
    }

    fun check(function: () -> Unit) {
        try {
            function()
        } catch (e: AssertionError) {
            sb.append(counter++).append(". ").append(e.message).append("\n")
            isValid = false
        }
    }

    fun isValid(): Boolean {
        return isValid
    }

    fun message(): String? {
        return sb.toString()
    }
}

on use site:

val softAssertions = SoftAssertions()

softAssertions.check { 
    Assertions.assertThat(actual.types).describedAs("description").containsExactlyElementsOf(expected.types)
}

if (!softAssertions.isValid()) {
    failWithMessage(softAssertions.message())
}

The important point here is that "soft assertions" are important building blocks for more advanced assertions and conditions. Reinventing these all nice assertions which are already written doesn't make any sense. Because of that maybe event soft assertions should be a default, and then normal assertions should be based on results from them. I understand though that changing that might be extremely difficult.

@joel-costigliola
Copy link
Member

@joel-costigliola joel-costigliola commented Apr 28, 2019

The current soft assertions are a bit more elegant that the code you showed because

softAssertions.check { 
   Assertions.assertThat(actual.types).as("description").containsExactlyElementsOf(expected.types)
}

can be written as:

softAssertions.assertThat(actual.types).as("description").containsExactlyElementsOf(expected.types)

For this reason and the fact that it would be a breaking change, I don't think we should do the refactoring you suggest.

You can have a look at https://github.com/willowtreeapps/assertk if your using kotlin and in java you can also use JUnit5 assertAll to mimic soft assertions as in:

@Test
void groupedAssertions() {
    // In a grouped assertion all assertions are executed, and all failures will be reported together.
    assertAll("person",
        () -> assertEquals("Jane", person.getFirstName()),
        () -> assertEquals("Doe", person.getLastName())
   );
}

@aartiPl
Copy link
Author

@aartiPl aartiPl commented Apr 28, 2019

I completely agree with you:

softAssertions.assertThat(actual.types).as("description").containsExactlyElementsOf(expected.types)

is nicer. My code is just workaround for Android.

My solution can be applied though as an interim solution until a final solution for Android is available, so I think it makes sense to link this ticket in docs so that when someone needs custom assertions or conditions he can still have them in the current shape of AssertJ.

@mikezx6r
Copy link
Contributor

@mikezx6r mikezx6r commented Jul 19, 2019

I have edited this comment to now show a usage example with more than one assertion, and stricken out my comments just so people can understand the following comments with proper context.

    @Test
    fun `demonstrate custom soft assertions required for older Android`() {
        val softAssertions = SoftAssertions()
        softAssertions.check { assertThat("a").isEqualTo("b") }
        softAssertions.check { assertThat("cdef").isEqualTo("b") }
        if (!softAssertions.isValid()) {
            fail<Unit>(softAssertions.message())
        }

Just looking at this, and I don't think the suggested SoftAssertions class works. Unless I'm misunderstanding how it should work.

Sample test

    @Test
    fun `test soft assertions`() {
        val softAssertions = SoftAssertions()
        softAssertions.check {
             "a" shouldEqual "b"
            "cdef" shouldContain "b"
        }
        if (!softAssertions.isValid()) {
            fail<Unit>(softAssertions.message())
        }
    }

NOTE: shouldEqual is a Kotlin extension function that does assertThat and isEqualTo. shouldContain is similar.

and output:

java.lang.AssertionError: 
1. 
Expecting:
 <"a">
to be equal to:
 <"b">
but was not.

Soft Assertions are supposed to show the result of all failed exceptions, so the output should also contain

java.lang.AssertionError: 
1. 
Expecting:
 <"cdef">
to contain:
 <"b"> 

Using the AssertJ SoftAssertions works as expected. Here's the equivalent code

    @Test
    fun `test AssertJ soft assertions`() {
        val softAssertions = SoftAssertions()
        softAssertions.assertThat("a").isEqualTo("b")
        softAssertions.assertThat("cdef").contains("b")

        softAssertions.assertAll()
    }

and the output

    Expecting:
    <"a">
    to be equal to:
    <"b">
    but was not.

    java.lang.AssertionError:
    Expecting:
    <"cdef">
    to contain:
    <"b">
    at PropertyMaskerTest.test soft assertions(PropertyMaskerTest.java:19)

@aartiPl
Copy link
Author

@aartiPl aartiPl commented Jul 22, 2019

@mikezx6r Well, you put few assertions in the single block, so how is it supposed to work? You can even argue that this is, in fact, a single atomic check, so it doesn't matter which condition fails first (similarly as e.g. equalsIgnoringCase is making internally several checks, and you don't have any control if a string is compared from the start or from the end).

Just do that:

softAssertions.check {"a" shouldEqual "b"}
softAssertions.check {"cdef" shouldContain "b"}

and it will work as expected.

@mikezx6r
Copy link
Contributor

@mikezx6r mikezx6r commented Jul 22, 2019

Ahhh, yes. My mistake completely. I've updated the comment so people know I made a mistake, and to show how to use it for more than one assertion.

@joel-costigliola
Copy link
Member

@joel-costigliola joel-costigliola commented Jul 22, 2019

Closing this issue as won't do

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

Successfully merging a pull request may close this issue.

None yet
3 participants