From ac292113625484c191caac960b4b52d042463044 Mon Sep 17 00:00:00 2001 From: Patrick Rohr Date: Wed, 16 Jul 2025 10:49:13 -0700 Subject: [PATCH] Fix permissions in RestoreEthernetRule The previous runAsShell calls adopted permissions spuriously. This change fixes runAsShell to consistently adopt the correct permission for the respective API call. Also calls setEthernetEnabled() no matter what the current state is at the end of the test (as there is no harm in doing so). Test: TH Flag: EXEMPT mainline Change-Id: Ib11b889148213b160567cf0a3b9ea72b5b0c851e --- .../testutils/RestoreEthernetRuleTest.kt | 16 -------- .../android/testutils/RestoreEthernetRule.kt | 40 ++++++++++--------- 2 files changed, 22 insertions(+), 34 deletions(-) diff --git a/staticlibs/tests/unit/src/com/android/testutils/RestoreEthernetRuleTest.kt b/staticlibs/tests/unit/src/com/android/testutils/RestoreEthernetRuleTest.kt index 3b5fb2795f..5fb3bbc2e9 100644 --- a/staticlibs/tests/unit/src/com/android/testutils/RestoreEthernetRuleTest.kt +++ b/staticlibs/tests/unit/src/com/android/testutils/RestoreEthernetRuleTest.kt @@ -27,7 +27,6 @@ import org.mockito.kotlin.doCallRealMethod import org.mockito.kotlin.doReturn import org.mockito.kotlin.inOrder import org.mockito.kotlin.mock -import org.mockito.kotlin.never import org.mockito.kotlin.verify // EthernetManager#addEthernetStateListener is not supported before T. @@ -42,15 +41,6 @@ class RestoreEthernetRuleTest { on { apply(any(), any()) }.thenCallRealMethod() } - private fun doTestApply_stateUnchanged(initialState: Boolean) { - doReturn(initialState, initialState).`when`(rule).isEthernetEnabled() - - rule.apply(mockStatement, mockDescription).evaluate() - - verify(mockStatement).evaluate() - verify(rule, never()).setEthernetEnabled(any()) - } - private fun doTestApply_restoresInitialState(initialState: Boolean) { doReturn(initialState, !initialState).`when`(rule).isEthernetEnabled() @@ -61,12 +51,6 @@ class RestoreEthernetRuleTest { inOrder.verify(rule).setEthernetEnabled(initialState) } - @Test - fun testApply_stateUnchanged_wasEnabled() = doTestApply_stateUnchanged(initialState = true) - - @Test - fun testApply_stateUnchanged_wasDisabled() = doTestApply_stateUnchanged(initialState = false) - @Test fun testApply_restoresInitialState_wasEnabled() = doTestApply_restoresInitialState(initialState = true) diff --git a/staticlibs/testutils/devicetests/com/android/testutils/RestoreEthernetRule.kt b/staticlibs/testutils/devicetests/com/android/testutils/RestoreEthernetRule.kt index 80e3b9214a..8384e561bd 100644 --- a/staticlibs/testutils/devicetests/com/android/testutils/RestoreEthernetRule.kt +++ b/staticlibs/testutils/devicetests/com/android/testutils/RestoreEthernetRule.kt @@ -15,6 +15,7 @@ */ package com.android.testutils +import android.Manifest.permission.ACCESS_NETWORK_STATE import android.Manifest.permission.NETWORK_SETTINGS import android.annotation.SuppressLint import android.content.Context @@ -23,8 +24,7 @@ import android.net.EthernetManager.ETHERNET_STATE_DISABLED import android.net.EthernetManager.ETHERNET_STATE_ENABLED import android.os.SystemProperties import com.android.testutils.TestEthernetStateListener.EthernetStateChanged -import java.lang.IllegalArgumentException -import java.util.concurrent.Executor +import java.util.function.IntConsumer import org.junit.rules.TestRule import org.junit.runner.Description import org.junit.runners.model.Statement @@ -42,7 +42,6 @@ open class RestoreEthernetRule(private val context: Context) : TestRule { // Callers should ensure isEthernetSupported() before invoking anything needs // the EthernetManager instance. Otherwise, this would throw. private val em by lazy { context.getSystemService(EthernetManager::class.java)!! } - private val inlineExecutor = Executor { r -> r.run() } override fun apply(base: Statement, description: Description): Statement { return object : Statement() { @@ -51,14 +50,24 @@ open class RestoreEthernetRule(private val context: Context) : TestRule { try { base.evaluate() } finally { - if (wasEnabled != isEthernetEnabled()) { - setEthernetEnabled(wasEnabled) - } + setEthernetEnabled(wasEnabled) } } } } + private fun addEthernetStateListener(listener: IntConsumer) { + runAsShell(ACCESS_NETWORK_STATE) { + em.addEthernetStateListener({ r -> r.run() }, listener) + } + } + + private fun removeEthernetStateListener(listener: IntConsumer) { + runAsShell(ACCESS_NETWORK_STATE) { + em.removeEthernetStateListener(listener) + } + } + /** * Sets the ethernet enabled state and waits for the change to be confirmed. * @@ -66,18 +75,17 @@ open class RestoreEthernetRule(private val context: Context) : TestRule { */ @SuppressLint("MissingPermission") open fun setEthernetEnabled(enabled: Boolean) { - if (isAdbOverEthernet() && !enabled) { - throw IllegalArgumentException("Ethernet should not be disabled when adb over ethernet") - } + if (!enabled) require(!isAdbOverEthernet()) + + runAsShell(NETWORK_SETTINGS) { setEthernetEnabled(enabled) } val listener = TestEthernetStateListener() - em.addEthernetStateListener(inlineExecutor, listener) + addEthernetStateListener(listener) try { - runAsShell(NETWORK_SETTINGS) { em.setEthernetEnabled(enabled) } listener.eventuallyExpect { it.state == if (enabled) ETHERNET_STATE_ENABLED else ETHERNET_STATE_DISABLED } } finally { - em.removeEthernetStateListener(listener) + removeEthernetStateListener(listener) } } @@ -100,16 +108,12 @@ open class RestoreEthernetRule(private val context: Context) : TestRule { @SuppressLint("MissingPermission") open fun isEthernetEnabled(): Boolean { val listener = TestEthernetStateListener() - runAsShell(NETWORK_SETTINGS) { - em.addEthernetStateListener(inlineExecutor, listener) - } + addEthernetStateListener(listener) try { val event = listener.expect() return event.state == ETHERNET_STATE_ENABLED } finally { - runAsShell(NETWORK_SETTINGS) { - em.removeEthernetStateListener(listener) - } + removeEthernetStateListener(listener) } } }