Merge "Only restart SystemUI when Server Flags actually change." into tm-qpr-dev am: d7c9694969

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/21961694

Change-Id: I79f28255ac645cc656b35851b59fde239de3d70c
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
This commit is contained in:
Dave Mankoff
2023-03-15 14:29:15 +00:00
committed by Automerger Merge Worker
6 changed files with 247 additions and 104 deletions

View File

@@ -85,9 +85,34 @@ public class FeatureFlagsDebug implements FeatureFlags {
private final ServerFlagReader.ChangeListener mOnPropertiesChanged =
new ServerFlagReader.ChangeListener() {
@Override
public void onChange(Flag<?> flag) {
mRestarter.restartSystemUI(
"Server flag change: " + flag.getNamespace() + "." + flag.getName());
public void onChange(Flag<?> flag, String value) {
boolean shouldRestart = false;
if (mBooleanFlagCache.containsKey(flag.getName())) {
boolean newValue = value == null ? false : Boolean.parseBoolean(value);
if (mBooleanFlagCache.get(flag.getName()) != newValue) {
shouldRestart = true;
}
} else if (mStringFlagCache.containsKey(flag.getName())) {
String newValue = value == null ? "" : value;
if (mStringFlagCache.get(flag.getName()) != value) {
shouldRestart = true;
}
} else if (mIntFlagCache.containsKey(flag.getName())) {
int newValue = 0;
try {
newValue = value == null ? 0 : Integer.parseInt(value);
} catch (NumberFormatException e) {
}
if (mIntFlagCache.get(flag.getName()) != newValue) {
shouldRestart = true;
}
}
if (shouldRestart) {
mRestarter.restartSystemUI(
"Server flag change: " + flag.getNamespace() + "."
+ flag.getName());
}
}
};

View File

@@ -53,13 +53,38 @@ public class FeatureFlagsRelease implements FeatureFlags {
private final Map<String, Flag<?>> mAllFlags;
private final Map<String, Boolean> mBooleanCache = new HashMap<>();
private final Map<String, String> mStringCache = new HashMap<>();
private final Map<String, Integer> mIntCache = new HashMap<>();
private final ServerFlagReader.ChangeListener mOnPropertiesChanged =
new ServerFlagReader.ChangeListener() {
@Override
public void onChange(Flag<?> flag) {
mRestarter.restartSystemUI(
"Server flag change: " + flag.getNamespace() + "." + flag.getName());
public void onChange(Flag<?> flag, String value) {
boolean shouldRestart = false;
if (mBooleanCache.containsKey(flag.getName())) {
boolean newValue = value == null ? false : Boolean.parseBoolean(value);
if (mBooleanCache.get(flag.getName()) != newValue) {
shouldRestart = true;
}
} else if (mStringCache.containsKey(flag.getName())) {
String newValue = value == null ? "" : value;
if (mStringCache.get(flag.getName()) != newValue) {
shouldRestart = true;
}
} else if (mIntCache.containsKey(flag.getName())) {
int newValue = 0;
try {
newValue = value == null ? 0 : Integer.parseInt(value);
} catch (NumberFormatException e) {
}
if (mIntCache.get(flag.getName()) != newValue) {
shouldRestart = true;
}
}
if (shouldRestart) {
mRestarter.restartSystemUI(
"Server flag change: " + flag.getNamespace() + "."
+ flag.getName());
}
}
};
@@ -97,68 +122,97 @@ public class FeatureFlagsRelease implements FeatureFlags {
@Override
public boolean isEnabled(@NotNull ReleasedFlag flag) {
return mServerFlagReader.readServerOverride(flag.getNamespace(), flag.getName(), true);
// Fill the cache.
return isEnabledInternal(flag.getName(),
mServerFlagReader.readServerOverride(flag.getNamespace(), flag.getName(), true));
}
@Override
public boolean isEnabled(ResourceBooleanFlag flag) {
if (!mBooleanCache.containsKey(flag.getName())) {
return isEnabled(flag.getName(), mResources.getBoolean(flag.getResourceId()));
}
return mBooleanCache.get(flag.getName());
// Fill the cache.
return isEnabledInternal(flag.getName(), mResources.getBoolean(flag.getResourceId()));
}
@Override
public boolean isEnabled(SysPropBooleanFlag flag) {
if (!mBooleanCache.containsKey(flag.getName())) {
return isEnabled(
flag.getName(),
mSystemProperties.getBoolean(flag.getName(), flag.getDefault()));
}
return mBooleanCache.get(flag.getName());
// Fill the cache.
return isEnabledInternal(
flag.getName(),
mSystemProperties.getBoolean(flag.getName(), flag.getDefault()));
}
private boolean isEnabled(String name, boolean defaultValue) {
mBooleanCache.put(name, defaultValue);
return defaultValue;
/**
* Checks and fills the boolean cache. This is important, Always call through to this method!
*
* We use the cache as a way to decide if we need to restart the process when server-side
* changes occur.
*/
private boolean isEnabledInternal(String name, boolean defaultValue) {
// Fill the cache.
if (!mBooleanCache.containsKey(name)) {
mBooleanCache.put(name, defaultValue);
}
return mBooleanCache.get(name);
}
@NonNull
@Override
public String getString(@NonNull StringFlag flag) {
return getString(flag.getName(), flag.getDefault());
// Fill the cache.
return getStringInternal(flag.getName(), flag.getDefault());
}
@NonNull
@Override
public String getString(@NonNull ResourceStringFlag flag) {
if (!mStringCache.containsKey(flag.getName())) {
return getString(flag.getName(),
requireNonNull(mResources.getString(flag.getResourceId())));
}
return mStringCache.get(flag.getName());
// Fill the cache.
return getStringInternal(flag.getName(),
requireNonNull(mResources.getString(flag.getResourceId())));
}
private String getString(String name, String defaultValue) {
mStringCache.put(name, defaultValue);
return defaultValue;
/**
* Checks and fills the String cache. This is important, Always call through to this method!
*
* We use the cache as a way to decide if we need to restart the process when server-side
* changes occur.
*/
private String getStringInternal(String name, String defaultValue) {
if (!mStringCache.containsKey(name)) {
mStringCache.put(name, defaultValue);
}
return mStringCache.get(name);
}
@NonNull
@Override
public int getInt(@NonNull IntFlag flag) {
return flag.getDefault();
// Fill the cache.
return getIntInternal(flag.getName(), flag.getDefault());
}
@NonNull
@Override
public int getInt(@NonNull ResourceIntFlag flag) {
// Fill the cache.
return mResources.getInteger(flag.getResourceId());
}
/**
* Checks and fills the integer cache. This is important, Always call through to this method!
*
* We use the cache as a way to decide if we need to restart the process when server-side
* changes occur.
*/
private int getIntInternal(String name, int defaultValue) {
if (!mIntCache.containsKey(name)) {
mIntCache.put(name, defaultValue);
}
return mIntCache.get(name);
}
@Override
public void dump(@NonNull PrintWriter pw, @NonNull String[] args) {
pw.println("can override: false");

View File

@@ -37,7 +37,7 @@ interface ServerFlagReader {
fun listenForChanges(values: Collection<Flag<*>>, listener: ChangeListener)
interface ChangeListener {
fun onChange(flag: Flag<*>)
fun onChange(flag: Flag<*>, value: String?)
}
}
@@ -67,7 +67,7 @@ class ServerFlagReaderImpl @Inject constructor(
propLoop@ for (propName in properties.keyset) {
for (flag in flags) {
if (propName == flag.name) {
listener.onChange(flag)
listener.onChange(flag, properties.getString(propName, null))
break@propLoop
}
}
@@ -144,7 +144,7 @@ class ServerFlagReaderFake : ServerFlagReader {
for ((listener, flags) in listeners) {
flagLoop@ for (flag in flags) {
if (name == flag.name) {
listener.onChange(flag)
listener.onChange(flag, if (value) "true" else "false")
break@flagLoop
}
}
@@ -159,5 +159,6 @@ class ServerFlagReaderFake : ServerFlagReader {
flags: Collection<Flag<*>>,
listener: ServerFlagReader.ChangeListener
) {
listeners.add(Pair(listener, flags))
}
}

View File

@@ -37,6 +37,7 @@ import org.mockito.Mock
import org.mockito.Mockito.anyBoolean
import org.mockito.Mockito.anyString
import org.mockito.Mockito.inOrder
import org.mockito.Mockito.never
import org.mockito.Mockito.times
import org.mockito.Mockito.verify
import org.mockito.Mockito.verifyNoMoreInteractions
@@ -53,7 +54,7 @@ import org.mockito.Mockito.`when` as whenever
*/
@SmallTest
class FeatureFlagsDebugTest : SysuiTestCase() {
private lateinit var mFeatureFlagsDebug: FeatureFlagsDebug
private lateinit var featureFlagsDebug: FeatureFlagsDebug
@Mock
private lateinit var flagManager: FlagManager
@@ -85,7 +86,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
flagMap.put(Flags.TEAMFOOD.name, Flags.TEAMFOOD)
flagMap.put(teamfoodableFlagA.name, teamfoodableFlagA)
flagMap.put(teamfoodableFlagB.name, teamfoodableFlagB)
mFeatureFlagsDebug = FeatureFlagsDebug(
featureFlagsDebug = FeatureFlagsDebug(
flagManager,
mockContext,
globalSettings,
@@ -95,7 +96,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
flagMap,
restarter
)
mFeatureFlagsDebug.init()
featureFlagsDebug.init()
verify(flagManager).onSettingsChangedAction = any()
broadcastReceiver = withArgCaptor {
verify(mockContext).registerReceiver(
@@ -116,7 +117,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
whenever(flagManager.readFlagValue<Boolean>(eq("4"), any())).thenReturn(false)
assertThat(
mFeatureFlagsDebug.isEnabled(
featureFlagsDebug.isEnabled(
ReleasedFlag(
2,
name = "2",
@@ -125,7 +126,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
)
).isTrue()
assertThat(
mFeatureFlagsDebug.isEnabled(
featureFlagsDebug.isEnabled(
UnreleasedFlag(
3,
name = "3",
@@ -134,7 +135,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
)
).isTrue()
assertThat(
mFeatureFlagsDebug.isEnabled(
featureFlagsDebug.isEnabled(
ReleasedFlag(
4,
name = "4",
@@ -143,7 +144,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
)
).isFalse()
assertThat(
mFeatureFlagsDebug.isEnabled(
featureFlagsDebug.isEnabled(
UnreleasedFlag(
5,
name = "5",
@@ -157,8 +158,8 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
fun teamFoodFlag_False() {
whenever(flagManager.readFlagValue<Boolean>(
eq(Flags.TEAMFOOD.name), any())).thenReturn(false)
assertThat(mFeatureFlagsDebug.isEnabled(teamfoodableFlagA)).isFalse()
assertThat(mFeatureFlagsDebug.isEnabled(teamfoodableFlagB)).isTrue()
assertThat(featureFlagsDebug.isEnabled(teamfoodableFlagA)).isFalse()
assertThat(featureFlagsDebug.isEnabled(teamfoodableFlagB)).isTrue()
// Regular boolean flags should still test the same.
// Only our teamfoodableFlag should change.
@@ -169,8 +170,8 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
fun teamFoodFlag_True() {
whenever(flagManager.readFlagValue<Boolean>(
eq(Flags.TEAMFOOD.name), any())).thenReturn(true)
assertThat(mFeatureFlagsDebug.isEnabled(teamfoodableFlagA)).isTrue()
assertThat(mFeatureFlagsDebug.isEnabled(teamfoodableFlagB)).isTrue()
assertThat(featureFlagsDebug.isEnabled(teamfoodableFlagA)).isTrue()
assertThat(featureFlagsDebug.isEnabled(teamfoodableFlagB)).isTrue()
// Regular boolean flags should still test the same.
// Only our teamfoodableFlag should change.
@@ -185,8 +186,8 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
.thenReturn(false)
whenever(flagManager.readFlagValue<Boolean>(
eq(Flags.TEAMFOOD.name), any())).thenReturn(true)
assertThat(mFeatureFlagsDebug.isEnabled(teamfoodableFlagA)).isTrue()
assertThat(mFeatureFlagsDebug.isEnabled(teamfoodableFlagB)).isFalse()
assertThat(featureFlagsDebug.isEnabled(teamfoodableFlagA)).isTrue()
assertThat(featureFlagsDebug.isEnabled(teamfoodableFlagB)).isFalse()
// Regular boolean flags should still test the same.
// Only our teamfoodableFlag should change.
@@ -205,7 +206,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
whenever(flagManager.readFlagValue<Boolean>(eq("5"), any())).thenReturn(false)
assertThat(
mFeatureFlagsDebug.isEnabled(
featureFlagsDebug.isEnabled(
ResourceBooleanFlag(
1,
"1",
@@ -214,16 +215,16 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
)
)
).isFalse()
assertThat(mFeatureFlagsDebug.isEnabled(ResourceBooleanFlag(2, "2", "test", 1002))).isTrue()
assertThat(mFeatureFlagsDebug.isEnabled(ResourceBooleanFlag(3, "3", "test", 1003))).isTrue()
assertThat(featureFlagsDebug.isEnabled(ResourceBooleanFlag(2, "2", "test", 1002))).isTrue()
assertThat(featureFlagsDebug.isEnabled(ResourceBooleanFlag(3, "3", "test", 1003))).isTrue()
Assert.assertThrows(NameNotFoundException::class.java) {
mFeatureFlagsDebug.isEnabled(ResourceBooleanFlag(4, "4", "test", 1004))
featureFlagsDebug.isEnabled(ResourceBooleanFlag(4, "4", "test", 1004))
}
// Test that resource is loaded (and validated) even when the setting is set.
// This prevents developers from not noticing when they reference an invalid resource.
Assert.assertThrows(NameNotFoundException::class.java) {
mFeatureFlagsDebug.isEnabled(ResourceBooleanFlag(5, "5", "test", 1005))
featureFlagsDebug.isEnabled(ResourceBooleanFlag(5, "5", "test", 1005))
}
}
@@ -236,11 +237,11 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
return@thenAnswer it.getArgument(1)
}
assertThat(mFeatureFlagsDebug.isEnabled(SysPropBooleanFlag(1, "a", "test"))).isFalse()
assertThat(mFeatureFlagsDebug.isEnabled(SysPropBooleanFlag(2, "b", "test"))).isTrue()
assertThat(mFeatureFlagsDebug.isEnabled(SysPropBooleanFlag(3, "c", "test", true))).isTrue()
assertThat(featureFlagsDebug.isEnabled(SysPropBooleanFlag(1, "a", "test"))).isFalse()
assertThat(featureFlagsDebug.isEnabled(SysPropBooleanFlag(2, "b", "test"))).isTrue()
assertThat(featureFlagsDebug.isEnabled(SysPropBooleanFlag(3, "c", "test", true))).isTrue()
assertThat(
mFeatureFlagsDebug.isEnabled(
featureFlagsDebug.isEnabled(
SysPropBooleanFlag(
4,
"d",
@@ -249,17 +250,17 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
)
)
).isFalse()
assertThat(mFeatureFlagsDebug.isEnabled(SysPropBooleanFlag(5, "e", "test"))).isFalse()
assertThat(featureFlagsDebug.isEnabled(SysPropBooleanFlag(5, "e", "test"))).isFalse()
}
@Test
fun readStringFlag() {
whenever(flagManager.readFlagValue<String>(eq("3"), any())).thenReturn("foo")
whenever(flagManager.readFlagValue<String>(eq("4"), any())).thenReturn("bar")
assertThat(mFeatureFlagsDebug.getString(StringFlag(1, "1", "test", "biz"))).isEqualTo("biz")
assertThat(mFeatureFlagsDebug.getString(StringFlag(2, "2", "test", "baz"))).isEqualTo("baz")
assertThat(mFeatureFlagsDebug.getString(StringFlag(3, "3", "test", "buz"))).isEqualTo("foo")
assertThat(mFeatureFlagsDebug.getString(StringFlag(4, "4", "test", "buz"))).isEqualTo("bar")
assertThat(featureFlagsDebug.getString(StringFlag(1, "1", "test", "biz"))).isEqualTo("biz")
assertThat(featureFlagsDebug.getString(StringFlag(2, "2", "test", "baz"))).isEqualTo("baz")
assertThat(featureFlagsDebug.getString(StringFlag(3, "3", "test", "buz"))).isEqualTo("foo")
assertThat(featureFlagsDebug.getString(StringFlag(4, "4", "test", "buz"))).isEqualTo("bar")
}
@Test
@@ -276,7 +277,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
whenever(flagManager.readFlagValue<String>(eq("6"), any())).thenReturn("override6")
assertThat(
mFeatureFlagsDebug.getString(
featureFlagsDebug.getString(
ResourceStringFlag(
1,
"1",
@@ -286,7 +287,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
)
).isEqualTo("")
assertThat(
mFeatureFlagsDebug.getString(
featureFlagsDebug.getString(
ResourceStringFlag(
2,
"2",
@@ -296,7 +297,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
)
).isEqualTo("resource2")
assertThat(
mFeatureFlagsDebug.getString(
featureFlagsDebug.getString(
ResourceStringFlag(
3,
"3",
@@ -307,15 +308,15 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
).isEqualTo("override3")
Assert.assertThrows(NullPointerException::class.java) {
mFeatureFlagsDebug.getString(ResourceStringFlag(4, "4", "test", 1004))
featureFlagsDebug.getString(ResourceStringFlag(4, "4", "test", 1004))
}
Assert.assertThrows(NameNotFoundException::class.java) {
mFeatureFlagsDebug.getString(ResourceStringFlag(5, "5", "test", 1005))
featureFlagsDebug.getString(ResourceStringFlag(5, "5", "test", 1005))
}
// Test that resource is loaded (and validated) even when the setting is set.
// This prevents developers from not noticing when they reference an invalid resource.
Assert.assertThrows(NameNotFoundException::class.java) {
mFeatureFlagsDebug.getString(ResourceStringFlag(6, "6", "test", 1005))
featureFlagsDebug.getString(ResourceStringFlag(6, "6", "test", 1005))
}
}
@@ -323,10 +324,10 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
fun readIntFlag() {
whenever(flagManager.readFlagValue<Int>(eq("3"), any())).thenReturn(22)
whenever(flagManager.readFlagValue<Int>(eq("4"), any())).thenReturn(48)
assertThat(mFeatureFlagsDebug.getInt(IntFlag(1, "1", "test", 12))).isEqualTo(12)
assertThat(mFeatureFlagsDebug.getInt(IntFlag(2, "2", "test", 93))).isEqualTo(93)
assertThat(mFeatureFlagsDebug.getInt(IntFlag(3, "3", "test", 8))).isEqualTo(22)
assertThat(mFeatureFlagsDebug.getInt(IntFlag(4, "4", "test", 234))).isEqualTo(48)
assertThat(featureFlagsDebug.getInt(IntFlag(1, "1", "test", 12))).isEqualTo(12)
assertThat(featureFlagsDebug.getInt(IntFlag(2, "2", "test", 93))).isEqualTo(93)
assertThat(featureFlagsDebug.getInt(IntFlag(3, "3", "test", 8))).isEqualTo(22)
assertThat(featureFlagsDebug.getInt(IntFlag(4, "4", "test", 234))).isEqualTo(48)
}
@Test
@@ -342,17 +343,17 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
whenever(flagManager.readFlagValue<Int>(eq(4), any())).thenReturn(500)
whenever(flagManager.readFlagValue<Int>(eq(5), any())).thenReturn(9519)
assertThat(mFeatureFlagsDebug.getInt(ResourceIntFlag(1, "1", "test", 1001))).isEqualTo(88)
assertThat(mFeatureFlagsDebug.getInt(ResourceIntFlag(2, "2", "test", 1002))).isEqualTo(61)
assertThat(mFeatureFlagsDebug.getInt(ResourceIntFlag(3, "3", "test", 1003))).isEqualTo(20)
assertThat(featureFlagsDebug.getInt(ResourceIntFlag(1, "1", "test", 1001))).isEqualTo(88)
assertThat(featureFlagsDebug.getInt(ResourceIntFlag(2, "2", "test", 1002))).isEqualTo(61)
assertThat(featureFlagsDebug.getInt(ResourceIntFlag(3, "3", "test", 1003))).isEqualTo(20)
Assert.assertThrows(NotFoundException::class.java) {
mFeatureFlagsDebug.getInt(ResourceIntFlag(4, "4", "test", 1004))
featureFlagsDebug.getInt(ResourceIntFlag(4, "4", "test", 1004))
}
// Test that resource is loaded (and validated) even when the setting is set.
// This prevents developers from not noticing when they reference an invalid resource.
Assert.assertThrows(NotFoundException::class.java) {
mFeatureFlagsDebug.getInt(ResourceIntFlag(5, "5", "test", 1005))
featureFlagsDebug.getInt(ResourceIntFlag(5, "5", "test", 1005))
}
}
@@ -432,11 +433,11 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
whenever(flagManager.readFlagValue<String>(eq("1"), any())).thenReturn("original")
// gets the flag & cache it
assertThat(mFeatureFlagsDebug.getString(flag1)).isEqualTo("original")
assertThat(featureFlagsDebug.getString(flag1)).isEqualTo("original")
verify(flagManager, times(1)).readFlagValue(eq("1"), eq(StringFlagSerializer))
// hit the cache
assertThat(mFeatureFlagsDebug.getString(flag1)).isEqualTo("original")
assertThat(featureFlagsDebug.getString(flag1)).isEqualTo("original")
verifyNoMoreInteractions(flagManager)
// set the flag
@@ -444,7 +445,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
verifyPutData("1", "{\"type\":\"string\",\"value\":\"new\"}", numReads = 2)
whenever(flagManager.readFlagValue<String>(eq("1"), any())).thenReturn("new")
assertThat(mFeatureFlagsDebug.getString(flag1)).isEqualTo("new")
assertThat(featureFlagsDebug.getString(flag1)).isEqualTo("new")
verify(flagManager, times(3)).readFlagValue(eq("1"), eq(StringFlagSerializer))
}
@@ -454,7 +455,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
serverFlagReader.setFlagValue(flag.namespace, flag.name, false)
assertThat(mFeatureFlagsDebug.isEnabled(flag)).isFalse()
assertThat(featureFlagsDebug.isEnabled(flag)).isFalse()
}
@Test
@@ -462,7 +463,33 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
val flag = UnreleasedFlag(100, name = "100", namespace = "test")
serverFlagReader.setFlagValue(flag.namespace, flag.name, true)
assertThat(mFeatureFlagsDebug.isEnabled(flag)).isTrue()
assertThat(featureFlagsDebug.isEnabled(flag)).isTrue()
}
@Test
fun serverSide_OverrideUncached_NoRestart() {
// No one has read the flag, so it's not in the cache.
serverFlagReader.setFlagValue(
teamfoodableFlagA.namespace, teamfoodableFlagA.name, !teamfoodableFlagA.default)
verify(restarter, never()).restartSystemUI(anyString())
}
@Test
fun serverSide_Override_Restarts() {
// Read it to put it in the cache.
featureFlagsDebug.isEnabled(teamfoodableFlagA)
serverFlagReader.setFlagValue(
teamfoodableFlagA.namespace, teamfoodableFlagA.name, !teamfoodableFlagA.default)
verify(restarter).restartSystemUI(anyString())
}
@Test
fun serverSide_RedundantOverride_NoRestart() {
// Read it to put it in the cache.
featureFlagsDebug.isEnabled(teamfoodableFlagA)
serverFlagReader.setFlagValue(
teamfoodableFlagA.namespace, teamfoodableFlagA.name, teamfoodableFlagA.default)
verify(restarter, never()).restartSystemUI(anyString())
}
@Test
@@ -482,13 +509,13 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
.thenReturn("override7")
// WHEN the flags have been accessed
assertThat(mFeatureFlagsDebug.isEnabled(flag1)).isTrue()
assertThat(mFeatureFlagsDebug.isEnabled(flag2)).isTrue()
assertThat(mFeatureFlagsDebug.isEnabled(flag3)).isFalse()
assertThat(mFeatureFlagsDebug.getString(flag4)).isEmpty()
assertThat(mFeatureFlagsDebug.getString(flag5)).isEqualTo("flag5default")
assertThat(mFeatureFlagsDebug.getString(flag6)).isEqualTo("resource1006")
assertThat(mFeatureFlagsDebug.getString(flag7)).isEqualTo("override7")
assertThat(featureFlagsDebug.isEnabled(flag1)).isTrue()
assertThat(featureFlagsDebug.isEnabled(flag2)).isTrue()
assertThat(featureFlagsDebug.isEnabled(flag3)).isFalse()
assertThat(featureFlagsDebug.getString(flag4)).isEmpty()
assertThat(featureFlagsDebug.getString(flag5)).isEqualTo("flag5default")
assertThat(featureFlagsDebug.getString(flag6)).isEqualTo("resource1006")
assertThat(featureFlagsDebug.getString(flag7)).isEqualTo("override7")
// THEN the dump contains the flags and the default values
val dump = dumpToString()
@@ -527,7 +554,7 @@ class FeatureFlagsDebugTest : SysuiTestCase() {
private fun dumpToString(): String {
val sw = StringWriter()
val pw = PrintWriter(sw)
mFeatureFlagsDebug.dump(pw, emptyArray<String>())
featureFlagsDebug.dump(pw, emptyArray<String>())
pw.flush()
return sw.toString()
}

View File

@@ -24,6 +24,8 @@ import org.junit.Assert.assertThrows
import org.junit.Before
import org.junit.Test
import org.mockito.Mock
import org.mockito.Mockito
import org.mockito.Mockito.never
import org.mockito.MockitoAnnotations
import org.mockito.Mockito.`when` as whenever
@@ -33,7 +35,7 @@ import org.mockito.Mockito.`when` as whenever
*/
@SmallTest
class FeatureFlagsReleaseTest : SysuiTestCase() {
private lateinit var mFeatureFlagsRelease: FeatureFlagsRelease
private lateinit var featureFlagsRelease: FeatureFlagsRelease
@Mock private lateinit var mResources: Resources
@Mock private lateinit var mSystemProperties: SystemPropertiesHelper
@@ -41,15 +43,21 @@ class FeatureFlagsReleaseTest : SysuiTestCase() {
private val flagMap = mutableMapOf<String, Flag<*>>()
private val serverFlagReader = ServerFlagReaderFake()
private val flagA = ReleasedFlag(501, name = "a", namespace = "test")
@Before
fun setup() {
MockitoAnnotations.initMocks(this)
mFeatureFlagsRelease = FeatureFlagsRelease(
flagMap.put(flagA.name, flagA)
featureFlagsRelease = FeatureFlagsRelease(
mResources,
mSystemProperties,
serverFlagReader,
flagMap,
restarter)
featureFlagsRelease.init()
}
@Test
@@ -60,7 +68,7 @@ class FeatureFlagsReleaseTest : SysuiTestCase() {
val flagNamespace = "test"
val flag = ResourceBooleanFlag(flagId, flagName, flagNamespace, flagResourceId)
whenever(mResources.getBoolean(flagResourceId)).thenReturn(true)
assertThat(mFeatureFlagsRelease.isEnabled(flag)).isTrue()
assertThat(featureFlagsRelease.isEnabled(flag)).isTrue()
}
@Test
@@ -70,16 +78,16 @@ class FeatureFlagsReleaseTest : SysuiTestCase() {
whenever(mResources.getString(1003)).thenReturn(null)
whenever(mResources.getString(1004)).thenAnswer { throw NameNotFoundException() }
assertThat(mFeatureFlagsRelease.getString(
assertThat(featureFlagsRelease.getString(
ResourceStringFlag(1, "1", "test", 1001))).isEqualTo("")
assertThat(mFeatureFlagsRelease.getString(
assertThat(featureFlagsRelease.getString(
ResourceStringFlag(2, "2", "test", 1002))).isEqualTo("res2")
assertThrows(NullPointerException::class.java) {
mFeatureFlagsRelease.getString(ResourceStringFlag(3, "3", "test", 1003))
featureFlagsRelease.getString(ResourceStringFlag(3, "3", "test", 1003))
}
assertThrows(NameNotFoundException::class.java) {
mFeatureFlagsRelease.getString(ResourceStringFlag(4, "4", "test", 1004))
featureFlagsRelease.getString(ResourceStringFlag(4, "4", "test", 1004))
}
}
@@ -92,7 +100,7 @@ class FeatureFlagsReleaseTest : SysuiTestCase() {
val flag = SysPropBooleanFlag(flagId, flagName, flagNamespace, flagDefault)
whenever(mSystemProperties.getBoolean(flagName, flagDefault)).thenReturn(flagDefault)
assertThat(mFeatureFlagsRelease.isEnabled(flag)).isEqualTo(flagDefault)
assertThat(featureFlagsRelease.isEnabled(flag)).isEqualTo(flagDefault)
}
@Test
@@ -101,7 +109,7 @@ class FeatureFlagsReleaseTest : SysuiTestCase() {
serverFlagReader.setFlagValue(flag.namespace, flag.name, false)
assertThat(mFeatureFlagsRelease.isEnabled(flag)).isFalse()
assertThat(featureFlagsRelease.isEnabled(flag)).isFalse()
}
@Test
@@ -110,6 +118,32 @@ class FeatureFlagsReleaseTest : SysuiTestCase() {
serverFlagReader.setFlagValue(flag.namespace, flag.name, true)
assertThat(mFeatureFlagsRelease.isEnabled(flag)).isFalse()
assertThat(featureFlagsRelease.isEnabled(flag)).isFalse()
}
@Test
fun serverSide_OverrideUncached_NoRestart() {
// No one has read the flag, so it's not in the cache.
serverFlagReader.setFlagValue(
flagA.namespace, flagA.name, !flagA.default)
Mockito.verify(restarter, never()).restartSystemUI(Mockito.anyString())
}
@Test
fun serverSide_Override_Restarts() {
// Read it to put it in the cache.
featureFlagsRelease.isEnabled(flagA)
serverFlagReader.setFlagValue(
flagA.namespace, flagA.name, !flagA.default)
Mockito.verify(restarter).restartSystemUI(Mockito.anyString())
}
@Test
fun serverSide_RedundantOverride_NoRestart() {
// Read it to put it in the cache.
featureFlagsRelease.isEnabled(flagA)
serverFlagReader.setFlagValue(
flagA.namespace, flagA.name, flagA.default)
Mockito.verify(restarter, never()).restartSystemUI(Mockito.anyString())
}
}

View File

@@ -21,11 +21,13 @@ import android.testing.AndroidTestingRunner
import com.android.systemui.SysuiTestCase
import com.android.systemui.util.DeviceConfigProxyFake
import com.android.systemui.util.concurrency.FakeExecutor
import com.android.systemui.util.mockito.any
import com.android.systemui.util.time.FakeSystemClock
import org.junit.Before
import org.junit.Test
import org.junit.runner.RunWith
import org.mockito.Mock
import org.mockito.Mockito.anyString
import org.mockito.Mockito.never
import org.mockito.Mockito.verify
import org.mockito.MockitoAnnotations
@@ -57,18 +59,18 @@ class ServerFlagReaderImplTest : SysuiTestCase() {
deviceConfig.setProperty(NAMESPACE, "flag_1", "1", false)
executor.runAllReady()
verify(changeListener).onChange(flag)
verify(changeListener).onChange(flag, "1")
}
@Test
fun testChange_ignoresListenersDuringTest() {
val serverFlagReader = ServerFlagReaderImpl(NAMESPACE, deviceConfig, executor, true)
val flag = ReleasedFlag(1, "1", "test")
val flag = ReleasedFlag(1, "1", " test")
serverFlagReader.listenForChanges(listOf(flag), changeListener)
deviceConfig.setProperty(NAMESPACE, "flag_override_1", "1", false)
executor.runAllReady()
verify(changeListener, never()).onChange(flag)
verify(changeListener, never()).onChange(any(), anyString())
}
}