Add code inspector to ensure fragments implements search.
Unless a fragment is grandfathered:
- If it's a SearchPreferenceFragment, it needs to implement Indexable.
- If it's a Indexable, it needs to contain SEARCH_INDEX_DATA_PROVIDER.
Bug: 33209410
Test: make RunSettingsRoboTests
Change-Id: I078c54374341ba2966145429fc1507a3d5763d3b
diff --git a/src/com/android/settings/search/Index.java b/src/com/android/settings/search/Index.java
index ccedd62..bff841b 100644
--- a/src/com/android/settings/search/Index.java
+++ b/src/com/android/settings/search/Index.java
@@ -110,6 +110,9 @@
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 @@
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 0000000..dd80440
--- /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 0000000..c1b8cf5
--- /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 0000000..5bd96af
--- /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 d6d6963..9127d5f 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.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 @@
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 88d8171..4aa576f 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 @@
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 @@
* 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<Class<?>> mClasses;
@@ -45,5 +47,6 @@
@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 80a2c11..86c14a5 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 @@
*/
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<Class<?>> mClasses;
@@ -36,4 +48,41 @@
* 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<String> 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 5e8cfdc..109d2ee 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 @@
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 @@
*/
public class InstrumentableFragmentCodeInspector extends CodeInspector {
- private static final String TEST_CLASS_SUFFIX = "Test";
-
- private static final List<String> 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<String> grandfather_notImplmentingInstrumentable;
public InstrumentableFragmentCodeInspector(List<Class<?>> classes) {
super(classes);
+ grandfather_notImplmentingInstrumentable = new ArrayList<>();
+ initializeGrandfatherList(grandfather_notImplmentingInstrumentable,
+ "grandfather_not_implementing_instrumentable");
}
@Override
@@ -75,24 +46,14 @@
final Set<String> 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 0000000..ee26090
--- /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<String> notImplementingIndexableWhitelist;
+ private final List<String> notImplementingIndexProviderWhitelist;
+
+ public SearchIndexProviderCodeInspector(List<Class<?>> 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<String> notImplementingIndexable = new ArraySet<>();
+ final Set<String> 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;
+ }
+ }
+}