[Bug]: Unused binds in TestDataModule

[pitoszud]

Is there an existing issue for this?

• [X] I have searched the existing issues

Is there a StackOverflow question about this issue?

• [X] I have searched StackOverflow

What happened?

There are some repository binds in TestDataModule that are not provided or injected anywhere in the app, so you've got some FakeRepository classes sitting idle e.g FakeTopicsRepository

Instead, you are using test doubles data sources e.g TestNiaNetworkDataSource where you create a fake data source e.g FakeNiaNetworkDataSource from the network module.

package com.google.samples.apps.nowinandroid.core.data.test.TestDataModule

Thanks Patryk

Relevant logcat output

No response

Code of Conduct

• [X] I agree to follow this project's Code of Conduct

[SimonMarquis]

@pitoszud This TestDataModule is used in @HiltAndroidTest unit-tests and instrumented-tests in the main :app module. Without it, the tests would use the real implementations and this is not what we want for tests. Let's close this issue unless you need more info 😉

[JackEblan]

@pitoszud This TestDataModule is used in @HiltAndroidTest unit-tests and instrumented-tests in the main :app module. Without it, the tests would use the real implementations and this is not what we want for tests. Let's close this issue unless you need more info 😉

Can you explain where Fakes were used?

package com.google.samples.apps.nowinandroid.core.data.repository.fake I can't find any usage for the classes inside it. Only the TestdataModule is using them and yet I also can't find where did you inject those fakes

[SimonMarquis]

The fakes are implicitly used because of this:

https://github.com/android/nowinandroid/blob/f5b3ae56dcf0022456d061d0c4c121be5a144984/core/data-test/src/main/kotlin/com/google/samples/apps/nowinandroid/core/data/test/TestDataModule.kt#L36-L41

Hilt will replace the production binding in the @HiltAndroidTest. You can read more on this behavior in https://dagger.dev/hilt/testing.html

[JackEblan]

The fakes are implicitly used because of this:

https://github.com/android/nowinandroid/blob/f5b3ae56dcf0022456d061d0c4c121be5a144984/core/data-test/src/main/kotlin/com/google/samples/apps/nowinandroid/core/data/test/TestDataModule.kt#L36-L41

Hilt will replace the production binding in the @HiltAndroidTest. You can read more on this behavior in https://dagger.dev/hilt/testing.html

Simon, what module did you use this TestDataModule?

[SimonMarquis]

I'm not sure I understand your question. Can you rephrase that?

[JackEblan]

I'm not sure I understand your question. Can you rephrase that?

I mean why we need the fakes in com.google.samples.apps.nowinandroid.core.data.repository.fake if we only use the fakes in com.google.samples.apps.nowinandroid.core.testing.repository during testing?

By the way, I found that SyncWorker has some real repositories injected into it. Is this where we use the fakes during instrumented tests?

com.google.samples.apps.nowinandroid.sync.workers

[SimonMarquis]

I mean why we need the fakes in com.google.samples.apps.nowinandroid.core.data.repository.fake if we only use the fakes in com.google.samples.apps.nowinandroid.core.testing.repository during testing?

I think this was not the case before. As it appears now, it seems like that all these fake repos could be moved to :core:data-test:

• FakeNewsRepository
• FakeRecentSearchRepository
• FakeSearchContentsRepository
• FakeTopicsRepository
• FakeUserDataRepository

Ideally, we should migrate these to regular testFixtures, you can read more on that here.

• #1144

---

By the way, I found that SyncWorker has some real repositories injected into it. Is this where we use the fakes during instrumented tests?

Indeed, the instrumented tests of :sync:work will use real dependencies:

Details

niaPreferences=com.google.samples.apps.nowinandroid.core.datastore.NiaPreferencesDataSource@df0736b
topicRepository=com.google.samples.apps.nowinandroid.core.data.repository.OfflineFirstTopicsRepository@24deec8
newsRepository=com.google.samples.apps.nowinandroid.core.data.repository.OfflineFirstNewsRepository@9fcb61
searchContentsRepository=com.google.samples.apps.nowinandroid.core.data.repository.DefaultSearchContentsRepository@2075686
ioDispatcher=UnconfinedTestDispatcher[scheduler=kotlinx.coroutines.test.TestCoroutineScheduler@b20db47]
analyticsHelper=com.google.samples.apps.nowinandroid.core.analytics.StubAnalyticsHelper@fb72f74
syncSubscriber=com.google.samples.apps.nowinandroid.sync.status.StubSyncSubscriber@92cbc9d

But adding this dependency: androidTestImplementation(projects.core.dataTest) will result in fake dependencies to be used instead:

Details

niaPreferences=com.google.samples.apps.nowinandroid.core.datastore.NiaPreferencesDataSource@19affae
topicRepository=com.google.samples.apps.nowinandroid.core.data.repository.fake.FakeTopicsRepository@77a114f
newsRepository=com.google.samples.apps.nowinandroid.core.data.repository.fake.FakeNewsRepository@259a0dc
searchContentsRepository=com.google.samples.apps.nowinandroid.core.data.repository.fake.FakeSearchContentsRepository@554b5e5
ioDispatcher=UnconfinedTestDispatcher[scheduler=kotlinx.coroutines.test.TestCoroutineScheduler@89b68ba]
analyticsHelper=com.google.samples.apps.nowinandroid.core.analytics.StubAnalyticsHelper@df0736b
syncSubscriber=com.google.samples.apps.nowinandroid.sync.status.StubSyncSubscriber@24deec8

While this is more "appropriate" for instrumentations tests, this currently won't have any major impact since this instrumented test does not really do anything appart from checking if the worker is correctly enqueued, and all "real" dependencies are already provided on the classpath by the implementation(projects.core.data). The main issue here would be the architecture modularisation. Libraries and feature modules should not depend on the "real" "data" layer, otherwise it creates a strong coupling with real implementations.