Remove SetFlagsRule.getFakeFeatureFlags()
This method has never been used, and unnecessarily exposes the implementation details of SetFlagsRule.
Flag: TEST_ONLY
NOTE FOR REVIEWERS - original patch and result patch are not identical.
PLEASE REVIEW CAREFULLY.
Diffs between the patches:
/**
> - * Returns a FeatureFlags used by SetFlagsRule of given FeatureFlags
> - *
> - * @param featureFlagsClass The class of FeatureFlags. The interface of FakeFeatureFlagsImpl
> - * @return A FakeFeatureFlagsImpl in type of FeatureFlags
> - */
> - public <T> T getFakeFeatureFlags(Class<T> featureFlagsClass) {
> - if (!featureFlagsClass.isInterface()
> - || !featureFlagsClass.getSimpleName().equals(FEATURE_FLAGS_CLASS_NAME)) {
> - throw new IllegalArgumentException(
> - String.format(
> - "%s is not a FeatureFlags. " + "Please pass in FeatureFlags interface",
> - featureFlagsClass));
> - }
> -
> - String packageName = featureFlagsClass.getPackageName();
> - String flagsClassName = String.format("%s.%s", packageName, FLAGS_CLASS_NAME);
> - Class<?> flagsClass = null;
> -
> - try {
> - flagsClass = Class.forName(flagsClassName);
> - } catch (ClassNotFoundException e) {
> - throw new UnsupportedOperationException(
> - String.format("Failed to load class %s.", flagsClassName));
> - }
> -
> - Object fakeFlagsImplInstance = getOrCreateFakeFlagsImp(flagsClass);
> -
> - return featureFlagsClass.cast(fakeFlagsImplInstance);
> - }
> -
> --- libraries/flag-helpers/junit/test/src/android/platform/test/flag/junit/FakeFeatureFlagsImpl.java
> +++ libraries/flag-helpers/junit/test/src/android/platform/test/flag/junit/FakeFeatureFlagsImpl.java
> - public HashSet<String> readOnlyFlagSet = new HashSet<>();
> + private HashSet<String> mReadOnlyFlagSet = new HashSet<>();
> + this.mReadOnlyFlagSet.add(Flags.FLAG_RO_ENABLED);
> + this.mReadOnlyFlagSet.add(Flags.FLAG_RO_DISABLED);
> + @Override
> + public boolean roEnabled() {
> + return this.mFlagMap.get(Flags.FLAG_RO_ENABLED);
> + }
> +
> + @Override
> + public boolean roDisabled() {
> + return this.mFlagMap.get(Flags.FLAG_RO_DISABLED);
> + }
> +
> - return readOnlyFlagSet.contains(flagName);
> + return mReadOnlyFlagSet.contains(flagName);
> --- libraries/flag-helpers/junit/test/src/android/platform/test/flag/junit/FeatureFlags.java
> +++ libraries/flag-helpers/junit/test/src/android/platform/test/flag/junit/FeatureFlags.java
> +
> + boolean roEnabled();
> +
> + boolean roDisabled();
> --- libraries/flag-helpers/junit/test/src/android/platform/test/flag/junit/Flags.java
> +++ libraries/flag-helpers/junit/test/src/android/platform/test/flag/junit/Flags.java
> -
> - /** Returns the flag value. */
> - public static boolean flagName1() {
> - return true;
> - }
> -
> - /** Returns a non-boolean flag value. */
> - public static int flagName2() {
> - return 1;
> - }
> + public static final String FLAG_RO_ENABLED = "android.platform.test.flag.junit.ro_enabled";
> + public static final String FLAG_RO_DISABLED = "android.platform.test.flag.junit.ro_disabled";
> + public static boolean roEnabled() {
> + return true;
> + }
> +
> + public static boolean roDisabled() {
> + return false;
> + }
> +
> +
> + @Override
> + public boolean roEnabled() {
> + return true;
> + }
> +
> + @Override
> + public boolean roDisabled() {
> + return false;
> + }
> --- libraries/flag-helpers/junit/test/src/android/platform/test/flag/junit/SetFlagsRuleTest.java
> +++ libraries/flag-helpers/junit/test/src/android/platform/test/flag/junit/SetFlagsRuleTest.java
> - public void getFakeFeatureFlags_afterSet() {
> - mSetFlagsRule.enableFlags(Flags.FLAG_FLAG_NAME3, Flags.FLAG_FLAG_NAME4);
> - FeatureFlags fakeFlagsImpl = mSetFlagsRule.getFakeFeatureFlags(FeatureFlags.class);
> - assertTrue(fakeFlagsImpl.flagName3());
> - assertTrue(fakeFlagsImpl.flagName4());
> -
> - mSetFlagsRule.disableFlags(Flags.FLAG_FLAG_NAME3, Flags.FLAG_FLAG_NAME4);
> - assertFalse(fakeFlagsImpl.flagName3());
> - assertFalse(fakeFlagsImpl.flagName4());
> - }
> -
> - @Test
> - public void getFakeFeatureFlags_thenSet() {
> - FeatureFlags fakeFlagsImpl = mSetFlagsRule.getFakeFeatureFlags(FeatureFlags.class);
> - if (this.mIsInitWithDefault) {
> - assertFalse(Flags.flagName3());
> - assertFalse(fakeFlagsImpl.flagName3());
> - assertTrue(Flags.flagName4());
> - assertTrue(fakeFlagsImpl.flagName4());
> -
> - mSetFlagsRule.enableFlags(Flags.FLAG_FLAG_NAME3);
> - mSetFlagsRule.disableFlags(Flags.FLAG_FLAG_NAME4);
> -
> - assertTrue(Flags.flagName3());
> - assertTrue(fakeFlagsImpl.flagName3());
> - assertFalse(Flags.flagName4());
> - assertFalse(fakeFlagsImpl.flagName4());
> - } else {
> - assertThrows(
> - NullPointerException.class,
> - () -> {
> - fakeFlagsImpl.flagName3();
> - });
> - assertFalse(Flags.flagName3());
> -
> - mSetFlagsRule.enableFlags(Flags.FLAG_FLAG_NAME3);
> -
> - assertTrue(Flags.flagName3());
> - assertTrue(fakeFlagsImpl.flagName3());
> - assertThrows(
> - NullPointerException.class,
> - () -> {
> - fakeFlagsImpl.flagName4();
> - });
> - assertThrows(
> - NullPointerException.class,
> - () -> {
> - Flags.flagName4();
> - });
> -
> - mSetFlagsRule.disableFlags(Flags.FLAG_FLAG_NAME4);
> -
> - assertFalse(Flags.flagName4());
> - assertFalse(fakeFlagsImpl.flagName4());
> - }
> - }
> -
> - @Test
> - public void getFakeFeatureFlags_castToWrongType() {
> - mSetFlagsRule.enableFlags(Flags.FLAG_FLAG_NAME3, Flags.FLAG_FLAG_NAME4);
> -
> - assertThrows(
> - IllegalArgumentException.class,
> - () -> {
> - FakeFeatureFlagsImpl fakeFlagsImpl =
> - mSetFlagsRule.getFakeFeatureFlags(FakeFeatureFlagsImpl.class);
> - });
> - assertThrows(
> - IllegalArgumentException.class,
> - () -> {
> - Flags fakeFlagsImpl = mSetFlagsRule.getFakeFeatureFlags(Flags.class);
> - });
> - assertThrows(
> - IllegalArgumentException.class,
> - () -> {
> - FeatureFlagsImpl fakeFlagsImpl =
> - mSetFlagsRule.getFakeFeatureFlags(FeatureFlagsImpl.class);
> - });
> - assertThrows(
> - IllegalArgumentException.class,
> - () -> {
> - FakeFeatureFlags fakeFlagsImpl =
> - mSetFlagsRule.getFakeFeatureFlags(FakeFeatureFlags.class);
> - });
> - }
> -
> - @Test
> - FakeFeatureFlagsImpl fakeFlagsImpl =
> - (FakeFeatureFlagsImpl) mSetFlagsRule.getFakeFeatureFlags(FeatureFlags.class);
> - fakeFlagsImpl.readOnlyFlagSet.add(Flags.FLAG_FLAG_NAME3);
> - mSetFlagsRule.enableFlags(Flags.FLAG_FLAG_NAME3);
> + mSetFlagsRule.enableFlags(Flags.FLAG_RO_ENABLED);
> - }
> -
> - private class FeatureFlagsImpl implements FeatureFlags {
> - @Override
> - public boolean flagName3() {
> - return false;
> - }
> -
> - @Override
> - public boolean flagName4() {
> - return true;
> - }
> - }
> -
> - private interface FakeFeatureFlags {
> - /** Returns the flag value. */
> - boolean flagName3();
> + assertThrows(
> + AssumptionViolatedException.class,
> + () -> {
> + mSetFlagsRule.disableFlags(Flags.FLAG_RO_ENABLED);
> + });
> + assertThrows(
> + AssumptionViolatedException.class,
> + () -> {
> + mSetFlagsRule.enableFlags(Flags.FLAG_RO_DISABLED);
> + });
> + assertThrows(
> + AssumptionViolatedException.class,
> + () -> {
> + mSetFlagsRule.disableFlags(Flags.FLAG_RO_DISABLED);
> + });
Original patch:
diff --git a/libraries/flag-helpers/junit/src_base/android/platform/test/flag/junit/SetFlagsRule.java b/libraries/flag-helpers/junit/src_base/android/platform/test/flag/junit/SetFlagsRule.java
old mode 100644
new mode 100644
--- a/libraries/flag-helpers/junit/src_base/android/platform/test/flag/junit/SetFlagsRule.java
+++ b/libraries/flag-helpers/junit/src_base/android/platform/test/flag/junit/SetFlagsRule.java
@@ -183,37 +183,6 @@
setFlagValue(fullFlagName, false);
}
}
-
- /**
- * Returns a FeatureFlags used by SetFlagsRule of given FeatureFlags
- *
- * @param featureFlagsClass The class of FeatureFlags. The interface of FakeFeatureFlagsImpl
- * @return A FakeFeatureFlagsImpl in type of FeatureFlags
- */
- public <T> T getFakeFeatureFlags(Class<T> featureFlagsClass) {
- if (!featureFlagsClass.isInterface()
- || !featureFlagsClass.getSimpleName().equals(FEATURE_FLAGS_CLASS_NAME)) {
- throw new IllegalArgumentException(
-
[[[Original patch trimmed due to size. Decoded string size: 11335. Decoded string SHA1: 5e69c5baf3fd2c06ad8faf0a7df442caf0f11bac.]]]
Result patch:
diff --git a/libraries/flag-helpers/junit/src_base/android/platform/test/flag/junit/SetFlagsRule.java b/libraries/flag-helpers/junit/src_base/android/platform/test/flag/junit/SetFlagsRule.java
index 0c86961..6c6e2fd 100644
--- a/libraries/flag-helpers/junit/src_base/android/platform/test/flag/junit/SetFlagsRule.java
+++ b/libraries/flag-helpers/junit/src_base/android/platform/test/flag/junit/SetFlagsRule.java
@@ -184,37 +184,6 @@
}
}
- /**
- * Returns a FeatureFlags used by SetFlagsRule of given FeatureFlags
- *
- * @param featureFlagsClass The class of FeatureFlags. The interface of FakeFeatureFlagsImpl
- * @return A FakeFeatureFlagsImpl in type of FeatureFlags
- */
- public <T> T getFakeFeatureFlags(Class<T> featureFlagsClass) {
- if (!featureFlagsClass.isInterface()
- || !featureFlagsClass.getSimpleName().equals(FEATURE_FLAGS_CLASS_NAME)) {
- throw new IllegalArgumentException(
- String.format(
-
[[[Result patch trimmed due to size. Decoded string size: 11451. Decoded string SHA1: 7433127fbb448fd40c3c3dc12433b48e0394301c.]]]
Change-Id: Ib9f814bbad6fb549685c6cdba85682fd0806c3c3
diff --git a/libraries/flag-helpers/junit/src_base/android/platform/test/flag/junit/SetFlagsRule.java b/libraries/flag-helpers/junit/src_base/android/platform/test/flag/junit/SetFlagsRule.java
index 0c86961..6c6e2fd 100644
--- a/libraries/flag-helpers/junit/src_base/android/platform/test/flag/junit/SetFlagsRule.java
+++ b/libraries/flag-helpers/junit/src_base/android/platform/test/flag/junit/SetFlagsRule.java
@@ -184,37 +184,6 @@
}
}
- /**
- * Returns a FeatureFlags used by SetFlagsRule of given FeatureFlags
- *
- * @param featureFlagsClass The class of FeatureFlags. The interface of FakeFeatureFlagsImpl
- * @return A FakeFeatureFlagsImpl in type of FeatureFlags
- */
- public <T> T getFakeFeatureFlags(Class<T> featureFlagsClass) {
- if (!featureFlagsClass.isInterface()
- || !featureFlagsClass.getSimpleName().equals(FEATURE_FLAGS_CLASS_NAME)) {
- throw new IllegalArgumentException(
- String.format(
- "%s is not a FeatureFlags. " + "Please pass in FeatureFlags interface",
- featureFlagsClass));
- }
-
- String packageName = featureFlagsClass.getPackageName();
- String flagsClassName = String.format("%s.%s", packageName, FLAGS_CLASS_NAME);
- Class<?> flagsClass = null;
-
- try {
- flagsClass = Class.forName(flagsClassName);
- } catch (ClassNotFoundException e) {
- throw new UnsupportedOperationException(
- String.format("Failed to load class %s.", flagsClassName));
- }
-
- Object fakeFlagsImplInstance = getOrCreateFakeFlagsImp(flagsClass);
-
- return featureFlagsClass.cast(fakeFlagsImplInstance);
- }
-
private void ensureFlagsAreUnset() {
if (!mFlagsClassToFakeFlagsImpl.isEmpty()) {
throw new IllegalStateException("Some flags were set before the rule was initialized");
diff --git a/libraries/flag-helpers/junit/test/src/android/platform/test/flag/junit/FakeFeatureFlagsImpl.java b/libraries/flag-helpers/junit/test/src/android/platform/test/flag/junit/FakeFeatureFlagsImpl.java
index 2fd2ab3..c94637b 100644
--- a/libraries/flag-helpers/junit/test/src/android/platform/test/flag/junit/FakeFeatureFlagsImpl.java
+++ b/libraries/flag-helpers/junit/test/src/android/platform/test/flag/junit/FakeFeatureFlagsImpl.java
@@ -22,12 +22,14 @@
/** A Fake FakeFeatureFlagsImpl to test the {@code MockFlagsRule}. */
public class FakeFeatureFlagsImpl implements FeatureFlags {
- public HashSet<String> readOnlyFlagSet = new HashSet<>();
+ private HashSet<String> mReadOnlyFlagSet = new HashSet<>();
private HashMap<String, Boolean> mFlagMap = new HashMap<>();
public FakeFeatureFlagsImpl() {
this.mFlagMap.put(Flags.FLAG_FLAG_NAME3, null);
this.mFlagMap.put(Flags.FLAG_FLAG_NAME4, null);
+ this.mReadOnlyFlagSet.add(Flags.FLAG_RO_ENABLED);
+ this.mReadOnlyFlagSet.add(Flags.FLAG_RO_DISABLED);
}
/** Returns the flag value. */
@@ -42,6 +44,16 @@
return this.mFlagMap.get(Flags.FLAG_FLAG_NAME4);
}
+ @Override
+ public boolean roEnabled() {
+ return this.mFlagMap.get(Flags.FLAG_RO_ENABLED);
+ }
+
+ @Override
+ public boolean roDisabled() {
+ return this.mFlagMap.get(Flags.FLAG_RO_DISABLED);
+ }
+
public void setFlag(String flag, boolean value) {
this.mFlagMap.put(flag, value);
}
@@ -52,6 +64,6 @@
/** Verify if the given flag is read_only and optimized */
public boolean isFlagReadOnlyOptimized(String flagName) {
- return readOnlyFlagSet.contains(flagName);
+ return mReadOnlyFlagSet.contains(flagName);
}
}
diff --git a/libraries/flag-helpers/junit/test/src/android/platform/test/flag/junit/FeatureFlags.java b/libraries/flag-helpers/junit/test/src/android/platform/test/flag/junit/FeatureFlags.java
index 428ae07..9c69229 100644
--- a/libraries/flag-helpers/junit/test/src/android/platform/test/flag/junit/FeatureFlags.java
+++ b/libraries/flag-helpers/junit/test/src/android/platform/test/flag/junit/FeatureFlags.java
@@ -24,4 +24,8 @@
/** another flag */
boolean flagName4();
+
+ boolean roEnabled();
+
+ boolean roDisabled();
}
diff --git a/libraries/flag-helpers/junit/test/src/android/platform/test/flag/junit/Flags.java b/libraries/flag-helpers/junit/test/src/android/platform/test/flag/junit/Flags.java
index 8027d1e..0d67328 100644
--- a/libraries/flag-helpers/junit/test/src/android/platform/test/flag/junit/Flags.java
+++ b/libraries/flag-helpers/junit/test/src/android/platform/test/flag/junit/Flags.java
@@ -21,16 +21,8 @@
public static final String FLAG_FLAG_NAME3 = "android.platform.test.flag.junit.flag_name3";
public static final String FLAG_FLAG_NAME4 = "android.platform.test.flag.junit.flag_name4";
-
- /** Returns the flag value. */
- public static boolean flagName1() {
- return true;
- }
-
- /** Returns a non-boolean flag value. */
- public static int flagName2() {
- return 1;
- }
+ public static final String FLAG_RO_ENABLED = "android.platform.test.flag.junit.ro_enabled";
+ public static final String FLAG_RO_DISABLED = "android.platform.test.flag.junit.ro_disabled";
/** Returns the flag value. */
public static boolean flagName3() {
@@ -42,6 +34,14 @@
return FEATURE_FLAGS.flagName4();
}
+ public static boolean roEnabled() {
+ return true;
+ }
+
+ public static boolean roDisabled() {
+ return false;
+ }
+
public static void setFeatureFlags(FeatureFlags featureFlagsImpl) {
FEATURE_FLAGS = featureFlagsImpl;
}
@@ -57,5 +57,15 @@
public boolean flagName4() {
return true;
}
+
+ @Override
+ public boolean roEnabled() {
+ return true;
+ }
+
+ @Override
+ public boolean roDisabled() {
+ return false;
+ }
};
}
diff --git a/libraries/flag-helpers/junit/test/src/android/platform/test/flag/junit/SetFlagsRuleTest.java b/libraries/flag-helpers/junit/test/src/android/platform/test/flag/junit/SetFlagsRuleTest.java
index 6c255c7..58112fd 100644
--- a/libraries/flag-helpers/junit/test/src/android/platform/test/flag/junit/SetFlagsRuleTest.java
+++ b/libraries/flag-helpers/junit/test/src/android/platform/test/flag/junit/SetFlagsRuleTest.java
@@ -105,118 +105,26 @@
}
@Test
- public void getFakeFeatureFlags_afterSet() {
- mSetFlagsRule.enableFlags(Flags.FLAG_FLAG_NAME3, Flags.FLAG_FLAG_NAME4);
- FeatureFlags fakeFlagsImpl = mSetFlagsRule.getFakeFeatureFlags(FeatureFlags.class);
- assertTrue(fakeFlagsImpl.flagName3());
- assertTrue(fakeFlagsImpl.flagName4());
-
- mSetFlagsRule.disableFlags(Flags.FLAG_FLAG_NAME3, Flags.FLAG_FLAG_NAME4);
- assertFalse(fakeFlagsImpl.flagName3());
- assertFalse(fakeFlagsImpl.flagName4());
- }
-
- @Test
- public void getFakeFeatureFlags_thenSet() {
- FeatureFlags fakeFlagsImpl = mSetFlagsRule.getFakeFeatureFlags(FeatureFlags.class);
- if (this.mIsInitWithDefault) {
- assertFalse(Flags.flagName3());
- assertFalse(fakeFlagsImpl.flagName3());
- assertTrue(Flags.flagName4());
- assertTrue(fakeFlagsImpl.flagName4());
-
- mSetFlagsRule.enableFlags(Flags.FLAG_FLAG_NAME3);
- mSetFlagsRule.disableFlags(Flags.FLAG_FLAG_NAME4);
-
- assertTrue(Flags.flagName3());
- assertTrue(fakeFlagsImpl.flagName3());
- assertFalse(Flags.flagName4());
- assertFalse(fakeFlagsImpl.flagName4());
- } else {
- assertThrows(
- NullPointerException.class,
- () -> {
- fakeFlagsImpl.flagName3();
- });
- assertFalse(Flags.flagName3());
-
- mSetFlagsRule.enableFlags(Flags.FLAG_FLAG_NAME3);
-
- assertTrue(Flags.flagName3());
- assertTrue(fakeFlagsImpl.flagName3());
- assertThrows(
- NullPointerException.class,
- () -> {
- fakeFlagsImpl.flagName4();
- });
- assertThrows(
- NullPointerException.class,
- () -> {
- Flags.flagName4();
- });
-
- mSetFlagsRule.disableFlags(Flags.FLAG_FLAG_NAME4);
-
- assertFalse(Flags.flagName4());
- assertFalse(fakeFlagsImpl.flagName4());
- }
- }
-
- @Test
- public void getFakeFeatureFlags_castToWrongType() {
- mSetFlagsRule.enableFlags(Flags.FLAG_FLAG_NAME3, Flags.FLAG_FLAG_NAME4);
-
- assertThrows(
- IllegalArgumentException.class,
- () -> {
- FakeFeatureFlagsImpl fakeFlagsImpl =
- mSetFlagsRule.getFakeFeatureFlags(FakeFeatureFlagsImpl.class);
- });
- assertThrows(
- IllegalArgumentException.class,
- () -> {
- Flags fakeFlagsImpl = mSetFlagsRule.getFakeFeatureFlags(Flags.class);
- });
- assertThrows(
- IllegalArgumentException.class,
- () -> {
- FeatureFlagsImpl fakeFlagsImpl =
- mSetFlagsRule.getFakeFeatureFlags(FeatureFlagsImpl.class);
- });
- assertThrows(
- IllegalArgumentException.class,
- () -> {
- FakeFeatureFlags fakeFlagsImpl =
- mSetFlagsRule.getFakeFeatureFlags(FakeFeatureFlags.class);
- });
- }
-
- @Test
public void skipReadOnlyOptimizedFlag() {
- FakeFeatureFlagsImpl fakeFlagsImpl =
- (FakeFeatureFlagsImpl) mSetFlagsRule.getFakeFeatureFlags(FeatureFlags.class);
- fakeFlagsImpl.readOnlyFlagSet.add(Flags.FLAG_FLAG_NAME3);
assertThrows(
AssumptionViolatedException.class,
() -> {
- mSetFlagsRule.enableFlags(Flags.FLAG_FLAG_NAME3);
+ mSetFlagsRule.enableFlags(Flags.FLAG_RO_ENABLED);
});
- }
-
- private class FeatureFlagsImpl implements FeatureFlags {
- @Override
- public boolean flagName3() {
- return false;
- }
-
- @Override
- public boolean flagName4() {
- return true;
- }
- }
-
- private interface FakeFeatureFlags {
- /** Returns the flag value. */
- boolean flagName3();
+ assertThrows(
+ AssumptionViolatedException.class,
+ () -> {
+ mSetFlagsRule.disableFlags(Flags.FLAG_RO_ENABLED);
+ });
+ assertThrows(
+ AssumptionViolatedException.class,
+ () -> {
+ mSetFlagsRule.enableFlags(Flags.FLAG_RO_DISABLED);
+ });
+ assertThrows(
+ AssumptionViolatedException.class,
+ () -> {
+ mSetFlagsRule.disableFlags(Flags.FLAG_RO_DISABLED);
+ });
}
}