Fix EthernetTetheringTest testTestNetworkUpstream flaky

The test assume the first tethering upstream would be test network. But
if there are mutiple networks available, this is a wrong assumption
because of race condtion. If tethering choose upstream while wifi or
celluar is available but test network may not available for tethering yet
(it depend on the NetworkCallback from ConnenectivityService), the first
upstream may not be the test network.

Bug: 215224286

Test: atest EthernetTetheringTest
Change-Id: Idfa972008643f1fb5119179383b06e2c8f65c667
This commit is contained in:
markchien
2021-11-03 12:33:25 +08:00
parent 2606d9c862
commit 95cb089853

View File

@@ -106,6 +106,7 @@ import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Random;
import java.util.Set;
import java.util.concurrent.CompletableFuture;
@@ -278,7 +279,7 @@ public class EthernetTetheringTest {
final String localAddr = "192.0.2.3/28";
final String clientAddr = "192.0.2.2/28";
mTetheringEventCallback = enableEthernetTethering(iface,
requestWithStaticIpv4(localAddr, clientAddr));
requestWithStaticIpv4(localAddr, clientAddr), null /* any upstream */);
mTetheringEventCallback.awaitInterfaceTethered();
assertInterfaceHasIpAddress(iface, localAddr);
@@ -361,7 +362,8 @@ public class EthernetTetheringTest {
final TetheringRequest request = new TetheringRequest.Builder(TETHERING_ETHERNET)
.setConnectivityScope(CONNECTIVITY_SCOPE_LOCAL).build();
mTetheringEventCallback = enableEthernetTethering(iface, request);
mTetheringEventCallback = enableEthernetTethering(iface, request,
null /* any upstream */);
mTetheringEventCallback.awaitInterfaceLocalOnly();
// makePacketReader only works after tethering is started, because until then the interface
@@ -392,7 +394,7 @@ public class EthernetTetheringTest {
final String iface = mTetheredInterfaceRequester.getInterface();
// Enable Ethernet tethering and check that it starts.
mTetheringEventCallback = enableEthernetTethering(iface);
mTetheringEventCallback = enableEthernetTethering(iface, null /* any upstream */);
// There is nothing more we can do on a physical interface without connecting an actual
// client, which is not possible in this test.
@@ -405,8 +407,11 @@ public class EthernetTetheringTest {
private final CountDownLatch mLocalOnlyStartedLatch = new CountDownLatch(1);
private final CountDownLatch mLocalOnlyStoppedLatch = new CountDownLatch(1);
private final CountDownLatch mClientConnectedLatch = new CountDownLatch(1);
private final CountDownLatch mUpstreamConnectedLatch = new CountDownLatch(1);
private final CountDownLatch mUpstreamLatch = new CountDownLatch(1);
private final TetheringInterface mIface;
private final Network mExpectedUpstream;
private boolean mAcceptAnyUpstream = false;
private volatile boolean mInterfaceWasTethered = false;
private volatile boolean mInterfaceWasLocalOnly = false;
@@ -415,8 +420,14 @@ public class EthernetTetheringTest {
private volatile Network mUpstream = null;
MyTetheringEventCallback(TetheringManager tm, String iface) {
this(tm, iface, null);
mAcceptAnyUpstream = true;
}
MyTetheringEventCallback(TetheringManager tm, String iface, Network expectedUpstream) {
mTm = tm;
mIface = new TetheringInterface(TETHERING_ETHERNET, iface);
mExpectedUpstream = expectedUpstream;
}
public void unregister() {
@@ -526,19 +537,30 @@ public class EthernetTetheringTest {
Log.d(TAG, "Got upstream changed: " + network);
mUpstream = network;
if (mUpstream != null) mUpstreamConnectedLatch.countDown();
if (mAcceptAnyUpstream || Objects.equals(mUpstream, mExpectedUpstream)) {
mUpstreamLatch.countDown();
}
}
public Network awaitFirstUpstreamConnected() throws Exception {
assertTrue("Did not receive upstream connected callback after " + TIMEOUT_MS + "ms",
mUpstreamConnectedLatch.await(TIMEOUT_MS, TimeUnit.MILLISECONDS));
public Network awaitUpstreamChanged() throws Exception {
if (!mUpstreamLatch.await(TIMEOUT_MS, TimeUnit.MILLISECONDS)) {
fail("Did not receive upstream " + (mAcceptAnyUpstream ? "any" : mExpectedUpstream)
+ " callback after " + TIMEOUT_MS + "ms");
}
return mUpstream;
}
}
private MyTetheringEventCallback enableEthernetTethering(String iface,
TetheringRequest request) throws Exception {
MyTetheringEventCallback callback = new MyTetheringEventCallback(mTm, iface);
TetheringRequest request, Network expectedUpstream) throws Exception {
// Enable ethernet tethering with null expectedUpstream means the test accept any upstream
// after etherent tethering started.
final MyTetheringEventCallback callback;
if (expectedUpstream != null) {
callback = new MyTetheringEventCallback(mTm, iface, expectedUpstream);
} else {
callback = new MyTetheringEventCallback(mTm, iface);
}
mTm.registerTetheringEventCallback(mHandler::post, callback);
StartTetheringCallback startTetheringCallback = new StartTetheringCallback() {
@@ -565,10 +587,11 @@ public class EthernetTetheringTest {
return callback;
}
private MyTetheringEventCallback enableEthernetTethering(String iface) throws Exception {
private MyTetheringEventCallback enableEthernetTethering(String iface, Network expectedUpstream)
throws Exception {
return enableEthernetTethering(iface,
new TetheringRequest.Builder(TETHERING_ETHERNET)
.setShouldShowEntitlementUi(false).build());
.setShouldShowEntitlementUi(false).build(), expectedUpstream);
}
private int getMTU(TestNetworkInterface iface) throws SocketException {
@@ -592,7 +615,8 @@ public class EthernetTetheringTest {
private void checkVirtualEthernet(TestNetworkInterface iface, int mtu) throws Exception {
FileDescriptor fd = iface.getFileDescriptor().getFileDescriptor();
mDownstreamReader = makePacketReader(fd, mtu);
mTetheringEventCallback = enableEthernetTethering(iface.getInterfaceName());
mTetheringEventCallback = enableEthernetTethering(iface.getInterfaceName(),
null /* any upstream */);
checkTetheredClientCallbacks(mDownstreamReader);
}
@@ -690,7 +714,8 @@ public class EthernetTetheringTest {
private void assertInvalidStaticIpv4Request(String iface, String local, String client)
throws Exception {
try {
enableEthernetTethering(iface, requestWithStaticIpv4(local, client));
enableEthernetTethering(iface, requestWithStaticIpv4(local, client),
null /* any upstream */);
fail("Unexpectedly accepted invalid IPv4 configuration: " + local + ", " + client);
} catch (IllegalArgumentException | NullPointerException expected) { }
}
@@ -746,9 +771,10 @@ public class EthernetTetheringTest {
assertEquals("TetheredInterfaceCallback for unexpected interface",
mDownstreamIface.getInterfaceName(), iface);
mTetheringEventCallback = enableEthernetTethering(mDownstreamIface.getInterfaceName());
mTetheringEventCallback = enableEthernetTethering(mDownstreamIface.getInterfaceName(),
mUpstreamTracker.getNetwork());
assertEquals("onUpstreamChanged for unexpected network", mUpstreamTracker.getNetwork(),
mTetheringEventCallback.awaitFirstUpstreamConnected());
mTetheringEventCallback.awaitUpstreamChanged());
mDownstreamReader = makePacketReader(mDownstreamIface);
// TODO: do basic forwarding test here.
@@ -951,9 +977,10 @@ public class EthernetTetheringTest {
assertEquals("TetheredInterfaceCallback for unexpected interface",
mDownstreamIface.getInterfaceName(), iface);
mTetheringEventCallback = enableEthernetTethering(mDownstreamIface.getInterfaceName());
mTetheringEventCallback = enableEthernetTethering(mDownstreamIface.getInterfaceName(),
mUpstreamTracker.getNetwork());
assertEquals("onUpstreamChanged for unexpected network", mUpstreamTracker.getNetwork(),
mTetheringEventCallback.awaitFirstUpstreamConnected());
mTetheringEventCallback.awaitUpstreamChanged());
mDownstreamReader = makePacketReader(mDownstreamIface);
mUpstreamReader = makePacketReader(mUpstreamTracker.getTestIface());