diff --git a/src/com/android/settings/search/Index.java b/src/com/android/settings/search/Index.java index ccedd627c44..bff841b1ee5 100644 --- a/src/com/android/settings/search/Index.java +++ b/src/com/android/settings/search/Index.java @@ -110,6 +110,9 @@ public class Index { public static final String ENTRIES_SEPARATOR = "|"; + static final String FIELD_NAME_SEARCH_INDEX_DATA_PROVIDER = + "SEARCH_INDEX_DATA_PROVIDER"; + // If you change the order of columns here, you SHOULD change the COLUMN_INDEX_XXX values private static final String[] SELECT_COLUMNS = new String[] { IndexColumns.DATA_RANK, // 0 @@ -155,9 +158,6 @@ public class Index { private static final String HYPHEN = "-"; private static final String SPACE = " "; - private static final String FIELD_NAME_SEARCH_INDEX_DATA_PROVIDER = - "SEARCH_INDEX_DATA_PROVIDER"; - private static final String NODE_NAME_PREFERENCE_SCREEN = "PreferenceScreen"; private static final String NODE_NAME_CHECK_BOX_PREFERENCE = "CheckBoxPreference"; private static final String NODE_NAME_LIST_PREFERENCE = "ListPreference"; diff --git a/tests/robotests/assets/grandfather_not_implementing_index_provider b/tests/robotests/assets/grandfather_not_implementing_index_provider new file mode 100644 index 00000000000..dd80440ffd1 --- /dev/null +++ b/tests/robotests/assets/grandfather_not_implementing_index_provider @@ -0,0 +1,13 @@ +com.android.settings.connecteddevice.ConnectedDeviceDashboardFragment +com.android.settings.gestures.PickupGestureSettings +com.android.settings.notification.ConfigureNotificationSettings +com.android.settings.language.LanguageAndRegionSettings +com.android.settings.accounts.UserAndAccountDashboardFragment +com.android.settings.gestures.DoubleTapScreenSettings +com.android.settings.network.NetworkDashboardFragment +com.android.settings.applications.AppAndNotificationDashboardFragment +com.android.settings.gestures.SwipeToNotificationSettings +com.android.settings.notification.ZenModePrioritySettings +com.android.settings.gestures.DoubleTapPowerSettings +com.android.settings.inputmethod.InputAndGestureSettings +com.android.settings.gestures.DoubleTwistGestureSettings \ No newline at end of file diff --git a/tests/robotests/assets/grandfather_not_implementing_indexable b/tests/robotests/assets/grandfather_not_implementing_indexable new file mode 100644 index 00000000000..c1b8cf52b48 --- /dev/null +++ b/tests/robotests/assets/grandfather_not_implementing_indexable @@ -0,0 +1,90 @@ +com.android.settings.location.LocationMode +com.android.settings.notification.ZenModeVisualInterruptionSettings +com.android.settings.accessibility.ToggleScreenMagnificationPreferenceFragment +com.android.settings.deviceinfo.SimStatus +com.android.settings.deviceinfo.PrivateVolumeForget +com.android.settings.inputmethod.SpellCheckersSettings +com.android.settings.inputmethod.KeyboardLayoutPickerFragment +com.android.settings.notification.ZenModeEventRuleSettings +com.android.settings.fuelgauge.InactiveApps +com.android.settings.accessibility.CaptionPropertiesFragment +com.android.settings.accounts.ManageAccountsSettings +com.android.settings.accessibility.AccessibilitySettingsForSetupWizard +com.android.settings.deviceinfo.ImeiInformation +com.android.settings.datausage.DataUsageList +com.android.settings.vpn2.AppManagementFragment +com.android.settings.display.NightDisplaySettings +com.android.settings.vpn2.VpnSettings +com.android.settings.fingerprint.FingerprintSettings$FingerprintSettingsFragment +com.android.settings.applications.ProcessStatsDetail +com.android.settings.wifi.WifiInfo +com.android.settings.applications.VrListenerSettings +com.android.settings.nfc.PaymentSettings +com.android.settings.inputmethod.VirtualKeyboardFragment +com.android.settings.bluetooth.DevicePickerFragment +com.android.settings.inputmethod.UserDictionaryList +com.android.settings.deviceinfo.Status +com.android.settings.datausage.DataSaverSummary +com.android.settings.notification.ChannelNotificationSettings +com.android.settings.datausage.AppDataUsage +com.android.settings.accessibility.FontSizePreferenceFragmentForSetupWizard +com.android.settings.inputmethod.PhysicalKeyboardFragment +com.android.settings.applications.ManageDomainUrls +com.android.settings.applications.WriteSettingsDetails +com.android.settings.location.LocationSettings +com.android.settings.applications.ProcessStatsSummary +com.android.settings.users.RestrictedProfileSettings +com.android.settings.accounts.ChooseAccountActivity +com.android.settings.accounts.ManagedProfileSettings +com.android.settings.notification.ZenModeAutomationSettings +com.android.settings.accessibility.ToggleAutoclickPreferenceFragment +com.android.settings.applications.AppLaunchSettings +com.android.settings.fuelgauge.BatterySaverSettings +com.android.settings.location.ScanningSettings +com.android.settings.tts.TextToSpeechSettings +com.android.settings.applications.ProcessStatsUi +com.android.settings.fuelgauge.PowerUsageDetail +com.android.settings.notification.ZenModeScheduleRuleSettings +com.android.settings.datausage.BillingCycleSettings +com.android.settings.notification.NotificationStation +com.android.settings.print.PrintJobSettingsFragment +com.android.settings.applications.SpecialAccessSettings +com.android.settings.accessibility.ToggleScreenReaderPreferenceFragmentForSetupWizard +com.android.settings.accounts.AccountSyncSettings +com.android.settings.notification.RedactionInterstitial$RedactionInterstitialFragment +com.android.settings.inputmethod.InputMethodAndSubtypeEnabler +com.android.settings.inputmethod.AvailableVirtualKeyboardFragment +com.android.settings.applications.DrawOverlayDetails +com.android.settings.tts.TtsEngineSettingsFragment +com.android.settings.backup.ToggleBackupSettingFragment +com.android.settings.users.UserDetailsSettings +com.android.settings.datausage.UnrestrictedDataAccess +com.android.settings.accessibility.ToggleScreenMagnificationPreferenceFragmentForSetupWizard +com.android.settings.fuelgauge.BatteryHistoryDetail +com.android.settings.fuelgauge.PowerUsageSummary +com.android.settings.applications.RunningServices +com.android.settings.wifi.p2p.WifiP2pSettings +com.android.settings.applications.ManageAssist +com.android.settings.applications.ConfirmConvertToFbe +com.android.settings.deviceinfo.PublicVolumeSettings +com.android.settings.applications.InstalledAppDetails +com.android.settings.accessibility.ToggleAccessibilityServicePreferenceFragment +com.android.settings.print.PrintServiceSettingsFragment +com.android.settings.wfd.WifiDisplaySettings +com.android.settings.notification.AppNotificationSettings +com.android.settings.deviceinfo.PrivateVolumeSettings +com.android.settings.users.AppRestrictionsFragment +com.android.settings.deviceinfo.PrivateVolumeUnmount +com.android.settings.deletionhelper.AutomaticStorageManagerSettings +com.android.settings.notification.ZenAccessSettings +com.android.settings.accessibility.ToggleFontSizePreferenceFragment +com.android.settings.accessibility.ToggleGlobalGesturePreferenceFragment +com.android.settings.wifi.ConfigureWifiSettings +com.android.settings.wifi.AdvancedWifiSettings +com.android.settings.applications.PremiumSmsAccess +com.android.settings.applications.UsageAccessDetails +com.android.settings.applications.AppStorageSettings +com.android.settings.notification.NotificationAccessSettings +com.android.settings.notification.ZenModeSettings +com.android.settings.accessibility.ToggleDaltonizerPreferenceFragment +com.android.settings.applications.ConvertToFbe \ No newline at end of file diff --git a/tests/robotests/assets/grandfather_not_implementing_instrumentable b/tests/robotests/assets/grandfather_not_implementing_instrumentable new file mode 100644 index 00000000000..5bd96affec5 --- /dev/null +++ b/tests/robotests/assets/grandfather_not_implementing_instrumentable @@ -0,0 +1,4 @@ +com.android.settings.deletionhelper.ActivationWarningFragment +com.android.settings.core.lifecycle.ObservableDialogFragment +com.android.settings.applications.AppOpsCategory +com.android.settings.inputmethod.UserDictionaryLocalePicker diff --git a/tests/robotests/src/com/android/settings/SettingsRobolectricTestRunner.java b/tests/robotests/src/com/android/settings/SettingsRobolectricTestRunner.java index d6d69632563..9127d5f94a4 100644 --- a/tests/robotests/src/com/android/settings/SettingsRobolectricTestRunner.java +++ b/tests/robotests/src/com/android/settings/SettingsRobolectricTestRunner.java @@ -15,7 +15,6 @@ */ package com.android.settings; -import java.util.List; import org.junit.runners.model.InitializationError; import org.robolectric.RobolectricTestRunner; import org.robolectric.annotation.Config; @@ -23,6 +22,8 @@ import org.robolectric.manifest.AndroidManifest; import org.robolectric.res.Fs; import org.robolectric.res.ResourcePath; +import java.util.List; + /** * Custom test runner for the testing of BluetoothPairingDialogs. This is needed because the * default behavior for robolectric is just to grab the resource directory in the target package. @@ -47,7 +48,7 @@ public class SettingsRobolectricTestRunner extends RobolectricTestRunner { final String appRoot = "packages/apps/Settings"; final String manifestPath = appRoot + "/AndroidManifest.xml"; final String resDir = appRoot + "/res"; - final String assetsDir = appRoot + "/assets"; + final String assetsDir = appRoot + config.assetDir(); // By adding any resources from libraries we need to the AndroidManifest, we can access // them from within the parallel universe's resource loader. diff --git a/tests/robotests/src/com/android/settings/core/codeinspection/CodeInspectionTest.java b/tests/robotests/src/com/android/settings/core/codeinspection/CodeInspectionTest.java index 88d817150c1..4aa576f611c 100644 --- a/tests/robotests/src/com/android/settings/core/codeinspection/CodeInspectionTest.java +++ b/tests/robotests/src/com/android/settings/core/codeinspection/CodeInspectionTest.java @@ -19,6 +19,7 @@ package com.android.settings.core.codeinspection; import com.android.settings.SettingsRobolectricTestRunner; import com.android.settings.TestConfig; import com.android.settings.core.instrumentation.InstrumentableFragmentCodeInspector; +import com.android.settings.search.SearchIndexProviderCodeInspector; import org.junit.Before; import org.junit.Test; @@ -32,7 +33,8 @@ import java.util.List; * for conformance. */ @RunWith(SettingsRobolectricTestRunner.class) -@Config(manifest = TestConfig.MANIFEST_PATH, sdk = TestConfig.SDK_VERSION) +@Config(manifest = TestConfig.MANIFEST_PATH, sdk = TestConfig.SDK_VERSION, + assetDir = "/tests/robotests/assets") public class CodeInspectionTest { private List> mClasses; @@ -45,5 +47,6 @@ public class CodeInspectionTest { @Test public void runCodeInspections() { new InstrumentableFragmentCodeInspector(mClasses).run(); + new SearchIndexProviderCodeInspector(mClasses).run(); } } diff --git a/tests/robotests/src/com/android/settings/core/codeinspection/CodeInspector.java b/tests/robotests/src/com/android/settings/core/codeinspection/CodeInspector.java index 80a2c110af0..86c14a56176 100644 --- a/tests/robotests/src/com/android/settings/core/codeinspection/CodeInspector.java +++ b/tests/robotests/src/com/android/settings/core/codeinspection/CodeInspector.java @@ -16,6 +16,15 @@ package com.android.settings.core.codeinspection; +import com.google.common.truth.Truth; + +import org.junit.Assert; +import org.robolectric.shadows.ShadowApplication; + +import java.io.BufferedReader; +import java.io.InputStream; +import java.io.InputStreamReader; +import java.lang.reflect.Modifier; import java.util.List; /** @@ -24,7 +33,10 @@ import java.util.List; */ public abstract class CodeInspector { - public static final String PACKAGE_NAME = "com.android.settings"; + protected static final String PACKAGE_NAME = "com.android.settings"; + + protected static final String TEST_CLASS_SUFFIX = "Test"; + private static final String TEST_INNER_CLASS_SIGNATURE = "Test$"; protected final List> mClasses; @@ -36,4 +48,41 @@ public abstract class CodeInspector { * Code inspection runner method. */ public abstract void run(); + + protected boolean isConcreteSettingsClass(Class clazz) { + // Abstract classes + if (Modifier.isAbstract(clazz.getModifiers())) { + return false; + } + final String packageName = clazz.getPackage().getName(); + // Classes that are not in Settings + if (!packageName.contains(PACKAGE_NAME + ".")) { + return false; + } + final String className = clazz.getName(); + // Classes from tests + if (className.endsWith(TEST_CLASS_SUFFIX)) { + return false; + } + if (className.contains(TEST_INNER_CLASS_SIGNATURE)) { + return false; + } + return true; + } + + protected void initializeGrandfatherList(List grandfather, String filename) { + try { + final InputStream in = ShadowApplication.getInstance().getApplicationContext() + .getAssets() + .open(filename); + BufferedReader reader = new BufferedReader(new InputStreamReader(in)); + String line; + while ((line = reader.readLine()) != null) { + grandfather.add(line); + } + } catch (Exception e) { + throw new IllegalArgumentException("Error initializing grandfather " + filename, e); + } + + } } diff --git a/tests/robotests/src/com/android/settings/core/instrumentation/InstrumentableFragmentCodeInspector.java b/tests/robotests/src/com/android/settings/core/instrumentation/InstrumentableFragmentCodeInspector.java index 5e8cfdc95d1..109d2ee6987 100644 --- a/tests/robotests/src/com/android/settings/core/instrumentation/InstrumentableFragmentCodeInspector.java +++ b/tests/robotests/src/com/android/settings/core/instrumentation/InstrumentableFragmentCodeInspector.java @@ -19,20 +19,8 @@ package com.android.settings.core.instrumentation; import android.app.Fragment; import android.util.ArraySet; -import com.android.settings.ChooseLockPassword; -import com.android.settings.ChooseLockPattern; -import com.android.settings.CredentialCheckResultTracker; -import com.android.settings.CustomDialogPreference; -import com.android.settings.CustomEditTextPreference; -import com.android.settings.CustomListPreference; -import com.android.settings.RestrictedListPreference; -import com.android.settings.applications.AppOpsCategory; import com.android.settings.core.codeinspection.CodeInspector; -import com.android.settings.core.lifecycle.ObservableDialogFragment; -import com.android.settings.deletionhelper.ActivationWarningFragment; -import com.android.settings.inputmethod.UserDictionaryLocalePicker; -import java.lang.reflect.Modifier; import java.util.ArrayList; import java.util.List; import java.util.Set; @@ -44,30 +32,13 @@ import static com.google.common.truth.Truth.assertWithMessage; */ public class InstrumentableFragmentCodeInspector extends CodeInspector { - private static final String TEST_CLASS_SUFFIX = "Test"; - - private static final List whitelist; - - static { - whitelist = new ArrayList<>(); - whitelist.add( - CustomEditTextPreference.CustomPreferenceDialogFragment.class.getName()); - whitelist.add( - CustomListPreference.CustomListPreferenceDialogFragment.class.getName()); - whitelist.add( - RestrictedListPreference.RestrictedListPreferenceDialogFragment.class.getName()); - whitelist.add(ChooseLockPassword.SaveAndFinishWorker.class.getName()); - whitelist.add(ChooseLockPattern.SaveAndFinishWorker.class.getName()); - whitelist.add(ActivationWarningFragment.class.getName()); - whitelist.add(ObservableDialogFragment.class.getName()); - whitelist.add(CustomDialogPreference.CustomPreferenceDialogFragment.class.getName()); - whitelist.add(AppOpsCategory.class.getName()); - whitelist.add(UserDictionaryLocalePicker.class.getName()); - whitelist.add(CredentialCheckResultTracker.class.getName()); - } + private final List grandfather_notImplmentingInstrumentable; public InstrumentableFragmentCodeInspector(List> classes) { super(classes); + grandfather_notImplmentingInstrumentable = new ArrayList<>(); + initializeGrandfatherList(grandfather_notImplmentingInstrumentable, + "grandfather_not_implementing_instrumentable"); } @Override @@ -75,24 +46,14 @@ public class InstrumentableFragmentCodeInspector extends CodeInspector { final Set broken = new ArraySet<>(); for (Class clazz : mClasses) { - // Skip abstract classes. - if (Modifier.isAbstract(clazz.getModifiers())) { - continue; - } - final String packageName = clazz.getPackage().getName(); - // Skip classes that are not in Settings. - if (!packageName.contains(PACKAGE_NAME + ".")) { + if (!isConcreteSettingsClass(clazz)) { continue; } final String className = clazz.getName(); - // Skip classes from tests. - if (className.endsWith(TEST_CLASS_SUFFIX)) { - continue; - } // If it's a fragment, it must also be instrumentable. if (Fragment.class.isAssignableFrom(clazz) && !Instrumentable.class.isAssignableFrom(clazz) - && !whitelist.contains(className)) { + && !grandfather_notImplmentingInstrumentable.contains(className)) { broken.add(className); } } diff --git a/tests/robotests/src/com/android/settings/search/SearchIndexProviderCodeInspector.java b/tests/robotests/src/com/android/settings/search/SearchIndexProviderCodeInspector.java new file mode 100644 index 00000000000..ee260901f06 --- /dev/null +++ b/tests/robotests/src/com/android/settings/search/SearchIndexProviderCodeInspector.java @@ -0,0 +1,107 @@ +/* + * Copyright (C) 2016 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.settings.search; + +import android.util.ArraySet; +import android.util.Log; + +import com.android.settings.SettingsPreferenceFragment; +import com.android.settings.core.codeinspection.CodeInspector; + +import java.lang.reflect.Field; +import java.util.ArrayList; +import java.util.List; +import java.util.Set; + +import static com.google.common.truth.Truth.assertWithMessage; + +/** + * {@link CodeInspector} to ensure fragments implement search components correctly. + */ +public class SearchIndexProviderCodeInspector extends CodeInspector { + private static final String TAG = "SearchCodeInspector"; + + private final List notImplementingIndexableWhitelist; + private final List notImplementingIndexProviderWhitelist; + + public SearchIndexProviderCodeInspector(List> classes) { + super(classes); + notImplementingIndexableWhitelist = new ArrayList<>(); + notImplementingIndexProviderWhitelist = new ArrayList<>(); + initializeGrandfatherList(notImplementingIndexableWhitelist, + "grandfather_not_implementing_indexable"); + initializeGrandfatherList(notImplementingIndexProviderWhitelist, + "grandfather_not_implementing_index_provider"); + } + + @Override + public void run() { + final Set notImplementingIndexable = new ArraySet<>(); + final Set notImplementingIndexProvider = new ArraySet<>(); + + for (Class clazz : mClasses) { + if (!isConcreteSettingsClass(clazz)) { + continue; + } + final String className = clazz.getName(); + // Skip fragments if it's not SettingsPreferenceFragment. + if (!SettingsPreferenceFragment.class.isAssignableFrom(clazz)) { + continue; + } + // If it's a SettingsPreferenceFragment, it must also be Indexable. + final boolean implementsIndexable = Indexable.class.isAssignableFrom(clazz); + if (!implementsIndexable && !notImplementingIndexableWhitelist.contains(className)) { + notImplementingIndexable.add(className); + } + // If it implements Indexable, it must also implement the index provider field. + if (implementsIndexable && !hasSearchIndexProvider(clazz) + && !notImplementingIndexProviderWhitelist.contains(className)) { + notImplementingIndexProvider.add(className); + } + } + + // Build error messages + final StringBuilder indexableError = new StringBuilder( + "SettingsPreferenceFragment should implement Indexable, but these are not:\n"); + for (String c : notImplementingIndexable) { + indexableError.append(c).append("\n"); + } + final StringBuilder indexProviderError = new StringBuilder( + "Indexable should have public field " + Index.FIELD_NAME_SEARCH_INDEX_DATA_PROVIDER + + " but these are not:\n"); + for (String c : notImplementingIndexProvider) { + indexProviderError.append(c).append("\n"); + } + + assertWithMessage(indexableError.toString()) + .that(notImplementingIndexable.isEmpty()) + .isTrue(); + assertWithMessage(indexProviderError.toString()) + .that(notImplementingIndexProvider.isEmpty()) + .isTrue(); + } + + private boolean hasSearchIndexProvider(Class clazz) { + try { + final Field f = clazz.getField(Index.FIELD_NAME_SEARCH_INDEX_DATA_PROVIDER); + return f != null; + } catch (NoSuchFieldException e) { + Log.e(TAG, "error fetching search provider from class " + clazz.getName()); + return false; + } + } +}