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
Prevent crashes in form filling when restoring process #5602
Conversation
@@ -418,7 +423,14 @@ public void onCreate(Bundle savedInstanceState) { | |||
.forClass(SelectOneFromMapDialogFragment.class, () -> new SelectOneFromMapDialogFragment(viewModelFactory)) | |||
.build()); | |||
|
|||
savedInstanceState = savedInstanceStateProvider.getState(savedInstanceState); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't feel great about this solution of forcing in a provider for savedInstanceState
, but there's no existing way to start an Activity with previous state using ActivityScenario
, and this is the best I could come up with (without forking AndroidX Test).
It currently is possible to write pure Robolectric code that that creates an Acivity with previous state so it should be possible to add that to ActivityScenario
for "local" tests (that under the hood use the Robolectric backed LocalActivityInvoker
), but I don't have enough knowledge of Android internals to know if it'd be doable for instrumentation tests. I'll file an issue over at AndroidX Test for this (and update this comment with a link).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Link to issue: android/android-test#1825.
@dbemke Tested this PR earlier this week. Tested with Success! Verified on device with Android: 10,12,13 Verified Cases:
|
There's a few problems with ActivityScenario for the way the test was previously written: - `recreate()` requires Activity to recreate to `RESUMED` state - No way to set/clear state between `onSaveInstanceState` and `onCreate` when calling `recreate()` - No way to start an Activity with `savedInstanceState`
Closes #5240
As described in the issue:
What has been done to verify that this works as intended?
New tests.
Why is this the best possible solution? Were any other approaches considered?
There's probably a bunch of other tricks we could use here, but I'm pretty happy with what I've gone for: write a UUID out to the
outState
and to application level state duringonSaveInstanceState
, and then compare later to determine whether the Activity is being recreated within the same or a new process.I'm feeling pretty unhappy with some of the code required to test this, but I've left a comment about that inline.
How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?
As well as checking the issue is resolved, it'd be good to test filling/editing and resuming (from save points) forms, forms with audit logs and the form hierarchy.
Before submitting this PR, please make sure you have:
./gradlew checkAll
and confirmed all checks still pass OR confirm CircleCI build passes and run./gradlew connectedDebugAndroidTest
locally.