Merge "[Disable Flags] Update logging to make it clear when there's been no change."

This commit is contained in:
Caitlin Cassidy
2022-01-07 16:28:12 +00:00
committed by Android (Google) Code Review
2 changed files with 37 additions and 39 deletions

View File

@@ -74,9 +74,12 @@ class DisableFlagsLogger constructor(
* Returns a string representing the, old, new, and new-after-modification disable flag states,
* as well as the differences between each of the states.
*
* Example:
* Old: EnaiHbcRso.qINgr | New: EnaihBcRso.qiNGR (hB.iGR) | New after local modification:
* EnaihBcRso.qInGR (.n)
* Example if [old], [new], and [newAfterLocalModification] are all different:
* Old: EnaiHbcRso.qINgr | New: EnaihBcRso.qiNGR (changed: hB.iGR) | New after local
* modification: EnaihBcRso.qInGR (changed: .n)
*
* Example if [old] and [new] are the same:
* EnaihBcRso.qiNGR (unchanged)
*
* A capital character signifies the flag is set and a lowercase character signifies that the
* flag isn't set. The flag states will be logged in the same order as the passed-in lists.
@@ -96,54 +99,51 @@ class DisableFlagsLogger constructor(
new: DisableState,
newAfterLocalModification: DisableState? = null
): String {
val builder = StringBuilder("Received new disable state. ")
val builder = StringBuilder("Received new disable state: ")
old?.let {
// This if/else has slightly repetitive code but is easier to read.
if (old != null && old != new) {
builder.append("Old: ")
builder.append(getFlagsString(old))
builder.append(" | ")
}
builder.append("New: ")
if (old != null && old != new) {
builder.append(getFlagsStringWithDiff(old, new))
} else {
builder.append("New: ")
builder.append(getFlagsString(new))
builder.append(" ")
builder.append(getDiffString(old, new))
} else if (old != null && old == new) {
// If old and new are the same, we only need to print one of them.
builder.append(getFlagsString(new))
builder.append(" ")
builder.append(getDiffString(old, new))
} else { // old == null
builder.append(getFlagsString(new))
// Don't get a diff string because we have no [old] to compare with.
}
if (newAfterLocalModification != null && new != newAfterLocalModification) {
builder.append(" | New after local modification: ")
builder.append(getFlagsStringWithDiff(new, newAfterLocalModification))
builder.append(getFlagsString(newAfterLocalModification))
builder.append(" ")
builder.append(getDiffString(new, newAfterLocalModification))
}
return builder.toString()
}
/**
* Returns a string representing [new] state, as well as the difference from [old] to [new]
* (if there is one).
*/
private fun getFlagsStringWithDiff(old: DisableState, new: DisableState): String {
val builder = StringBuilder()
builder.append(getFlagsString(new))
builder.append(" ")
builder.append(getDiffString(old, new))
return builder.toString()
}
/**
* Returns a string representing the difference between [old] and [new], or an empty string if
* there is no difference.
* Returns a string representing the difference between [old] and [new].
*
* For example, if old was "abc.DE" and new was "aBC.De", the difference returned would be
* "(BC.e)".
* - If [old] was "abc.DE" and [new] was "aBC.De", the difference returned would be
* "(changed: BC.e)".
* - If [old] and [new] are the same, the difference returned would be "(unchanged)".
*/
private fun getDiffString(old: DisableState, new: DisableState): String {
if (old == new) {
return ""
return "(unchanged)"
}
val builder = StringBuilder("(")
builder.append("changed: ")
disable1FlagsList.forEach {
val newSymbol = it.getFlagStatus(new.disable1)
if (it.getFlagStatus(old.disable1) != newSymbol) {

View File

@@ -37,7 +37,7 @@ class DisableFlagsLoggerTest : SysuiTestCase() {
private val disableFlagsLogger = DisableFlagsLogger(disable1Flags, disable2Flags)
@Test
fun getDisableFlagsString_oldAndNewSame_statesLoggedButDiffsNotLogged() {
fun getDisableFlagsString_oldAndNewSame_newAndUnchangedLoggedOldNotLogged() {
val state = DisableFlagsLogger.DisableState(
0b111, // ABC
0b01 // mN
@@ -45,10 +45,9 @@ class DisableFlagsLoggerTest : SysuiTestCase() {
val result = disableFlagsLogger.getDisableFlagsString(state, state)
assertThat(result).contains("Old: ABC.mN")
assertThat(result).contains("New: ABC.mN")
assertThat(result).doesNotContain("(")
assertThat(result).doesNotContain(")")
assertThat(result).doesNotContain("Old")
assertThat(result).contains("ABC.mN")
assertThat(result).contains("(unchanged)")
}
@Test
@@ -66,7 +65,7 @@ class DisableFlagsLoggerTest : SysuiTestCase() {
assertThat(result).contains("Old: ABC.mN")
assertThat(result).contains("New: abC.Mn")
assertThat(result).contains("(ab.Mn)")
assertThat(result).contains("(changed: ab.Mn)")
}
@Test
@@ -82,7 +81,7 @@ class DisableFlagsLoggerTest : SysuiTestCase() {
)
)
assertThat(result).contains("(.n)")
assertThat(result).contains("(changed: .n)")
}
@Test
@@ -96,8 +95,7 @@ class DisableFlagsLoggerTest : SysuiTestCase() {
)
assertThat(result).doesNotContain("Old")
assertThat(result).contains("New: abC.mN")
// We have no state to diff on, so we shouldn't see any diff in parentheses
assertThat(result).contains("abC.mN")
assertThat(result).doesNotContain("(")
assertThat(result).doesNotContain(")")
}
@@ -141,7 +139,7 @@ class DisableFlagsLoggerTest : SysuiTestCase() {
)
)
assertThat(result).contains("local modification: Abc.Mn (A.M)")
assertThat(result).contains("local modification: Abc.Mn (changed: A.M)")
}
@Test