LineageOS/android_packages_modules_Connectivity
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge "Fix validation URL flakes with DeviceConfigRule" am: 39e121cc88

Browse Source 
Original change: https://android-review.googlesource.com/c/platform/packages/modules/Connectivity/+/2110144

Change-Id: Ice252371c114373e879f14ec2025886aad2217c5
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
This commit is contained in:
Remi NGUYEN VAN
2022-06-20 08:57:37 +00:00
committed by Automerger Merge Worker
parent 29287c5b75 39e121cc88
commit 156ddb01a4
4 changed files with 199 additions and 105 deletions
Download Patch File Download Diff File Expand all files Collapse all files

52
tests/cts/net/src/android/net/cts/CaptivePortalTest.kt
View File

@@ -33,7 +33,6 @@ import android.net.NetworkCapabilities.TRANSPORT_CELLULAR
import android.net.NetworkCapabilities.TRANSPORT_WIFI
import android.net.NetworkRequest
import android.net.Uri
import android.net.cts.NetworkValidationTestUtil.clearValidationTestUrlsDeviceConfig
import android.net.cts.NetworkValidationTestUtil.setHttpUrlDeviceConfig
import android.net.cts.NetworkValidationTestUtil.setHttpsUrlDeviceConfig
import android.net.cts.NetworkValidationTestUtil.setUrlExpirationDeviceConfig
@@ -60,6 +59,8 @@ import org.junit.After
import org.junit.Assume.assumeTrue
import org.junit.Assume.assumeFalse
import org.junit.Before
import org.junit.BeforeClass
import org.junit.Rule
import org.junit.runner.RunWith
import java.util.concurrent.CompletableFuture
import java.util.concurrent.TimeUnit
@@ -99,34 +100,42 @@ class CaptivePortalTest {
private val server = TestHttpServer("localhost")
@get:Rule
val deviceConfigRule = DeviceConfigRule(retryCountBeforeSIfConfigChanged = 5)
companion object {
@JvmStatic @BeforeClass
fun setUpClass() {
runAsShell(READ_DEVICE_CONFIG) {
// Verify that the test URLs are not normally set on the device, but do not fail if
// the test URLs are set to what this test uses (URLs on localhost), in case the
// test was interrupted manually and rerun.
assertEmptyOrLocalhostUrl(TEST_CAPTIVE_PORTAL_HTTPS_URL)
assertEmptyOrLocalhostUrl(TEST_CAPTIVE_PORTAL_HTTP_URL)
}
NetworkValidationTestUtil.clearValidationTestUrlsDeviceConfig()
}
private fun assertEmptyOrLocalhostUrl(urlKey: String) {
val url = DeviceConfig.getProperty(NAMESPACE_CONNECTIVITY, urlKey)
assertTrue(TextUtils.isEmpty(url) || LOCALHOST_HOSTNAME == Uri.parse(url).host,
"$urlKey must not be set in production scenarios (current value: $url)")
}
}
@Before
fun setUp() {
runAsShell(READ_DEVICE_CONFIG) {
// Verify that the test URLs are not normally set on the device, but do not fail if the
// test URLs are set to what this test uses (URLs on localhost), in case the test was
// interrupted manually and rerun.
assertEmptyOrLocalhostUrl(TEST_CAPTIVE_PORTAL_HTTPS_URL)
assertEmptyOrLocalhostUrl(TEST_CAPTIVE_PORTAL_HTTP_URL)
}
clearValidationTestUrlsDeviceConfig()
server.start()
}
@After
fun tearDown() {
clearValidationTestUrlsDeviceConfig()
if (pm.hasSystemFeature(FEATURE_WIFI)) {
reconnectWifi()
deviceConfigRule.runAfterNextCleanup { reconnectWifi() }
}
server.stop()
}
private fun assertEmptyOrLocalhostUrl(urlKey: String) {
val url = DeviceConfig.getProperty(NAMESPACE_CONNECTIVITY, urlKey)
assertTrue(TextUtils.isEmpty(url) || LOCALHOST_HOSTNAME == Uri.parse(url).host,
"$urlKey must not be set in production scenarios (current value: $url)")
}
@Test
fun testCaptivePortalIsNotDefaultNetwork() {
assumeTrue(pm.hasSystemFeature(FEATURE_TELEPHONY))
@@ -154,12 +163,13 @@ class CaptivePortalTest {
server.addResponse(Request(TEST_HTTPS_URL_PATH), Status.INTERNAL_ERROR)
val headers = mapOf("Location" to makeUrl(TEST_PORTAL_URL_PATH))
server.addResponse(Request(TEST_HTTP_URL_PATH), Status.REDIRECT, headers)
setHttpsUrlDeviceConfig(makeUrl(TEST_HTTPS_URL_PATH))
setHttpUrlDeviceConfig(makeUrl(TEST_HTTP_URL_PATH))
setHttpsUrlDeviceConfig(deviceConfigRule, makeUrl(TEST_HTTPS_URL_PATH))
setHttpUrlDeviceConfig(deviceConfigRule, makeUrl(TEST_HTTP_URL_PATH))
Log.d(TAG, "Set portal URLs to $TEST_HTTPS_URL_PATH and $TEST_HTTP_URL_PATH")
// URL expiration needs to be in the next 10 minutes
assertTrue(WIFI_CONNECT_TIMEOUT_MS < TimeUnit.MINUTES.toMillis(10))
setUrlExpirationDeviceConfig(System.currentTimeMillis() + WIFI_CONNECT_TIMEOUT_MS)
setUrlExpirationDeviceConfig(deviceConfigRule,
System.currentTimeMillis() + WIFI_CONNECT_TIMEOUT_MS)
// Wait for a captive portal to be detected on the network
val wifiNetworkFuture = CompletableFuture<Network>()
@@ -215,4 +225,4 @@ class CaptivePortalTest {
utils.ensureWifiDisconnected(null /* wifiNetworkToCheck */)
utils.ensureWifiConnected()
}
}
}

43
tests/cts/net/src/android/net/cts/ConnectivityManagerTest.java
View File

@@ -249,6 +249,10 @@ public class ConnectivityManagerTest {
@Rule
public final DevSdkIgnoreRule ignoreRule = new DevSdkIgnoreRule();
@Rule
public final DeviceConfigRule mTestValidationConfigRule = new DeviceConfigRule(
5 /* retryCountBeforeSIfConfigChanged */);
private static final String TAG = ConnectivityManagerTest.class.getSimpleName();
public static final int TYPE_MOBILE = ConnectivityManager.TYPE_MOBILE;
@@ -2765,9 +2769,8 @@ public class ConnectivityManagerTest {
// Accept partial connectivity network should result in a validated network
expectNetworkHasCapability(network, NET_CAPABILITY_VALIDATED, WIFI_CONNECT_TIMEOUT_MS);
} finally {
resetValidationConfig();
// Reconnect wifi to reset the wifi status
reconnectWifi();
mHttpServer.stop();
mTestValidationConfigRule.runAfterNextCleanup(this::reconnectWifi);
}
}
@@ -2792,11 +2795,13 @@ public class ConnectivityManagerTest {
// Reject partial connectivity network should cause the network being torn down
assertEquals(network, cb.waitForLost());
} finally {
resetValidationConfig();
mHttpServer.stop();
// Wifi will not automatically reconnect to the network. ensureWifiDisconnected cannot
// apply here. Thus, turn off wifi first and restart to restore.
runShellCommand("svc wifi disable");
mCtsNetUtils.ensureWifiConnected();
mTestValidationConfigRule.runAfterNextCleanup(() -> {
runShellCommand("svc wifi disable");
mCtsNetUtils.ensureWifiConnected();
});
}
}
@@ -2832,11 +2837,13 @@ public class ConnectivityManagerTest {
});
waitForLost(wifiCb);
} finally {
resetValidationConfig();
mHttpServer.stop();
/// Wifi will not automatically reconnect to the network. ensureWifiDisconnected cannot
// apply here. Thus, turn off wifi first and restart to restore.
runShellCommand("svc wifi disable");
mCtsNetUtils.ensureWifiConnected();
mTestValidationConfigRule.runAfterNextCleanup(() -> {
runShellCommand("svc wifi disable");
mCtsNetUtils.ensureWifiConnected();
});
}
}
@@ -2896,9 +2903,8 @@ public class ConnectivityManagerTest {
wifiCb.assertNoCallbackThat(NO_CALLBACK_TIMEOUT_MS, c -> isValidatedCaps(c));
} finally {
resetAvoidBadWifi(previousAvoidBadWifi);
resetValidationConfig();
// Reconnect wifi to reset the wifi status
reconnectWifi();
mHttpServer.stop();
mTestValidationConfigRule.runAfterNextCleanup(this::reconnectWifi);
}
}
@@ -2942,11 +2948,6 @@ public class ConnectivityManagerTest {
return future.get(timeout, TimeUnit.MILLISECONDS);
}
private void resetValidationConfig() {
NetworkValidationTestUtil.clearValidationTestUrlsDeviceConfig();
mHttpServer.stop();
}
private void prepareHttpServer() throws Exception {
runAsShell(READ_DEVICE_CONFIG, () -> {
// Verify that the test URLs are not normally set on the device, but do not fail if the
@@ -3019,9 +3020,11 @@ public class ConnectivityManagerTest {
mHttpServer.addResponse(new TestHttpServer.Request(
TEST_HTTP_URL_PATH, Method.GET, "" /* queryParameters */),
httpStatusCode, null /* locationHeader */, "" /* content */);
NetworkValidationTestUtil.setHttpsUrlDeviceConfig(makeUrl(TEST_HTTPS_URL_PATH));
NetworkValidationTestUtil.setHttpUrlDeviceConfig(makeUrl(TEST_HTTP_URL_PATH));
NetworkValidationTestUtil.setUrlExpirationDeviceConfig(
NetworkValidationTestUtil.setHttpsUrlDeviceConfig(mTestValidationConfigRule,
makeUrl(TEST_HTTPS_URL_PATH));
NetworkValidationTestUtil.setHttpUrlDeviceConfig(mTestValidationConfigRule,
makeUrl(TEST_HTTP_URL_PATH));
NetworkValidationTestUtil.setUrlExpirationDeviceConfig(mTestValidationConfigRule,
System.currentTimeMillis() + WIFI_CONNECT_TIMEOUT_MS);
}

124
tests/cts/net/src/android/net/cts/DeviceConfigRule.kt Normal file
View File

@@ -0,0 +1,124 @@
/*
* Copyright (C) 2022 The Android Open Source Project
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package android.net.cts
import android.Manifest.permission.READ_DEVICE_CONFIG
import android.Manifest.permission.WRITE_DEVICE_CONFIG
import android.provider.DeviceConfig
import android.util.Log
import com.android.modules.utils.build.SdkLevel
import com.android.testutils.runAsShell
import com.android.testutils.tryTest
import org.junit.rules.TestRule
import org.junit.runner.Description
import org.junit.runners.model.Statement
private val TAG = DeviceConfigRule::class.simpleName
/**
* A [TestRule] that helps set [DeviceConfig] for tests and clean up the test configuration
* automatically on teardown.
*
* The rule can also optionally retry tests when they fail following an external change of
* DeviceConfig before S; this typically happens because device config flags are synced while the
* test is running, and DisableConfigSyncTargetPreparer is only usable starting from S.
*
* @param retryCountBeforeSIfConfigChanged if > 0, when the test fails before S, check if
* the configs that were set through this rule were changed, and retry the test
* up to the specified number of times if yes.
*/
class DeviceConfigRule @JvmOverloads constructor(
val retryCountBeforeSIfConfigChanged: Int = 0
) : TestRule {
// Maps (namespace, key) -> value
private val originalConfig = mutableMapOf<Pair<String, String>, String?>()
private val usedConfig = mutableMapOf<Pair<String, String>, String?>()
/**
* Actions to be run after cleanup of the config, for the current test only.
*/
private val currentTestCleanupActions = mutableListOf<Runnable>()
override fun apply(base: Statement, description: Description): Statement {
return TestValidationUrlStatement(base, description)
}
private inner class TestValidationUrlStatement(
private val base: Statement,
private val description: Description
) : Statement() {
override fun evaluate() {
var retryCount = if (SdkLevel.isAtLeastS()) 1 else retryCountBeforeSIfConfigChanged + 1
while (retryCount > 0) {
retryCount--
tryTest {
base.evaluate()
// Can't use break/return out of a loop here because this is a tryTest lambda,
// so set retryCount to exit instead
retryCount = 0
}.catch<Throwable> { e -> // junit AssertionFailedError does not extend Exception
if (retryCount == 0) throw e
usedConfig.forEach { (key, value) ->
val currentValue = runAsShell(READ_DEVICE_CONFIG) {
DeviceConfig.getProperty(key.first, key.second)
}
if (currentValue != value) {
Log.w(TAG, "Test failed with unexpected device config change, retrying")
return@catch
}
}
throw e
} cleanupStep {
runAsShell(WRITE_DEVICE_CONFIG) {
originalConfig.forEach { (key, value) ->
DeviceConfig.setProperty(
key.first, key.second, value, false /* makeDefault */)
}
}
} cleanupStep {
originalConfig.clear()
usedConfig.clear()
} cleanup {
currentTestCleanupActions.forEach { it.run() }
currentTestCleanupActions.clear()
}
}
}
}
/**
* Set a configuration key/value. After the test case ends, it will be restored to the value it
* had when this method was first called.
*/
fun setConfig(namespace: String, key: String, value: String?) {
runAsShell(READ_DEVICE_CONFIG, WRITE_DEVICE_CONFIG) {
val keyPair = Pair(namespace, key)
if (!originalConfig.containsKey(keyPair)) {
originalConfig[keyPair] = DeviceConfig.getProperty(namespace, key)
}
usedConfig[keyPair] = value
DeviceConfig.setProperty(namespace, key, value, false /* makeDefault */)
}
}
/**
* Add an action to be run after config cleanup when the current test case ends.
*/
fun runAfterNextCleanup(action: Runnable) {
currentTestCleanupActions.add(action)
}
}

85
tests/cts/net/src/android/net/cts/NetworkValidationTestUtil.kt
View File

@@ -16,16 +16,11 @@
package android.net.cts
import android.Manifest
import android.Manifest.permission.WRITE_DEVICE_CONFIG
import android.net.util.NetworkStackUtils
import android.provider.DeviceConfig
import android.provider.DeviceConfig.NAMESPACE_CONNECTIVITY
import android.util.Log
import com.android.testutils.runAsShell
import com.android.testutils.tryTest
import java.util.concurrent.CompletableFuture
import java.util.concurrent.Executor
import java.util.concurrent.TimeUnit
/**
* Collection of utility methods for configuring network validation.
@@ -38,9 +33,14 @@ internal object NetworkValidationTestUtil {
* Clear the test network validation URLs.
*/
@JvmStatic fun clearValidationTestUrlsDeviceConfig() {
setHttpsUrlDeviceConfig(null)
setHttpUrlDeviceConfig(null)
setUrlExpirationDeviceConfig(null)
runAsShell(WRITE_DEVICE_CONFIG) {
DeviceConfig.setProperty(NAMESPACE_CONNECTIVITY,
NetworkStackUtils.TEST_CAPTIVE_PORTAL_HTTPS_URL, null, false)
DeviceConfig.setProperty(NAMESPACE_CONNECTIVITY,
NetworkStackUtils.TEST_CAPTIVE_PORTAL_HTTP_URL, null, false)
DeviceConfig.setProperty(NAMESPACE_CONNECTIVITY,
NetworkStackUtils.TEST_URL_EXPIRATION_TIME, null, false)
}
}
/**
@@ -48,71 +48,28 @@ internal object NetworkValidationTestUtil {
*
* @see NetworkStackUtils.TEST_CAPTIVE_PORTAL_HTTPS_URL
*/
@JvmStatic fun setHttpsUrlDeviceConfig(url: String?) =
setConfig(NetworkStackUtils.TEST_CAPTIVE_PORTAL_HTTPS_URL, url)
@JvmStatic
fun setHttpsUrlDeviceConfig(rule: DeviceConfigRule, url: String?) =
rule.setConfig(NAMESPACE_CONNECTIVITY,
NetworkStackUtils.TEST_CAPTIVE_PORTAL_HTTPS_URL, url)
/**
* Set the test validation HTTP URL.
*
* @see NetworkStackUtils.TEST_CAPTIVE_PORTAL_HTTP_URL
*/
@JvmStatic fun setHttpUrlDeviceConfig(url: String?) =
setConfig(NetworkStackUtils.TEST_CAPTIVE_PORTAL_HTTP_URL, url)
@JvmStatic
fun setHttpUrlDeviceConfig(rule: DeviceConfigRule, url: String?) =
rule.setConfig(NAMESPACE_CONNECTIVITY,
NetworkStackUtils.TEST_CAPTIVE_PORTAL_HTTP_URL, url)
/**
* Set the test validation URL expiration.
*
* @see NetworkStackUtils.TEST_URL_EXPIRATION_TIME
*/
@JvmStatic fun setUrlExpirationDeviceConfig(timestamp: Long?) =
setConfig(NetworkStackUtils.TEST_URL_EXPIRATION_TIME, timestamp?.toString())
private fun setConfig(configKey: String, value: String?): String? {
Log.i(TAG, "Setting config \"$configKey\" to \"$value\"")
val readWritePermissions = arrayOf(
Manifest.permission.READ_DEVICE_CONFIG,
Manifest.permission.WRITE_DEVICE_CONFIG)
val existingValue = runAsShell(*readWritePermissions) {
DeviceConfig.getProperty(NAMESPACE_CONNECTIVITY, configKey)
}
if (existingValue == value) {
// Already the correct value. There may be a race if a change is already in flight,
// but if multiple threads update the config there is no way to fix that anyway.
Log.i(TAG, "\$configKey\" already had value \"$value\"")
return value
}
val future = CompletableFuture<String>()
val listener = DeviceConfig.OnPropertiesChangedListener {
// The listener receives updates for any change to any key, so don't react to
// changes that do not affect the relevant key
if (!it.keyset.contains(configKey)) return@OnPropertiesChangedListener
if (it.getString(configKey, null) == value) {
future.complete(value)
}
}
return tryTest {
runAsShell(*readWritePermissions) {
DeviceConfig.addOnPropertiesChangedListener(
NAMESPACE_CONNECTIVITY,
inlineExecutor,
listener)
DeviceConfig.setProperty(
NAMESPACE_CONNECTIVITY,
configKey,
value,
false /* makeDefault */)
// Don't drop the permission until the config is applied, just in case
future.get(TIMEOUT_MS, TimeUnit.MILLISECONDS)
}.also {
Log.i(TAG, "Config \"$configKey\" successfully set to \"$value\"")
}
} cleanup {
DeviceConfig.removeOnPropertiesChangedListener(listener)
}
}
private val inlineExecutor get() = Executor { r -> r.run() }
@JvmStatic
fun setUrlExpirationDeviceConfig(rule: DeviceConfigRule, timestamp: Long?) =
rule.setConfig(NAMESPACE_CONNECTIVITY,
NetworkStackUtils.TEST_URL_EXPIRATION_TIME, timestamp?.toString())
}