Assertj-core: Matcher that verifies change in some value

[regishideki]

Summary

In Rspec (a Ruby test library) there is a matcher called change:
https://relishapp.com/rspec/rspec-expectations/docs/built-in-matchers/change-matcher

It is very useful for some situations in which we want to verify if a value changed (or not).

Example

I see no Java/Kotlin libraries that do something similar. So I tried to start something to see if it was possible.

The code example below is in Kotlin because I use Kotlin with java assertj library, but I can translate it if necessary. Another thing to mention is that the order of the methods is not following the same order of Rspec because of the static typing system.

What do you think? Do you know it would be useful to have some matchers like that? This is just an example, in the Rspec link there are many more related features.

import ChangeAssert.Companion.assertThat
import org.assertj.core.api.AbstractAssert
import org.junit.Test

class AssertChangeTest {
    @Test
    fun `test change`() {
       val person = Person(name = "maria")

       assertThat { person.name }
               .changedFrom("maria")
               .to("pedro")
               .whenExecuting { person.name = "pedro" }

    }
}

data class Person(var name: String)

class ChangeAssert {
    companion object {
        fun assertThat(changedVariable: () -> String): StringChangedAssert {
           return StringChangedAssert.assertThat(changedVariable)
        }
    }
}

class StringChangedAssert(val changedVariable: () -> String)
    : AbstractAssert<StringChangedAssert, () -> String>(changedVariable, StringChangedAssert::class.java) {

    private var expectedInitialValue: String = ""
    private var expectedFinalValue: String = ""

    companion object {
        fun assertThat(changedVariable: () -> String): StringChangedAssert {
            return StringChangedAssert(changedVariable)
        }
    }

    fun changedFrom(value: String): StringChangedAssert {
        expectedInitialValue = value

        return this
    }

    fun to(value: String): StringChangedAssert {
        expectedFinalValue = value

        return this
    }

    fun whenExecuting(action: () -> Unit) {
        val actualInitialValue = changedVariable.invoke()
        if(expectedInitialValue != actualInitialValue) {
           failWithMessage("Expected initial value to be '$expectedInitialValue' but it is $actualInitialValue")
        }

        action.invoke()

        val actualFinalValue = changedVariable.invoke()
        if(expectedFinalValue != actualFinalValue) {
            failWithMessage("Expected final value to be '$expectedFinalValue' but it is $actualFinalValue")
        }
    }
}

[joel-costigliola]

I get the idea but isn't that similar to:

// GIVEN
val person = Person(name = "maria")
assertThat { person.name }.isEqalTo("maria")
// WHEN
person.name = "pedro" 
// THEN
assertThat { person.name }.isEqalTo("pedro")

I'm not 100% convinced of the value of adding such construct (which is more fluent though), is there something I don't see?

[regishideki]

@joel-costigliola you are right saying you can do it the way you show us. But I prefer change matcher because:

• it shows the intent more clearly.
• as you said, the construct is more fluent. I think this counts a lot, especially in a test. If it was not the case, we could always use assertTrue for every assertion.

[joel-costigliola]

@scordio WDYT?

[scordio]

The initial example changes the order of the statements compared to the RSpec matcher page and honestly I find confusing having the "when" as last statement.

Assuming we want to have this construct similar to the RSpec change matcher, I would structure it differently using lambdas (please forgive my java example):

Person person = new Person("maria");
assertThat(() -> person.setName("pedro")).changes(person, Person::getName)
                                         .from("maria")
                                         .to("pedro");

or reusing RSpec terminology:

Person person = new Person("maria");
expect(() -> person.setName("pedro")).toChange(person, Person::getName)
                                     .from("maria")
                                     .to("pedro");

and internally it would execute like the example in https://github.com/joel-costigliola/assertj-core/issues/1734#issuecomment-569033515.

I think this could be a separate feature that should not touch the existing core classes. It could even be a separate module.

[regishideki]

Hi @scordio, I explained why I modified the order of the methods in my first comment. The problem resides in the static type system (or in my knowledge about it). For example, what would you return in assertThat(() -> person.setName("pedro")) if you don't know the type variable you are verifying?

[scordio]

Hi @regishideki, sorry I didn't express myself correctly. I understood why you did it but before going into that direction I would try to grab the needed type within the changes()/toChange() invocation, which would not force us to put the when at the end of the chain.

I will create a small PoC about this topic so I can verify if my approach is feasible and we can see how it would look like.

[scordio]

I will create a small PoC about this topic so I can verify if my approach is feasible and we can see how it would look like.

Here we go: https://github.com/joel-costigliola/assertj-core/compare/poc/rspec_change_matcher

It seems feasible but I already see some points to address:

• Better error messages are needed
• by() will grow with boilerplate to support all numeric types
• ExpectChangeMatcher methods should probably have better segregation (e.g., from/to should not be mixed with by)

@joel-costigliola @regishideki what do you think so far?

[joel-costigliola]

I'm ok with the feature if some people find it useful as long as it is not something huge to maintain (it looks ok in that regard so far).

my personal preference would go for the first syntax propose by @scordio which I feel is the most readable:

Person person = new Person("maria");
assertThat(() -> person.setName("pedro")).changes(person, Person::getName)
                                         .from("maria")
                                         .to("pedro");

my only concern would be that it is another assertThat but I think I can live with it if we don't find better alternatives.

[scordio]

my personal preference would go for the first syntax propose by @scordio which I feel is the most readable:

Person person = new Person("maria");
assertThat(() -> person.setName("pedro")).changes(person, Person::getName)
                                         .from("maria")
                                         .to("pedro");

my only concern would be that it is another assertThat but I think I can live with it if we don't find better alternatives.

As a result of the PoC, I would propose to use assertThat as entry point only if this is going to be a separate module from core features, because it has a special behavior and having it in the core could confuse the end user.

To see it better, imagine the corresponding BDD entry point: assertThat(() -> person.setName("pedro")) should correspond to then(() -> person.setName("pedro")) but in reality this is more a when(...), which is a deviation from all other existing entry points.

[regishideki]

@scordio, I liked the changes you did.