corona-warn-app / cwa-app-android

Native Android app using the Apple/Google exposure notification API. The CWA development ends on May 31, 2023. You still can warn other users until April 30, 2023. More information:
https://coronawarn.app/en/faq/#ramp_down
Apache License 2.0
2.44k stars 495 forks source link

Product flavor for using mock exposure notification API #321

Closed pocmo closed 2 years ago

pocmo commented 4 years ago

In the code you mention a plan to add a product flavor that should use a mock exposure notification API: https://github.com/corona-warn-app/cwa-app-android/blob/a326bb4bfc2661b3e5ff8e53a1951b5ad351a395/Corona-Warn-App/build.gradle#L67-L77

Is this something that could be done as a contribution? Looking at InternalExposureNotificationClient it seems like one option could be to inject a different/mock ExposureNotificationClient implementation that returns some dummy values. This at least would allow launching the app without any API support on the device. Did you have more elaborate plans? (e.g. controlling the mocked API via an activity or test app?).

Would be interested in contributing this if possible.


Internal Tracking ID: EXPOSUREAPP-1967

marcmuschko commented 4 years ago

Dear @pocmo,

you are right, we have considered this but due to the tight time constraints and the change frequency within the app and the EN api, we were not able to ensure a properly maintained mock client and had to drop it for now.

This is definitely something that could be done as a contribution and we are interested in having this; it would make testing for collaborators without whitelisting much easier. But we also want to be transparent about the inclusion into this repository when the mock client is developed and tested: Mentioning time constraints and change frequency again, we cannot guarantee that we will be able to merge the feature, as we would have to maintain it. My suggestion is a fork from master with the feature included which would then be independent from the official repository but offer a very nice addition to the project in general.

Knowing this, are you still willing to invest some effort? If so, please let me know and I'll assign you the issue! Perhaps some other colleagues are interested in collaborating together with you? Thanks and best, Marc

pocmo commented 4 years ago

Hey @marcmuschko,

thank you for your reply!

This is definitely something that could be done as a contribution and we are interested in having this; it would make testing for collaborators without whitelisting much easier.

Is whitelisting something that is at all possible for outside contributors?

Mentioning time constraints and change frequency again, we cannot guarantee that we will be able to merge the feature, as we would have to maintain it.

I can fully understand that you are not interested in maintaining this as part of this repository if this is not something that benefits the core team directly or in the worst case adds more work for them.

Nevertheless I wonder if an mock of ExposureNotificationClient that completely lives in a build flavor could be a working compromise? It wouldn't affect the main build flavor and the mock flavor would only be affected if the interface of ExposureNotificationClient changes in the future (which I assume will not be that often since it is a public google play API?). For the mock flavor you could decide to not guarantee any run or compile guarantee and leave that to the community?

My suggestion is a fork from master with the feature included which would then be independent from the official repository but offer a very nice addition to the project in general. Knowing this, are you still willing to invest some effort?

I'm still interested. But a fork outside of this repository would essentially not be visible to anyone interested in this project?

marcmuschko commented 4 years ago

Hi @pocmo,

regarding whitelisting, it's not possible at the moment, our community managers will provide updates here.

I am not saying there is no chance for this addition to be merged if it's fine quality / content wise, but as many things are currently in clarification (e.g. who maintains community-built content after release that is not critical to core functionality but still really nice), it would be unfair to impact your decision to work on this by a "maybe / likely in the future".

Of course I understand that if you provide this feature as a fork, visibility is desirable and also quite beneficial to us. Therefore we could include your fork as a highlighted mention in the readme and pin this issue. Is this something that sounds acceptable for the time being?

pocmo commented 4 years ago

Alright, I'll work in my fork for now and will report back once something interesting comes out of it.

marcmuschko commented 4 years ago

@pocmo thank you very much, sounds great and I'm looking forward to the results! Shall we try to get some additional collaborators on this? I would assume there are others interested in a mock client as well and if you don't mind some helping hands within the fork, I will pin the issue already :)

pocmo commented 4 years ago

I was about to look further into this and noticed that a deviceForTesters product flavor appeared. This one seems to be primarily used for testing with the actual google play API and custom dev backend servers, right?

marcmuschko commented 4 years ago

@pocmo correct, the development menus are also included there to make testing a bit easier. Do you want to provide your fork link? Then I can include it in the readme and the community has a chance to reach out to you for additional help as we receive a lot of issues at the moment and it's hard to keep track - I would not want this one to be overlooked!

pocmo commented 4 years ago

Added a first iteration to my repository: https://github.com/pocmo/cwa-app-android/commit/7043609ea79b8ee062c2e1e6542d4c7391350424

In that patch I changed InternalExposureNotificationClient to delegate the creation of ExposureNotificationClient to a factory object. There are different versions of the factory for the device and deviceForTesters flavors. The device flavor continues to use Nearby.getExposureNotificationClient while the deviceForTesters returns a mock implementation that for now just returns hardcoded values for the API calls.

In further iterations I would like to (only in the deviceForTesters flavor):

heinezen commented 3 years ago

Dear @pocmo ,

Did you pursue this further? Looks like this issue got lost in time :)


Corona-Warn-App Open Source Team

pocmo commented 3 years ago

@heinezen No, not really beyond the initial work to mock the API. Is there an interested to revive this effort? :)

MikeMcC399 commented 3 years ago

I don't know whether it would be worth pursuing this issue further, although in the past I have often wished it had been implemented!

The Android CWA app has become more tester-friendly when using the Build Variant "deviceForTestersDebug" in Android Studio and running with a Google account which has not been allowlisted by Google. It does mean that many aspects which do not rely on a working Google Exposure Notifications System API can be tested without generating multiple errors.

Also the Corona Contact Tracing Germany fork has produced a version which does not depend on Google Play services. (See README.md of CCTG).

heinezen commented 3 years ago

@pocmo I think it's not strictly necessary anymore, as @MikeMcC399 already pointed out, although it would still be nice. But testing the app has become a lot easier already.


Corona-Warn-App Open Source Team

fynngodau commented 3 years ago

Instead of creating a mock API client, this issue could be implemented using the fully functional microG implementation. Pretty much only a few libraries would have to be swapped, which can be done using the build flavor.

svengabr commented 2 years ago

I have just checked the linked Jira issue.

Jira Ticket is flagged as: Resolution: Wont Fix Status: Obsolete

Developer comment:

We have a test flavour but the whole ENF isn't mocked.