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

Fix external apps returning data when Collect is being restored #5663

Merged
merged 35 commits into from
Aug 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
6d3c251
Make sure process restore is simulated correctly
seadowg Aug 7, 2023
9277b64
Spike out being able to return external data after a process restore
seadowg Aug 7, 2023
1f2218d
Stop restores loading first question instead of hierarchy if it was p…
seadowg Aug 8, 2023
65a776a
Add Robolectric implementation of process restore simulation
seadowg Aug 9, 2023
db2b093
Replace instrumentation with local test
seadowg Aug 9, 2023
a40450e
Duplicate class and remove commonTest
seadowg Aug 9, 2023
c38bdba
Move test forms to dedicated module
seadowg Aug 9, 2023
9d66c99
Integrate test file utils into main set
seadowg Aug 10, 2023
c0211ce
Use real form for local process restore test
seadowg Aug 10, 2023
8d6f934
Correct recreating simulation
seadowg Aug 10, 2023
f03335f
Replace process restore with DialogFragment instrumentation test with…
seadowg Aug 10, 2023
873b650
Migrate FormLoaderTask to use Scheduler
seadowg Aug 10, 2023
41bc7bc
Use paused scheduler for FormFillingActiivtyTest
seadowg Aug 10, 2023
6454364
Make tests more specific
seadowg Aug 11, 2023
fc9629b
Pull out common helpers
seadowg Aug 11, 2023
6f0aefe
Add test for returning from hierarchy
seadowg Aug 11, 2023
33ef83a
Pull out helpers for Espresso
seadowg Aug 11, 2023
3e2b773
Remove shadows from FormFillingAcitivity
seadowg Aug 11, 2023
a5ddbf8
Move extension to shared module
seadowg Aug 11, 2023
3351c2c
Pull out shared Espresso helpers
seadowg Aug 11, 2023
116ef1c
Add Espresso helper for asserting on intents
seadowg Aug 11, 2023
6f584b2
Remove unused assertion
seadowg Aug 11, 2023
8b7f249
Improve intent assertion
seadowg Aug 11, 2023
4444a92
Add test for restoring and returning data
seadowg Aug 11, 2023
84da432
Correct lifecycle for process restore from external app
seadowg Aug 14, 2023
97f2a68
Ad helper for asserting on new intents
seadowg Aug 14, 2023
9d6da63
Add helper for restoring with result
seadowg Aug 14, 2023
6006029
Use receiveResult instead of protected method
seadowg Aug 14, 2023
385ebaf
Avoid ActivityController mem leaks
seadowg Aug 15, 2023
b8b6808
Fix java version
seadowg Aug 25, 2023
b733ead
Remove unused method
seadowg Aug 25, 2023
4181ce8
Remove no longer needed test dummy
seadowg Aug 25, 2023
174a290
Inline settings setup in androidTest
seadowg Aug 25, 2023
dee2561
Remove unused method
seadowg Aug 25, 2023
8985e19
Move form media files to test-forms module
seadowg Aug 25, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
package org.odk.collect.async

import android.os.AsyncTask

/**
* Basic reimplementation of the [AsyncTask] API that allows an [AsyncTask] implementation to
* use [Scheduler] with minimal internal and external changes.
*/
abstract class SchedulerAsyncTaskMimic<Params, Progress, Result>(private val scheduler: Scheduler) {

@Volatile
private var status: AsyncTask.Status = AsyncTask.Status.PENDING

@Volatile
private var cancelled = false

protected abstract fun onPreExecute()
protected abstract fun doInBackground(vararg params: Params): Result
protected abstract fun onProgressUpdate(vararg values: Progress)
protected abstract fun onPostExecute(result: Result)
protected abstract fun onCancelled()

/**
* Execute [doInBackground] on calling thread and return the [Result] value. Should probably
* not be used as a replacement for [AsyncTask.get] (unless it's for testing purposes).
*/
fun executeSynchronously(vararg params: Params): Result {
return doInBackground(*params)
}

fun execute(vararg params: Params): SchedulerAsyncTaskMimic<Params, Progress, Result> {
status = AsyncTask.Status.RUNNING
onPreExecute()

scheduler.immediate(
background = {
doInBackground(*params)
},
foreground = { result ->
if (cancelled) {
onCancelled()
} else {
onPostExecute(result)
}

status = AsyncTask.Status.FINISHED
}
)

return this
}

fun getStatus(): AsyncTask.Status {
return status
}

/**
* Unlike [AsyncTask.cancel], this does not offer the option to attempt to interrupt the
* background thread running [doInBackground]. Calling [cancel] will allow [doInBackground]
* to finish, but will prevent [onPostExecute] from running ([onCancelled] will be run
* instead).
*/
fun cancel() {
cancelled = true
}

fun isCancelled(): Boolean {
return cancelled
}

protected fun publishProgress(vararg values: Progress) {
scheduler.immediate(
foreground = { onProgressUpdate(*values) }
)
}
}
11 changes: 2 additions & 9 deletions collect_app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -200,15 +200,6 @@ android {
}
}


sourceSets {
androidTest {
java.srcDirs += "src/commonTest/java"
}
test {
java.srcDirs += "src/commonTest/java"
}
}
lint {
abortOnError true
checkDependencies true
Expand Down Expand Up @@ -373,6 +364,7 @@ dependencies {
exclude group: 'org.robolectric' // Some tests in `collect_app` don't work with newer Robolectric
}
testImplementation(project(":shadows"))
testImplementation(project(":test-forms"))

testImplementation Dependencies.robolectric

Expand All @@ -389,6 +381,7 @@ dependencies {
testImplementation Dependencies.androidx_test_core_ktx

androidTestImplementation project(':androidtest')
androidTestImplementation project(':test-forms')
seadowg marked this conversation as resolved.
Show resolved Hide resolved

androidTestImplementation Dependencies.mockito_android
androidTestImplementation Dependencies.androidx_test_ext_junit
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package org.odk.collect.android.feature.formentry;

import static org.odk.collect.android.support.FileUtils.copyFileFromAssets;
import static org.odk.collect.android.utilities.FileUtils.copyFileFromResources;

import android.app.Application;

Expand All @@ -10,12 +10,12 @@
import org.junit.Test;
import org.junit.rules.RuleChain;
import org.junit.runner.RunWith;
import org.odk.collect.android.support.rules.CollectTestRule;
import org.odk.collect.android.support.TestDependencies;
import org.odk.collect.android.support.rules.TestRuleChain;
import org.odk.collect.android.support.pages.FormEntryPage;
import org.odk.collect.android.support.pages.MainMenuPage;
import org.odk.collect.android.support.pages.OkDialog;
import org.odk.collect.android.support.rules.CollectTestRule;
import org.odk.collect.android.support.rules.TestRuleChain;
import org.odk.collect.audiorecorder.recording.AudioRecorder;
import org.odk.collect.audiorecorder.testsupport.StubAudioRecorder;

Expand All @@ -35,7 +35,7 @@ public AudioRecorder providesAudioRecorder(Application application) {
File stubRecording = File.createTempFile("test", ".m4a");
stubRecording.deleteOnExit();

copyFileFromAssets("media/test.m4a", stubRecording.getAbsolutePath());
copyFileFromResources("media/test.m4a", stubRecording.getAbsolutePath());
stubAudioRecorderViewModel = new StubAudioRecorder(stubRecording.getAbsolutePath());
} catch (IOException e) {
throw new RuntimeException(e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.CoreMatchers.nullValue;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.odk.collect.android.support.FileUtils.copyFileFromAssets;
import static org.odk.collect.android.utilities.FileUtils.copyFileFromResources;

import android.Manifest;
import android.app.Activity;
Expand Down Expand Up @@ -56,7 +56,7 @@ public AudioRecorder providesAudioRecorder(Application application) {
File stubRecording = File.createTempFile("test", ".m4a");
stubRecording.deleteOnExit();

copyFileFromAssets("media/test.m4a", stubRecording.getAbsolutePath());
copyFileFromResources("media/test.m4a", stubRecording.getAbsolutePath());
stubAudioRecorderViewModel = new StubAudioRecorder(stubRecording.getAbsolutePath());
} catch (IOException e) {
throw new RuntimeException(e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import static androidx.test.espresso.intent.Intents.intending;
import static androidx.test.espresso.intent.matcher.IntentMatchers.hasAction;
import static org.odk.collect.android.support.FileUtils.copyFileFromAssets;
import static org.odk.collect.android.utilities.FileUtils.copyFileFromResources;

import android.app.Activity;
import android.app.Instrumentation;
Expand Down Expand Up @@ -39,7 +39,7 @@ public class ExternalAudioRecordingTest {
try {
File stubRecording = File.createTempFile("test", ".m4a");
stubRecording.deleteOnExit();
copyFileFromAssets("media/test.m4a", stubRecording.getAbsolutePath());
copyFileFromResources("media/test.m4a", stubRecording.getAbsolutePath());

Intent intent = new Intent();
intent.setData(Uri.fromFile(stubRecording));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import static org.odk.collect.android.support.matchers.CustomMatchers.withIndex;

import android.app.Activity;
import android.app.Application;
import android.app.Instrumentation;
import android.graphics.Bitmap;
import android.graphics.BitmapFactory;
Expand All @@ -60,7 +61,7 @@
import org.junit.Test;
import org.junit.rules.RuleChain;
import org.odk.collect.android.R;
import org.odk.collect.android.TestSettingsProvider;
import org.odk.collect.android.injection.DaggerUtils;
import org.odk.collect.android.preferences.GuidanceHint;
import org.odk.collect.android.storage.StoragePathProvider;
import org.odk.collect.android.support.pages.FormEntryPage;
Expand Down Expand Up @@ -323,7 +324,11 @@ public void questionsAppearingBeforeCurrentBinaryQuestion_ShouldNotChangeFocus()

@Test
public void changeInValueUsedInGuidanceHint_ShouldChangeGuidanceHintText() {
TestSettingsProvider.getUnprotectedSettings().save(ProjectKeys.KEY_GUIDANCE_HINT, GuidanceHint.YES.toString());
DaggerUtils.getComponent(ApplicationProvider.<Application>getApplicationContext())
.settingsProvider()
.getUnprotectedSettings()
.save(ProjectKeys.KEY_GUIDANCE_HINT, GuidanceHint.YES.toString());

jumpToGroupWithText("Guidance hint");
onView(withText(startsWith("Source11"))).perform(click());

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,16 @@
import static androidx.test.espresso.matcher.ViewMatchers.withText;
import static org.hamcrest.CoreMatchers.not;

import android.app.Application;

import androidx.test.core.app.ApplicationProvider;
import androidx.test.espresso.matcher.ViewMatchers;

import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.RuleChain;
import org.odk.collect.android.R;
import org.odk.collect.android.TestSettingsProvider;
import org.odk.collect.android.injection.DaggerUtils;
import org.odk.collect.android.preferences.GuidanceHint;
import org.odk.collect.android.support.rules.BlankFormTestRule;
import org.odk.collect.android.support.rules.TestRuleChain;
Expand All @@ -36,7 +39,11 @@ public void guidanceHint_ShouldBeHiddenByDefault() {

@Test
public void guidanceHint_ShouldBeDisplayedWhenSettingSetToYes() {
TestSettingsProvider.getUnprotectedSettings().save(ProjectKeys.KEY_GUIDANCE_HINT, GuidanceHint.YES.toString());
DaggerUtils.getComponent(ApplicationProvider.<Application>getApplicationContext())
.settingsProvider()
.getUnprotectedSettings()
.save(ProjectKeys.KEY_GUIDANCE_HINT, GuidanceHint.YES.toString());

// jump to force recreation of the view after the settings change
onView(withId(R.id.menu_goto)).perform(click());
onView(withId(R.id.jumpBeginningButton)).perform(click());
Expand All @@ -46,7 +53,11 @@ public void guidanceHint_ShouldBeDisplayedWhenSettingSetToYes() {

@Test
public void guidanceHint_ShouldBeDisplayedAfterClickWhenSettingSetToYesCollapsed() {
TestSettingsProvider.getUnprotectedSettings().save(ProjectKeys.KEY_GUIDANCE_HINT, GuidanceHint.YES_COLLAPSED.toString());
DaggerUtils.getComponent(ApplicationProvider.<Application>getApplicationContext())
.settingsProvider()
.getUnprotectedSettings()
.save(ProjectKeys.KEY_GUIDANCE_HINT, GuidanceHint.YES_COLLAPSED.toString());

// jump to force recreation of the view after the settings change
onView(withId(R.id.menu_goto)).perform(click());
onView(withId(R.id.jumpBeginningButton)).perform(click());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import static org.odk.collect.android.support.matchers.CustomMatchers.withIndex;
import static org.odk.collect.android.support.FileUtils.copyFileFromAssets;
import static org.odk.collect.android.utilities.FileUtils.copyFileFromResources;

import android.app.Activity;
import android.app.Instrumentation;
Expand Down Expand Up @@ -251,7 +251,7 @@ private Uri createTempFile(String name, String extension) throws IOException {
.getExternalFilesDir(Environment.DIRECTORY_DOWNLOADS);

File file = File.createTempFile(name, extension, downloadsDir);
copyFileFromAssets("media" + File.separator + name + "." + extension, file.getPath());
copyFileFromResources("media" + File.separator + name + "." + extension, file.getPath());
return getUriForFile(file);
}

Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import org.junit.Rule
import org.junit.Test
import org.junit.rules.RuleChain
import org.junit.runner.RunWith
import org.odk.collect.android.support.CollectHelpers
import org.odk.collect.android.support.StorageUtils
import org.odk.collect.android.support.pages.AppClosedPage
import org.odk.collect.android.support.pages.FormEntryPage
Expand Down Expand Up @@ -223,18 +222,15 @@ class SavePointTest {
* being battery dying).
*/
private fun simulateBatteryDeath(): FormEntryActivityTestRule {
CollectHelpers.simulateProcessRestart()
return rule
return rule.simulateProcessRestart()
}

/**
* Simulate a "process death" case where an app in the background is killed
*/
private fun simulateProcessDeath(): FormEntryActivityTestRule {
rule.navigateAwayFromActivity()
return rule.navigateAwayFromActivity()
.destroyActivity()

CollectHelpers.simulateProcessRestart()
return rule
.simulateProcessRestart()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@

import static junit.framework.Assert.assertEquals;

import static org.mockito.Mockito.mock;

import android.app.Application;

import androidx.test.core.app.ApplicationProvider;
Expand Down Expand Up @@ -129,7 +131,7 @@ private void testIndices(String formName, String[] expectedIndices) throws Execu
Timber.i(e);
}

FormLoaderTask formLoaderTask = new FormLoaderTask(formPath(formName), null, null, formEntryControllerFactory);
FormLoaderTask formLoaderTask = new FormLoaderTask(formPath(formName), null, null, formEntryControllerFactory, mock());
formLoaderTask.setFormLoaderListener(new FormLoaderListener() {
@Override
public void loadingComplete(FormLoaderTask task, FormDef fd, String warningMsg) {
Expand Down Expand Up @@ -162,7 +164,7 @@ public void onProgressStep(String stepMessage) {

}
});
formLoaderTask.execute(formPath(formName)).get();
formLoaderTask.executeSynchronously(formPath(formName));
}

/**
Expand Down