Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FEATURE REQUEST] Move to AndroidX Preference and new structure for settings #2867

Closed
jesmrec opened this issue Apr 27, 2020 · 4 comments · Fixed by #3143
Closed

[FEATURE REQUEST] Move to AndroidX Preference and new structure for settings #2867

jesmrec opened this issue Apr 27, 2020 · 4 comments · Fixed by #3143

Comments

@jesmrec
Copy link
Collaborator

jesmrec commented Apr 27, 2020

Current implementation of Settings is deprecated. So, it is recommended to move to AndroidX Preferences.

Move to Android X implementation, fix flaky tests and check viability #2651

This issue is an epic, that will have several individual issues attached to move every section of the Settings to the new implementation.

Every implemented section will be merged to a branch, that will be merged into master when all the work is finished, to avoid bad intermediate statuses.

Additional context

Issues list

PR:

@jesmrec jesmrec added this to the 3.0-next milestone Apr 27, 2020
@abelgardep abelgardep mentioned this issue Jun 9, 2020
9 tasks
@jesmrec jesmrec modified the milestones: 2.16-current, 2.17-next Jun 12, 2020
@abelgardep abelgardep changed the title [FEATURE REQUEST] Migrate Settings to new implementation [FEATURE REQUEST] Move to AndroidX Preference Dec 18, 2020
@jesmrec jesmrec modified the milestones: 2.17-next, 2.18 Jan 14, 2021
@jesmrec jesmrec added the Epic label Feb 12, 2021
@jesmrec jesmrec removed this from the 2.18-next milestone Feb 18, 2021
@JuancaG05 JuancaG05 linked a pull request Mar 10, 2021 that will close this issue
5 tasks
@abelgardep abelgardep added this to the 2.18-next milestone Mar 10, 2021
@JuancaG05
Copy link
Collaborator

Due to a Google's intern bug, lint is not passing with Fragment version 1.3.1, showing the following error:
/bitrise/src/owncloudApp/src/main/java/com/owncloud/android/presentation/ui/settings/fragments/SettingsFragment.kt:72: Error: Upgrade Fragment version to at least 1.3.0. [InvalidFragmentVersionForActivityResult] registerForActivityResult(ActivityResultContracts.StartActivityForResult()) { result -> ^

To temporary fix it, we have added a new line to lint.xml:
<issue id="InvalidFragmentVersionForActivityResult" severity="ignore" />

The issue will be fixed in the next release, so we MUST remember to remove that line before merging with master, checking whether the new Activity release took place

Link to Google's issue: https://issuetracker.google.com/issues/182388985

@JuancaG05
Copy link
Collaborator

JuancaG05 commented Apr 20, 2021

Regarding some flaky tests related with settings and some activities that we use inside of them, I found the error when the process crashes and BitRise or the "Run" log in AndroidStudio don't give more information than "Process crashed". Looking in the Logcat tab, we can see that the root error that occurs when some of these flaky tests fail (they fail approximately 80% of times, so it's not worth it) is:

E/ModuleIdSetter: exception when setting module id java.lang.IllegalStateException: Unable to get current module info in ModuleManager created with non-module Context at com.google.android.chimera.config.ModuleManager.getCurrentModule(:com.google.android.gms@[email protected] (120700-361652764):1) at agze.a(:com.google.android.gms@[email protected] (120700-361652764):4) at agzh.aa(:com.google.android.gms@[email protected] (120700-361652764):1) at agvq.a(Unknown Source:3) at tbp.a(:com.google.android.gms@[email protected] (120700-361652764):0) at sxu.f(:com.google.android.gms@[email protected] (120700-361652764):1) at sxs.e(:com.google.android.gms@[email protected] (120700-361652764):1) at tam.p(:com.google.android.gms@[email protected] (120700-361652764):2) at tam.v(:com.google.android.gms@[email protected] (120700-361652764):3) at tam.e(:com.google.android.gms@[email protected] (120700-361652764):2) at taq.handleMessage(:com.google.android.gms@[email protected] (120700-361652764):70) at android.os.Handler.dispatchMessage(Handler.java:103) at agom.iE(:com.google.android.gms@[email protected] (120700-361652764):0) at agom.dispatchMessage(:com.google.android.gms@[email protected] (120700-361652764):10) at android.os.Looper.loop(Looper.java:214) at android.os.HandlerThread.run(HandlerThread.java:67)

Looking for some more info, I found it may be caused by a try to access system services before the execution of onCreate (https://stackoverflow.com/a/64349408), but revising the code, I would say that every invocation to getSystemService is done after onCreate. What is clear is that here there's some kind of race condition and that's why tests fail sometimes but not always.
I couldn't find the way to solve it, so maybe an option is to keep the flaky tests ignored and open an issue in the future to take a deeper look and research more about this.

Tested in Nexus 5 (API 29)

UPDATE: this was already solved. The problem was that some of the tests that were executed previously to the ones where this exception was thrown drove to the login screen, and since in the device where they are being executed has not an attached account, the device crashed.

@JuancaG05
Copy link
Collaborator

JuancaG05 commented Apr 27, 2021

The final state of the UI tests after the attempt to fix flakiness is the next:
All non-ignored tests pass currently, but we can highlight four of them:

  • In SettingsMoreFragmentTest, recommendOpensSender and feedbackOpensSender have been added a little delay so that the sender that is opened in those tests doesn't interfere with the subsequent tests. The conclusion I've arrived to is that it depends on the device's performance where tests are being executed, and in the case of BitRise's device used for UI tests, it does interfere.
  • In SettingsPictureUploadsFragmentTest and SettingsVideoUploadsFragmentTest, openPictureUploadSourcePathPicker and openVideoUploadSourcePathPicker respectively, have been ignored, since the LocalFolderPickerActivity they drive to, drives to the login screen at the same time. Since in the device in which we execute the tests we don't have an attached account, this will make it crash. However, this picker will have to be replaced with Android's native picker, so we decided to keep them in order to know how to test it and have a base in the future.

Everything I wrote here is also properly documented in the code.

@JuancaG05 JuancaG05 changed the title [FEATURE REQUEST] Move to AndroidX Preference [FEATURE REQUEST] Move to AndroidX Preference and new structure for settings Apr 28, 2021
@abelgardep
Copy link
Contributor

Ready to 2.18

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants