Makes Core and DB assertions coexist with only one import

[jmaniquet]

Proposal for : #329

The suggested implementation works nicely in my eyes : I am able to use both core and db assertions while using only one single org.assertj.core.api.Assertions.assertThat static import.

Now I did encounter a problem : class Changes_Constructor_Test stopped compiling with that implementation.
The reason is statements like this :

1. assertThat(changes.getRequestAtStartPoint()).isNull();
2. assertThat(changes.getTablesAtStartPointList()).hasSize(5);

The first assertThat used to return an ObjectAssert - which provides isNull assertion
The second used to return a ListAssert - which provides hasSize assertion

After the fix they respectively provide ChangesAssert and TableAssert, since inputs are now treated as AssertProvider instances.

I identified two possible workarounds :

1. Keep assertThat static import with something like this : assertThat((Object) changes.getRequestAtStartPoint()).isNull();
2. Remove it and declare custom assertThat methods with signatures that suit me

I went with second one; but neither maybe acceptable; let me know.

If we go forward with a fix (whatever it may be), I see also this :

• Clear bonus : it seems to work with BDDAssertions.then which has the same characteristics (I did not notice at first since i do not use it) - AssertJ Core provides its then variants and one has an AssertProvider overload
• Possible bonus : assertThat and then methods from AssertJ DB seems to become obsolete, and maybe can be deprected; even removed in the future. I could be wrong though.

I also added a commit that changes the pom version to a new Snapshot.
I am not sure if I was supposed to do it or not; but I can remove this commit if need be.

Let me know what you think

[scordio]

• Possible bonus : assertThat and then methods from AssertJ DB seems to become obsolete, and maybe can be deprected; even removed in the future. I could be wrong though.

I had the same line of thinking – let's see if it's feasible.

[VanRoy]

Thanks @jmaniquet , I'll review it soon.

@jmaniquet @scordio I agree with you regarding the depreciation. We will consider it for the next major version.

[VanRoy]

@jmaniquet Seems pretty good 👍 . Can you just add few tests for methods "assertThat" added to Table, Request and Changes ?
Thanks a lot.

[jmaniquet]

@jmaniquet Seems pretty good 👍 . Can you just add few tests for methods "assertThat" added to Table, Request and Changes ? Thanks a lot.

Hi !
Thank you for your feedback.
I will do that.
I was actually wondering how i should have tested this.
I will look at the test suite and have something ready by this week-end.

[VanRoy]

@jmaniquet Thanks perfect. Let me know in case you don't have enough time to do it, I can try to work on it next week.

[jmaniquet]

Hi again

I came up with something for the tests.
I'm not sure if this was the right way to test it, but it did show me that i had completely botched the Request part

I was calling org.assertj.core.api.Assertions.assertThat instead of org.assertj.db.api.Assertions .
The result was a beautiful StackOverflowError since the object was calling itself in an infinite loop.

I'm ashamed to say that I had only tested the compile aspect of the solution, and not the runtime part.

So aside from the new unit tests I added, I was more thorough : I made a dummy project to test a real insert (using Spring Boot / HSQLDB / flyway with a simple table) and covered the three items with it (changes / request / table)

The tests are in a new commit.
But I did the fix by amending the initial commit (I don't like having botched commits).
I'm not sure if thats ok but did not think of it before force pushing it.

As a side note, I noticed that the tests are still using JUnit4 and have a mix of AssertJ and JUnit4-based assertions.
Is there any intention to use AssertJ-based assertions everywhere ? And to drop Junit4 completely ?

[VanRoy]

Hi @jmaniquet , perfect , no worries for push force 👍 .
For JUnit4 assertions, to be honest I didn't have plan to migrate, but it's a very good point I created a ticket for that.