Skip to content
This repository has been archived by the owner on Jun 20, 2023. It is now read-only.

Product flavor for using mock exposure notification API #321

Closed
pocmo opened this issue Jun 9, 2020 · 14 comments
Closed

Product flavor for using mock exposure notification API #321

pocmo opened this issue Jun 9, 2020 · 14 comments
Assignees
Labels
community Tag issues created by community members enhancement Improvement of an existing feature help wanted Extra attention is needed mirrored-to-jira This item is also tracked internally in JIRA

Comments

@pocmo
Copy link

pocmo commented Jun 9, 2020

In the code you mention a plan to add a product flavor that should use a mock exposure notification API:

// https://developer.android.com/studio/build/build-variants
// We use flavors to build different versions of the app. One version which uses
// the mock ExposureNotification API and one for the real Google API
flavorDimensions "version"
productFlavors {
device {
dimension "version"
buildConfigField "String", "BUILD_VARIANT", "\"device\""
resValue "string", "app_name", "Corona-Warn"
}
}

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

@pocmo pocmo added the enhancement Improvement of an existing feature label Jun 9, 2020
@marcmuschko
Copy link
Contributor

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
Copy link
Author

pocmo commented Jun 10, 2020

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
Copy link
Contributor

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
Copy link
Author

pocmo commented Jun 10, 2020

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

@marcmuschko marcmuschko added the community Tag issues created by community members label Jun 10, 2020
@marcmuschko
Copy link
Contributor

@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
Copy link
Author

pocmo commented Jun 16, 2020

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
Copy link
Contributor

marcmuschko commented Jun 17, 2020

@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!

@marcmuschko marcmuschko added the help wanted Extra attention is needed label Jun 17, 2020
pocmo added a commit to pocmo/cwa-app-android that referenced this issue Jun 18, 2020
@pocmo
Copy link
Author

pocmo commented Jun 18, 2020

Added a first iteration to my repository:
pocmo@7043609

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):

  • Introduce a test fragment (like you already have for testing the API and the risk level) that allows configuring what the API should return (e.g. values in ExposureSummary)
  • Persist the values (e.g. SharedPreferences) to make them stable across app restarts and for background tasks.
  • Provide a toggle in the test fragment to switch between using the actual API and the mock API.

@MarlisFriedl MarlisFriedl added the mirrored-to-jira This item is also tracked internally in JIRA label Jul 28, 2020
jakobmoellerdev added a commit that referenced this issue Aug 7, 2020
added error handling for risk level transaction
@svengabr svengabr moved this from Initial_OLD to Initial in [CM] cwa-app-android Nov 19, 2020
@heinezen heinezen moved this from Initial to Mirrored to Jira in [CM] cwa-app-android Jan 12, 2021
@heinezen
Copy link
Member

heinezen commented May 9, 2021

Dear @pocmo ,

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


Corona-Warn-App Open Source Team

@pocmo
Copy link
Author

pocmo commented May 12, 2021

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

@cwa-bot cwa-bot bot moved this from Mirrored to Jira to ToDo in [CM] cwa-app-android May 12, 2021
@MikeMcC399
Copy link
Contributor

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 heinezen moved this from ToDo to Mirrored to Jira in [CM] cwa-app-android May 16, 2021
@heinezen heinezen moved this from Mirrored to Jira to ToDo in [CM] cwa-app-android May 16, 2021
@heinezen
Copy link
Member

@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

@heinezen heinezen moved this from ToDo to Mirrored to Jira in [CM] cwa-app-android May 16, 2021
@fynngodau
Copy link
Contributor

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.

@cwa-bot cwa-bot bot moved this from Mirrored to Jira to ToDo in [CM] cwa-app-android Jul 7, 2021
@heinezen heinezen moved this from ToDo to Mirrored to Jira in [CM] cwa-app-android Jul 8, 2021
@svengabr
Copy link
Member

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
community Tag issues created by community members enhancement Improvement of an existing feature help wanted Extra attention is needed mirrored-to-jira This item is also tracked internally in JIRA
Development

No branches or pull requests

8 participants