Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 4715bc36 authored by Caitlin Cassidy's avatar Caitlin Cassidy
Browse files

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

change.

Fixes: 211696714
Test: DisableFlagsLoggerTest
Test: manual
Change-Id: I72eb5a92723bc3477a8d56af1910364d13d2d86f
parent 3398ba9c
Loading
Loading
Loading
Loading
+29 −29
Original line number Diff line number Diff line
@@ -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(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))
        }

        return builder.toString()
            builder.append(getFlagsString(newAfterLocalModification))
            builder.append(" ")
            builder.append(getDiffString(new, newAfterLocalModification))
        }

    /**
     * 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) {
+8 −10
Original line number Diff line number Diff line
@@ -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