Thank you for adding these helpful comments!

I might be missing something here, but since we're not using a custom application for instrumentation tests anymore, do we still need to use this custom test runner?

You're right to catch here. This line doesn't make sense. But I didn't want to focus test in this commit.
My next commit will be for fixing test, and I'm creating a new test application. I'll explain the details for that commit.

@irfano I did the same mistake here on my initial migration in WCAndroid, but later one we found out that when running multiple tests on the same session, the caching of the AndroidInjector causes an issue of having inconsistent copies of dependencies.
For more details check my last PR in WCAndroid: woocommerce/woocommerce-android#5445

Thank you for warning me. I'll consider it while fixing UI tests. 🙏🏻

np, depending on the comment here to ensure WellSql is initialized is not the best approach IMO, since you added a separate class for the initialization, I would suggest adding an explicit init function to it and calling it from the AppInitializer#init function, instead of depending on the init bloc.
This would also ensure that the behavior is correct even if Dagger's behavior changed from eager injecting to lazier injecting here 🙂.

Same remark for WebViewDataDirectorySuffixInitializer.

I initialized them with @Inject annotation instead of AppInitializer#init function, because other @Inject initializations need them.

If I init WellSql in AppInitializer#init, AccountStore crashes because of uninitialized WellSql.

I want to initialize WebViewDataDirectorySuffixInitializer and WellSqlInitializer before everything else. I think doing it in AppInitializer#init is not a right way to do that but we can discuss alternative ways.

What is your main concern here? Does it seem weird to inject a class just for initializing them?

If I init WellSql in AppInitializer#init, AccountStore crashes because of uninitialized WellSql.

Yes, I forgot about this, so maybe calling init directly in the app's onCreate 🤔

What is your main concern here? Does it seem weird to inject a class just for initializing them?

It's nothing major, but just two small issues IMO:

1. It's not super clear, and that's why we depend on the comment to explain why it's needed.
2. It depends on Dagger's logic, if Dagger was to inject the dependencies lazily by default (like Koin for example), this wouldn't work, since the class won't be instantiated until it's injected.

Anyway, it's just a small point, so whatever you and @renanferrari agree on, for WCAndroid, we didn't create any class or WellSql's initialization, and instead it's done manually in the Application's onCreate.

Yes, I forgot about this, so maybe calling init directly in the app's onCreate 🤔

It's called in WooCommerce in WCAndroid. At first I thought doing the same, but then I thought keeping application class minimal could be a better approach. Otherwise, I'd have to handle WellSql initialization also in test application, like this made in WCAndroid.

What about initialize them in AppInitializer's init and injecting AccountStore lazily?
https://gist.github.com/irfano/790096d16ed4b0c4ca1adc366102d191

What about initialize them in AppInitializer's init and injecting AccountStore lazily?

Yes, this is another possibility, I like it 👍

With @Rule annotation, the TestRule runs for each test cases. That means InitializationRule will be called for each test cases. But that causes crash because some functions in AppInitializer must be called only once for application lifecycle.

With isInitialized, I ensured that AppInitializer will be called once. The disadvantage of this way can be that same AppInitializer instance will be used for each test cases. If I encounter this problem, I'll try to clean resources in AppInitializer before test cases.

I tried to use @ClassRule to run test rule once for this class, but it didn't work because of Hilt's Entry Point. I couldn't find the reason. I may try to use @ClassRule again later.

I removed isInitialized here. This was a wrong approach. AppInitializer should be recreated for each test cases.

@hichamboushaba I copied DummyInterceptorModule from WCAndroid. It works but I couldn't understand what this actually does. Is @Named("network-interceptors") required for fluxc library? Should I check fluxc documents?

@irfano I need to check, but if I remember correctly the reason we needed this it was just to replace the actual InterceptorModule in tests with a dummy one, so the main thing is:

@TestInstallIn(
        components = [SingletonComponent::class],
        replaces = [InterceptorModule::class]
)

and not the interceptor itself.
I didn't test this, but maybe you can just keep the module empty, without the provided interceptor at all 🤔

Thanks for the explanation.

I learnt that "network-interceptors" is mainly for logging, "interceptors" is for mocking here.
WCAndroid is using Flipper, WPAndroid is using Stetho for network debugging. By adding DummyInterceptorModule, WCAndroid removed Flipper for testing. I couldn't understand the exact reason why do we need to cancel network debugging, but I will also do the same for WPAndroid.

I mean I removed Stetho for testing by adding DummyInterceptorModule.

We remove the logging because it adds an overhead which is not necessary for tests, since the network requests are already mocked, but I don't think it will hurt if it stayed, if it's just pure logging.

It makes sense now. Thanks! 👍🏻

Before this commit, the test was passing successfully. Then activity was finishing while recylerview is still refreshing and then ReaderExpandableTagsView on "Reader" screen was crashing because of unaccessible Hilt objects.

If test was waiting enough for reloading of recyclerview, test was passing. But sometimes it was failing.

Waiting for 2 seconds at confirmSignup() can prevent failing.
I chose navigating "My Sites" screen that also prevent failing.

Just replicating here what I said on Slack, for visibility: I personally think both approaches are fine in this case, but maybe we should add an extra comment there, because it’s not immediately obvious that removing that piece of code could cause the test to fail.

Just replicating here what I said on Slack, for visibility: I personally think both approaches are fine in this case, but maybe we should add an extra comment there, because it’s not immediately obvious that removing that piece of code could cause the test to fail.