Louvorg/ReaderForSelfoss-multiplatform
Louvorg/ReaderForSelfoss-multiplatform
3
2
Fork 2
Code Issues 16 Pull Requests Projects 1 Releases 65 Wiki Activity

Repository Unit Tests #50

Merged
AmineB merged 38 commits from davidoskky/ReaderForSelfoss-multiplatform:repository_tests into master 2022-09-30 11:31:55 +00:00
Conversation 7 Commits 38 Files Changed 7 +81 -1
3 changed files with 81 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit a382fc89ea - Show all commits

1
androidApp/src/main/java/bou/amine/apps/readerforselfossv2/android/fragments/ArticleFragment.kt
View File

@ -269,6 +269,7 @@ class ArticleFragment : Fragment(), DIAware {
private fun getContentFromMercury(customTabsIntent: CustomTabsIntent) {
if (repository.isNetworkAvailable()) {
binding.progressBar.visibility = View.VISIBLE
// TODO: The api should be accessed through the repository
AmineB marked this conversation as resolved Outdated
AmineB commented 2022-09-27 13:11:52 +00:00
Outdated
Review
Copy Link

This is a simple api call to fetch the artcile content. There is no need to put it inside the repository. It is only used here.

This is a simple api call to fetch the artcile content. There is no need to put it inside the repository. It is only used here.
davidoskky commented 2022-09-27 18:16:59 +00:00
Outdated
Review
Copy Link

Yes, I do understand.
I suggest this edit just because it would allow to keep all information about the connectivity private within the repository and increase incapsulation.
But there is no real significant difference.

Yes, I do understand. I suggest this edit just because it would allow to keep all information about the connectivity private within the repository and increase incapsulation. But there is no real significant difference.
👍 1
val parser = MercuryApi()
parser.parseUrl(url).enqueue(

5
shared/src/commonMain/kotlin/bou/amine/apps/readerforselfossv2/repository/RepositoryImpl.kt
View File

@ -378,6 +378,7 @@ class Repository(private val api: SelfossApi, private val appSettingsService: Ap
api.refreshLoginInformation()
}
// TODO: This should be private
suspend fun updateApiVersion() {
val apiMajorVersion = appSettingsService.getApiVersion()
@ -389,6 +390,8 @@ class Repository(private val api: SelfossApi, private val appSettingsService: Ap
}
}
// TODO: This should be private (since all api calls are made through the repository
// no other entity needs to know about the connectivity status)
fun isNetworkAvailable() = isConnectionAvailable.value && !offlineOverride
private fun getDBActions(): List<ACTION> =
@ -440,7 +443,7 @@ class Repository(private val api: SelfossApi, private val appSettingsService: Ap
db.itemsQueries.updateItem(item.datetime, item.title.getHtmlDecoded(), item.content, item.unread, item.starred, item.thumbnail, item.icon, item.link, item.sourcetitle, item.tags.joinToString(","), item.id.toString())
suspend fun tryToCacheItemsAndGetNewOnes(): List<SelfossModel.Item>? {
suspend fun tryToCacheItemsAndGetNewOnes(): List<SelfossModel.Item> {
try {
val newItems = getMaxItemsForBackground(ItemType.UNREAD)
val allItems = getMaxItemsForBackground(ItemType.ALL)

76
shared/src/commonTest/kotlin/bou/amine/apps/readerforselfossv2/repository/RepositoryTest.kt
View File

@ -692,6 +692,82 @@ class RepositoryTest() {
coVerify(exactly = 1) { api.refreshLoginInformation() }
coVerify(exactly = 1) {appSettingsService.refreshLoginInformation("https://test.com/selfoss/", "login", "password")}
}
@Test
fun `cache items`() {
coEvery { api.getItems(any(), any(), any(), any(), any(), any(), any()) } returns
SelfossModel.StatusAndData(success = true, data = generateTestApiItem())
val repository = Repository(api, appSettingsService, connectivityStatus, db)
repository.tagFilter = SelfossModel.Tag("Tag", "read", 0)
repository.sourceFilter = SelfossModel.Source(
1,
"First source",
listOf("Test", "second"),
"spouts\\rss\\fulltextrss",
"",
"d8c92cdb1ef119ea85c4b9205c879ca7.png"
)
repository.searchFilter = "search"
var items = emptyList<SelfossModel.Item>()
runBlocking {
items = repository.tryToCacheItemsAndGetNewOnes()
}
coVerify(exactly = 3) { api.getItems(any(), 0, null, null, null, null, 200) }
AmineB marked this conversation as resolved Outdated
AmineB commented 2022-09-27 14:09:43 +00:00
Outdated
Review
Copy Link

This should be changed to coVerify(exactly = 3) { api.getItems(any(), 0, "Tag", 1, "search", null, 200) }

This should be changed to `coVerify(exactly = 3) { api.getItems(any(), 0, "Tag", 1, "search", null, 200) }`
davidoskky commented 2022-09-27 18:28:54 +00:00
Outdated
Review
Copy Link

This function is supposed to store articles for the background; it should store all articles independently of the tags selected by the user in the foreground.

This function is supposed to store articles for the background; it should store all articles independently of the tags selected by the user in the foreground.
AmineB commented 2022-09-27 18:34:44 +00:00
Outdated
Review
Copy Link

I don't know what you wanted to test.

The thing is, passing null, null, null, null in the function will make it fail, because in your calls, you pass data.

I don't know what you wanted to test. The thing is, passing `null, null, null, null` in the function will make it fail, because in your calls, you pass data.
davidoskky commented 2022-09-27 18:55:02 +00:00
Outdated
Review
Copy Link

I expect the function to behave differently from how it is currently behaving.

Expected behavior:
When background sync starts, the last 200 articles for each item type are cached.
Thus I expect these 3 calls
getItems("unread", 0, null, null, null, null, 200)
getItems("all", 0, null, null, null, null, 200)
getItems("starred", 0, null, null, null, 200)

If one of those nulls is substituted by another value, it will not be the last 200 articles that will be stored, but 200 articles filtered according to some external factor.
This is not what I would expect as an user, I would expect that the latest articles are always available if I enable background syncronization.

I expect the function to behave differently from how it is currently behaving. Expected behavior: When background sync starts, the last 200 articles for each item type are cached. Thus I expect these 3 calls getItems("unread", 0, null, null, null, null, 200) getItems("all", 0, null, null, null, null, 200) getItems("starred", 0, null, null, null, 200) If one of those nulls is substituted by another value, it will not be the last 200 articles that will be stored, but 200 articles filtered according to some external factor. This is not what I would expect as an user, I would expect that the latest articles are always available if I enable background syncronization.
AmineB commented 2022-09-27 19:20:16 +00:00
Outdated
Review
Copy Link

If one of those nulls is substituted by another value, it will not be the last 200 articles that will be stored, but 200 articles filtered according to some external factor.

Well, this is because you set filters from lines 701 to 711. When using the app, no filter would be set when calling this method, so you'll have what you expect.

> If one of those nulls is substituted by another value, it will not be the last 200 articles that will be stored, but 200 articles filtered according to some external factor. Well, this is because you set filters from lines 701 to 711. When using the app, no filter would be set when calling this method, so you'll have what you expect.
davidoskky commented 2022-09-27 19:29:14 +00:00
Outdated
Review
Copy Link

No, filters can be set when this method is called.
This method should deliberately ignore filters.

Imagine this:

  • Open the app
  • Select one tag
  • The background sync starts at this moment

Since the repository stores the filter internally, the filter will be set for this background function as well.
This function should make the call without any filter even when filters are set in the repository.

No, filters can be set when this method is called. This method should deliberately ignore filters. Imagine this: - Open the app - Select one tag - The background sync starts at this moment Since the repository stores the filter internally, the filter will be set for this background function as well. This function should make the call without any filter even when filters are set in the repository.
AmineB commented 2022-09-27 19:32:34 +00:00
Outdated
Review
Copy Link

Ok.

So getMaxItemsForBackground should be changed, then.

Ok. So `getMaxItemsForBackground` should be changed, then.
👍 1
assertSame(3, items.size)
AmineB marked this conversation as resolved Outdated
AmineB commented 2022-09-27 14:13:00 +00:00
Outdated
Review
Copy Link

tryToCacheItemsAndGetNewOnes will only return 1 item as it only returns new items.

You will also need to add every { db.itemsQueries.transaction(any(), any()) } returns Unit to the setup.

`tryToCacheItemsAndGetNewOnes` will only return `1` item as it only returns new items. You will also need to add `every { db.itemsQueries.transaction(any(), any()) } returns Unit` to the setup.
👍 1
}
@Test
fun `cache items but response fails`() {
coEvery { api.getItems(any(), any(), any(), any(), any(), any(), any()) } returns
SelfossModel.StatusAndData(success = false, data = generateTestApiItem())
val repository = Repository(api, appSettingsService, connectivityStatus, db)
repository.tagFilter = SelfossModel.Tag("Tag", "read", 0)
repository.sourceFilter = SelfossModel.Source(
1,
"First source",
listOf("Test", "second"),
"spouts\\rss\\fulltextrss",
"",
"d8c92cdb1ef119ea85c4b9205c879ca7.png"
)
repository.searchFilter = "search"
var items = emptyList<SelfossModel.Item>()
runBlocking {
items = repository.tryToCacheItemsAndGetNewOnes()
}
coVerify(exactly = 3) { api.getItems(any(), 0, null, null, null, null, 200) }
AmineB marked this conversation as resolved Outdated
AmineB commented 2022-09-27 14:14:32 +00:00
Outdated
Review
Copy Link

This should be coVerify(exactly = 3) { api.getItems(any(), 0, "Tag", 1, "search", null, 200) }

This should be `coVerify(exactly = 3) { api.getItems(any(), 0, "Tag", 1, "search", null, 200) }`
davidoskky commented 2022-09-27 18:35:48 +00:00
Outdated
Review
Copy Link

As above

As above
AmineB commented 2022-09-27 18:36:45 +00:00
Outdated
Review
Copy Link

As above.

I don't know what you wanted to test.

The thing is, passing null, null, null, null in the function will make it fail, because in your calls, you pass data.

As above. I don't know what you wanted to test. The thing is, passing null, null, null, null in the function will make it fail, because in your calls, you pass data.
assertSame(0, items.size)
}
@Test
fun `cache items without connection`() {
coEvery { api.getItems(any(), any(), any(), any(), any(), any(), any()) } returns
SelfossModel.StatusAndData(success = false, data = generateTestApiItem())
every { connectivityStatus.isNetworkConnected } returns MutableStateFlow(false)
val repository = Repository(api, appSettingsService, connectivityStatus, db)
repository.tagFilter = SelfossModel.Tag("Tag", "read", 0)
repository.sourceFilter = SelfossModel.Source(
1,
"First source",
listOf("Test", "second"),
"spouts\\rss\\fulltextrss",
"",
"d8c92cdb1ef119ea85c4b9205c879ca7.png"
)
repository.searchFilter = "search"
var items = emptyList<SelfossModel.Item>()
runBlocking {
items = repository.tryToCacheItemsAndGetNewOnes()
}
coVerify(exactly = 0) { api.getItems(any(), 0, null, null, null, null, 200) }
assertSame(emptyList<SelfossModel.Item>(), items)
}
}
fun generateTestDBItems(item : FakeItemParameters = FakeItemParameters()) : List<ITEM> {