From 5b9c28b93bf65d66b53fe645d8fd3e63d4729b8e Mon Sep 17 00:00:00 2001 From: XiangRongLin <41164160+XiangRongLin@users.noreply.github.com> Date: Sun, 16 Jan 2022 09:10:45 +0100 Subject: [PATCH] Replace JUnit asserts with AssertJ in HistoryRecordManagerTest (#7654) * Replace JUnit asserts with AssertJ in HistoryRecordManagerTest They provide a wider range of assertions, which allow for more detailed error messages. Also convert SearchHistoryEntry to kotlin data class for better error messages, since toString() is implemented. Co-authored-by: Mohammed Anas --- app/build.gradle | 2 + .../local/history/HistoryRecordManagerTest.kt | 52 ++++++------ .../history/model/SearchHistoryEntry.java | 79 ------------------- .../history/model/SearchHistoryEntry.kt | 40 ++++++++++ 4 files changed, 69 insertions(+), 104 deletions(-) delete mode 100644 app/src/main/java/org/schabi/newpipe/database/history/model/SearchHistoryEntry.java create mode 100644 app/src/main/java/org/schabi/newpipe/database/history/model/SearchHistoryEntry.kt diff --git a/app/build.gradle b/app/build.gradle index 0386e7aa2..7bac807a3 100644 --- a/app/build.gradle +++ b/app/build.gradle @@ -112,6 +112,7 @@ ext { leakCanaryVersion = '2.5' stethoVersion = '1.6.0' mockitoVersion = '4.0.0' + assertJVersion = '3.22.0' } configurations { @@ -290,6 +291,7 @@ dependencies { androidTestImplementation "androidx.test.ext:junit:1.1.3" androidTestImplementation "androidx.test:runner:1.4.0" androidTestImplementation "androidx.room:room-testing:${androidxRoomVersion}" + androidTestImplementation "org.assertj:assertj-core:${assertJVersion}" } static String getGitWorkingBranch() { diff --git a/app/src/androidTest/java/org/schabi/newpipe/local/history/HistoryRecordManagerTest.kt b/app/src/androidTest/java/org/schabi/newpipe/local/history/HistoryRecordManagerTest.kt index 3f3a038d8..15e88b09e 100644 --- a/app/src/androidTest/java/org/schabi/newpipe/local/history/HistoryRecordManagerTest.kt +++ b/app/src/androidTest/java/org/schabi/newpipe/local/history/HistoryRecordManagerTest.kt @@ -1,9 +1,9 @@ package org.schabi.newpipe.local.history import androidx.test.core.app.ApplicationProvider +import org.assertj.core.api.Assertions.assertThat import org.junit.After import org.junit.Assert.assertEquals -import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Rule import org.junit.Test @@ -45,10 +45,10 @@ class HistoryRecordManagerTest { // that the number of Lists it returns is exactly 1, we can only check if the first List is // correct. Why on earth has a Flowable been used instead of a Single for getAll()?!? val entities = database.searchHistoryDAO().all.blockingFirst() - assertEquals(1, entities.size) - assertEquals(1, entities[0].id) - assertEquals(0, entities[0].serviceId) - assertEquals("Hello", entities[0].search) + assertThat(entities).hasSize(1) + assertThat(entities[0].id).isEqualTo(1) + assertThat(entities[0].serviceId).isEqualTo(0) + assertThat(entities[0].search).isEqualTo("Hello") } @Test @@ -62,25 +62,25 @@ class HistoryRecordManagerTest { // make sure all 4 were inserted database.searchHistoryDAO().insertAll(entries) - assertEquals(entries.size, database.searchHistoryDAO().all.blockingFirst().size) + assertThat(database.searchHistoryDAO().all.blockingFirst()).hasSameSizeAs(entries) // try to delete only "A" entries, "B" entries should be untouched manager.deleteSearchHistory("A").test().await().assertValue(2) val entities = database.searchHistoryDAO().all.blockingFirst() - assertEquals(2, entities.size) - assertTrue(entries[2].hasEqualValues(entities[0])) - assertTrue(entries[3].hasEqualValues(entities[1])) + assertThat(entities).hasSize(2) + assertThat(entities).usingElementComparator { o1, o2 -> if (o1.hasEqualValues(o2)) 0 else 1 } + .containsExactly(*entries.subList(2, 4).toTypedArray()) // assert that nothing happens if we delete a search query that does exist in the db manager.deleteSearchHistory("A").test().await().assertValue(0) val entities2 = database.searchHistoryDAO().all.blockingFirst() - assertEquals(2, entities2.size) - assertTrue(entries[2].hasEqualValues(entities2[0])) - assertTrue(entries[3].hasEqualValues(entities2[1])) + assertThat(entities2).hasSize(2) + assertThat(entities2).usingElementComparator { o1, o2 -> if (o1.hasEqualValues(o2)) 0 else 1 } + .containsExactly(*entries.subList(2, 4).toTypedArray()) // delete all remaining entries manager.deleteSearchHistory("B").test().await().assertValue(2) - assertEquals(0, database.searchHistoryDAO().all.blockingFirst().size) + assertThat(database.searchHistoryDAO().all.blockingFirst()).isEmpty() } @Test @@ -93,11 +93,11 @@ class HistoryRecordManagerTest { // make sure all 3 were inserted database.searchHistoryDAO().insertAll(entries) - assertEquals(entries.size, database.searchHistoryDAO().all.blockingFirst().size) + assertThat(database.searchHistoryDAO().all.blockingFirst()).hasSameSizeAs(entries) // should remove everything manager.deleteCompleteSearchHistory().test().await().assertValue(entries.size) - assertEquals(0, database.searchHistoryDAO().all.blockingFirst().size) + assertThat(database.searchHistoryDAO().all.blockingFirst()).isEmpty() } @Test @@ -111,11 +111,12 @@ class HistoryRecordManagerTest { // make sure correct number of searches is returned and in correct order val searches = manager.getRelatedSearches("", 6, 4).blockingFirst() - assertEquals(4, searches.size) - assertEquals(RELATED_SEARCHES_ENTRIES[6].search, searches[0]) // A (even if in two places) - assertEquals(RELATED_SEARCHES_ENTRIES[4].search, searches[1]) // B - assertEquals(RELATED_SEARCHES_ENTRIES[5].search, searches[2]) // AA - assertEquals(RELATED_SEARCHES_ENTRIES[2].search, searches[3]) // BA + assertThat(searches).containsExactly( + RELATED_SEARCHES_ENTRIES[6].search, // A (even if in two places) + RELATED_SEARCHES_ENTRIES[4].search, // B + RELATED_SEARCHES_ENTRIES[5].search, // AA + RELATED_SEARCHES_ENTRIES[2].search, // BA + ) } @Test @@ -129,14 +130,15 @@ class HistoryRecordManagerTest { // make sure correct number of searches is returned and in correct order val searches = manager.getRelatedSearches("A", 3, 5).blockingFirst() - assertEquals(3, searches.size) - assertEquals(RELATED_SEARCHES_ENTRIES[6].search, searches[0]) // A (even if in two places) - assertEquals(RELATED_SEARCHES_ENTRIES[5].search, searches[1]) // AA - assertEquals(RELATED_SEARCHES_ENTRIES[1].search, searches[2]) // BA + assertThat(searches).containsExactly( + RELATED_SEARCHES_ENTRIES[6].search, // A (even if in two places) + RELATED_SEARCHES_ENTRIES[5].search, // AA + RELATED_SEARCHES_ENTRIES[1].search, // BA + ) // also make sure that the string comparison is case insensitive val searches2 = manager.getRelatedSearches("a", 3, 5).blockingFirst() - assertEquals(searches, searches2) + assertThat(searches).isEqualTo(searches2) } companion object { diff --git a/app/src/main/java/org/schabi/newpipe/database/history/model/SearchHistoryEntry.java b/app/src/main/java/org/schabi/newpipe/database/history/model/SearchHistoryEntry.java deleted file mode 100644 index fd4588700..000000000 --- a/app/src/main/java/org/schabi/newpipe/database/history/model/SearchHistoryEntry.java +++ /dev/null @@ -1,79 +0,0 @@ -package org.schabi.newpipe.database.history.model; - -import androidx.room.ColumnInfo; -import androidx.room.Entity; -import androidx.room.Ignore; -import androidx.room.Index; -import androidx.room.PrimaryKey; - -import java.time.OffsetDateTime; - -import static org.schabi.newpipe.database.history.model.SearchHistoryEntry.SEARCH; - -@Entity(tableName = SearchHistoryEntry.TABLE_NAME, - indices = {@Index(value = SEARCH)}) -public class SearchHistoryEntry { - public static final String ID = "id"; - public static final String TABLE_NAME = "search_history"; - public static final String SERVICE_ID = "service_id"; - public static final String CREATION_DATE = "creation_date"; - public static final String SEARCH = "search"; - - @ColumnInfo(name = ID) - @PrimaryKey(autoGenerate = true) - private long id; - - @ColumnInfo(name = CREATION_DATE) - private OffsetDateTime creationDate; - - @ColumnInfo(name = SERVICE_ID) - private int serviceId; - - @ColumnInfo(name = SEARCH) - private String search; - - public SearchHistoryEntry(final OffsetDateTime creationDate, final int serviceId, - final String search) { - this.serviceId = serviceId; - this.creationDate = creationDate; - this.search = search; - } - - public long getId() { - return id; - } - - public void setId(final long id) { - this.id = id; - } - - public OffsetDateTime getCreationDate() { - return creationDate; - } - - public void setCreationDate(final OffsetDateTime creationDate) { - this.creationDate = creationDate; - } - - public int getServiceId() { - return serviceId; - } - - public void setServiceId(final int serviceId) { - this.serviceId = serviceId; - } - - public String getSearch() { - return search; - } - - public void setSearch(final String search) { - this.search = search; - } - - @Ignore - public boolean hasEqualValues(final SearchHistoryEntry otherEntry) { - return getServiceId() == otherEntry.getServiceId() - && getSearch().equals(otherEntry.getSearch()); - } -} diff --git a/app/src/main/java/org/schabi/newpipe/database/history/model/SearchHistoryEntry.kt b/app/src/main/java/org/schabi/newpipe/database/history/model/SearchHistoryEntry.kt new file mode 100644 index 000000000..13f3cefc2 --- /dev/null +++ b/app/src/main/java/org/schabi/newpipe/database/history/model/SearchHistoryEntry.kt @@ -0,0 +1,40 @@ +package org.schabi.newpipe.database.history.model + +import androidx.room.ColumnInfo +import androidx.room.Entity +import androidx.room.Ignore +import androidx.room.Index +import androidx.room.PrimaryKey +import java.time.OffsetDateTime + +@Entity( + tableName = SearchHistoryEntry.TABLE_NAME, + indices = [Index(value = [SearchHistoryEntry.SEARCH])] +) +data class SearchHistoryEntry( + @field:ColumnInfo(name = CREATION_DATE) var creationDate: OffsetDateTime, + @field:ColumnInfo( + name = SERVICE_ID + ) var serviceId: Int, + @field:ColumnInfo(name = SEARCH) var search: String +) { + @ColumnInfo(name = ID) + @PrimaryKey(autoGenerate = true) + var id: Long = 0 + + @Ignore + fun hasEqualValues(otherEntry: SearchHistoryEntry): Boolean { + return ( + serviceId == otherEntry.serviceId && + search == otherEntry.search + ) + } + + companion object { + const val ID = "id" + const val TABLE_NAME = "search_history" + const val SERVICE_ID = "service_id" + const val CREATION_DATE = "creation_date" + const val SEARCH = "search" + } +}