Write Test Code In 'core.domain' package #1359

squart300kg · 2024-04-03T09:51:03Z

[What I have done and why]
write test code named 'GetRecentSearchQueriesUseCaseTest' in 'core.comain' package

[why i have done]
JoseAlcerreca raised issue numbered #1327.

[note]
#1327 issue completion need to write 3 class test code additionaly. Firstly, i write one class of them. so If this code is appropriate and merged, i will try one pull request containing the rest of 2 class test code.

if this code is inappropriate, please feed back for me. I will gladly accept your opinion. Thanks : )

Fixes #1327

How I'm testing it

Choose at least one:

Unit tests
UI tests
Screenshot tests
N/A (provide justification)

Do tests pass?

Run local tests on DemoDebug variant: ./gradlew testDemoDebug
Check formatting: ./gradlew --init-script gradle/init.gradle.kts spotlessApply

Is this your first pull request?

Sign the CLA
Run ./tools/setup.sh
Import the code formatting style as explained in the setup script.

This reverts commit e1ee9b4.

… into Issue1327/ImproveDomainCoverage

…ndroid#1327])

android#1327])

… into Issue1327/ImproveDomainCoverage

Change-Id: I63e3f7ad009549a8673c61da59995c263526bbac

JoseAlcerreca · 2024-04-03T11:08:06Z

...kotlin/com/google/samples/apps/nowinandroid/core/domain/GetRecentSearchQueriesUseCaseTest.kt

+        for (index in 0 until 5) {
+            recentSearchRepository.insertOrReplaceRecentSearch(testRecentSearchQueries[index])
+            // delay for saving value
+            delay(10L)


we shouldn't rely on timing in unit tests. It's an in-memory cache, why do you need the delay in the first place?

In the loop for saving search queries, there is an issue where the sequence of the saved values is not maintained in order. I hypothesized that the problem might be due to the timing of saving values multiple times.

Therefore, I applied a delay of 10 milliseconds. and so, i resolve for timing issue.

If there are any issues with this approach, I would appreciate your review.

JoseAlcerreca · 2024-04-03T11:08:37Z

...kotlin/com/google/samples/apps/nowinandroid/core/domain/GetRecentSearchQueriesUseCaseTest.kt

+import org.junit.Test
+import kotlin.test.assertEquals
+
+class GetRecentSearchQueriesUseCaseTest {


Please check out https://github.com/android/nowinandroid/wiki/Testing-strategy-and-how-to-test and apply conventions

Thanks. i will check this convention.

JoseAlcerreca · 2024-04-03T11:09:09Z

...kotlin/com/google/samples/apps/nowinandroid/core/domain/GetRecentSearchQueriesUseCaseTest.kt

+        val recentSearchQueries = useCase()
+
+        // insert 5 search queries.
+        for (index in 0 until 5) {


testRecentSearchQueries.take(5).forEach

I realized that the loop type of 'forEach' is mainly used exceping for 'for (item in items)' type. i will use these type from now on :)

JoseAlcerreca · 2024-04-03T11:10:07Z

...kotlin/com/google/samples/apps/nowinandroid/core/domain/GetRecentSearchQueriesUseCaseTest.kt

+            recentSearchQueries.first().map { it.query },
+        )
+
+        // insert 9 more search queries.


any reason to do this again? I think this should be in a different test

after careful consideration, it is correct for deleting this code. :)

JoseAlcerreca · 2024-04-03T11:11:23Z

...kotlin/com/google/samples/apps/nowinandroid/core/domain/GetRecentSearchQueriesUseCaseTest.kt

+    )
+
+    @Test
+    fun whenNoParams_recentSearchQueriesAreReturnedUpTo10() = runTest {


Params should be replaced by its actual name, which is "limit"

yes, your opinion is more reasonable. but, test code naming convention(='whenNoParams_followableTopicsAreReturnedWithNoSorting' in 'GetFollowableTopicsUseCaseTest') is not correct.

so according to what you said, is it correct that this method name should also be modified? if yes, i will try one of them.

Modifying the method name of 'whenNoParams_followableTopicsAreReturnedWithNoSorting' together and pull request.

Register as another issue and proceed.

Or, is there a reason to maintain the name?

JoseAlcerreca · 2024-04-03T11:12:00Z

...kotlin/com/google/samples/apps/nowinandroid/core/domain/GetRecentSearchQueriesUseCaseTest.kt

+        // Obtain a stream of recent search queries with param set 5.
+        val recentSearchQueries = useCase(5)
+
+        // insert 2 search queries.


this is already tested in a previous test, can be removed

Regardless of parameter injection, after thinking about it again, I think the test logic is duplicate.

JoseAlcerreca · 2024-04-03T11:12:15Z

...n/com/google/samples/apps/nowinandroid/core/testing/repository/TestRecentSearchRepository.kt


 class TestRecentSearchRepository : RecentSearchRepository {

    private val cachedRecentSearches: MutableList<RecentSearchQuery> = mutableListOf()

    override fun getRecentSearchQueries(limit: Int): Flow<List<RecentSearchQuery>> =
-        flowOf(cachedRecentSearches.sortedByDescending { it.queriedDate }.take(limit))


is this change necessary?

yes, i think that this code need to be changed. because the flow producers such as 'flowOf' and 'flow { emit() }' have different in emitting updated list. and although one depth inner code is same when emitting data by 'flow { emit() }, two depth inner code is different. because of this, i realized that the way of data emitting can not be same.

[two depth inner code]

left : flow { emit() } / right : flowOf()
additional reference, my blog column(lang korean) : different between flowOf and flow { emit() }

because of this change, i had tested 'SearchViewModelTest' and passed. so, i think that this change is reasonable.

how about your opinion? :)

Change-Id: Ia1d9049642e447d9233713a7e3ee593d0bd56930

Change-Id: I2a818503cffc458085e7aee04c2c12d9df532b3c

android#1327]) Change-Id: I392a1e3ebcbc8bc30623559d97c1657e5d50fbbc

…squart300kg/nowinandroid into Issue1327/ImproveDomainCoverage Change-Id: I9a779a4d11230dd849d1e961d3e572fc13c2002c

squart300kg · 2024-04-05T09:44:22Z

@JoseAlcerreca
I acceped your feedback and pull request again. so, when you have time, I would like you to review my updated code.

Thanks : )

...kotlin/com/google/samples/apps/nowinandroid/core/domain/GetRecentSearchQueriesUseCaseTest.kt

yongsuk44 · 2024-04-10T06:50:21Z

...n/com/google/samples/apps/nowinandroid/core/testing/repository/TestRecentSearchRepository.kt

@@ -19,14 +19,16 @@ package com.google.samples.apps.nowinandroid.core.testing.repository
 import com.google.samples.apps.nowinandroid.core.data.model.RecentSearchQuery
 import com.google.samples.apps.nowinandroid.core.data.repository.RecentSearchRepository
 import kotlinx.coroutines.flow.Flow
-import kotlinx.coroutines.flow.flowOf
+import kotlinx.coroutines.flow.flow

 class TestRecentSearchRepository : RecentSearchRepository {

    private val cachedRecentSearches: MutableList<RecentSearchQuery> = mutableListOf()


How about using MutableSharedFlow like another TestRepository? :)

@yongsuk44 I agree with your opinion. but because i didn't fully understand using 'MutableList' instead of "MutableSahredFlow' like other unit test code, i did not change this code.

and the primary purpose of this 'PR' is writing the test code of 'UseCase' class in domain layer. :)

i have thought that your opinion is reasonable so far 👍

@squart300kg
I've determined that handling test cases reactively when utilizing this repository in the future would likely lead to a smoother process. Hence, I've left a comment to that effect. :)

squart300kg added 12 commits April 3, 2024 16:18

Add GetRecentSearchQueriesUseCaseTest.kt ([android#1327](android#1327))

77677e9

Add GetRecentSearchQueriesUseCaseTest.kt (android#1327)

e1ee9b4

Revert "Add GetRecentSearchQueriesUseCaseTest.kt (android#1327)"

31a4560

This reverts commit e1ee9b4.

Merge remote-tracking branch 'origin/Issue1327/ImproveDomainCoverage'…

7313f6c

… into Issue1327/ImproveDomainCoverage

Fix Change the way data is created in flow ([android#1327])

0f711be

Add test code : whenNoParams_recentSearchQueriesAreReturnedUpTo10 ([a…

17c937f

…ndroid#1327])

FIX test code : whenNoParams_recentSearchQueriesAreReturnedUpTo10 ([a…

db16878

…ndroid#1327])

FIX test code : whenParamIsSetTo5_recentSearchQueriesAreReturnedUpTo5 ([

b782291

android#1327])

FIX test code : whenParamIsSetTo5_recentSearchQueriesAreReturnedUpTo5 ([

98fd9ee

android#1327])

Add test code in GetFollowableTopicsUseCaseTest.kt([android#1327])

16d73b3

Merge remote-tracking branch 'origin/Issue1327/ImproveDomainCoverage'…

80c147c

… into Issue1327/ImproveDomainCoverage

FIX test code : apply code formmat ([android#1327])

5b1c704

Change-Id: I63e3f7ad009549a8673c61da59995c263526bbac

JoseAlcerreca mentioned this pull request Apr 3, 2024

[CI] Code coverage report not shown when PR is generated from fork #1360

Open

JoseAlcerreca self-assigned this Apr 3, 2024

JoseAlcerreca reviewed Apr 3, 2024

View reviewed changes

squart300kg added 4 commits April 4, 2024 21:55

Update test code by applying code conventions and reflecting feedback

ba04452

Change-Id: Ia1d9049642e447d9233713a7e3ee593d0bd56930

apply code convention with spotlessApply ([android#1327])

bf1b117

Change-Id: I2a818503cffc458085e7aee04c2c12d9df532b3c

Update test code by applying code conventions and reflecting feedback ([

2a9fad4

android#1327]) Change-Id: I392a1e3ebcbc8bc30623559d97c1657e5d50fbbc

Merge branch 'Issue1327/ImproveDomainCoverage' of https://github.com/…

212e957

…squart300kg/nowinandroid into Issue1327/ImproveDomainCoverage Change-Id: I9a779a4d11230dd849d1e961d3e572fc13c2002c

SimonMarquis reviewed Apr 5, 2024

View reviewed changes

...kotlin/com/google/samples/apps/nowinandroid/core/domain/GetRecentSearchQueriesUseCaseTest.kt Show resolved Hide resolved

yongsuk44 reviewed Apr 10, 2024

View reviewed changes

squart300kg requested review from JoseAlcerreca and SimonMarquis April 20, 2024 11:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Write Test Code In 'core.domain' package #1359

Write Test Code In 'core.domain' package #1359

squart300kg commented Apr 3, 2024 •

edited

JoseAlcerreca Apr 3, 2024

squart300kg Apr 4, 2024 •

edited

JoseAlcerreca Apr 3, 2024

squart300kg Apr 4, 2024

JoseAlcerreca Apr 3, 2024

squart300kg Apr 4, 2024

JoseAlcerreca Apr 3, 2024

squart300kg Apr 4, 2024

JoseAlcerreca Apr 3, 2024

squart300kg Apr 4, 2024 •

edited

JoseAlcerreca Apr 3, 2024

squart300kg Apr 4, 2024

JoseAlcerreca Apr 3, 2024

squart300kg Apr 4, 2024 •

edited

squart300kg commented Apr 5, 2024 •

edited

yongsuk44 Apr 10, 2024

squart300kg Apr 11, 2024

yongsuk44 Apr 11, 2024

Write Test Code In 'core.domain' package #1359

Are you sure you want to change the base?

Write Test Code In 'core.domain' package #1359

Conversation

squart300kg commented Apr 3, 2024 • edited

Choose a reason for hiding this comment

squart300kg Apr 4, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

squart300kg Apr 4, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

squart300kg Apr 4, 2024 • edited

Choose a reason for hiding this comment

squart300kg commented Apr 5, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

squart300kg commented Apr 3, 2024 •

edited

squart300kg Apr 4, 2024 •

edited

squart300kg Apr 4, 2024 •

edited

squart300kg Apr 4, 2024 •

edited

squart300kg commented Apr 5, 2024 •

edited