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

Commit 9b105a99 authored by Chiachang Wang's avatar Chiachang Wang
Browse files

Correct tests for verifying data stall metrics 

NetworkMonitor sends data stall metrics data via static method.
The DataStallStatsUtils object from its constructor is never
used. Tests in NetworkMonitorTest that verify the interaction
with the mock object are incorrect.

Bug: 152374582
Test: atest NetworkStackTests NetworkStackNextTests
Merged-In: I308344a80deef6aaf3ed4fb57723f2f210b30483
Change-Id: I308344a80deef6aaf3ed4fb57723f2f210b30483
parent 014f79a8

src/com/android/server/connectivity/NetworkMonitor.java

+16 −6
Original line number Diff line number Diff line
@@ -355,7 +355,6 @@ public class NetworkMonitor extends StateMachine { 
    private final NetworkStackNotifier mNotifier;

    private final IpConnectivityLog mMetricsLog;

    private final Dependencies mDependencies;

    private final DataStallStatsUtils mDetectionStatsUtils;

    private final TcpSocketTracker mTcpTracker;

    // Configuration values for captive portal detection probes.

    private final String mCaptivePortalUserAgent;
@@ -438,15 +437,14 @@ public class NetworkMonitor extends StateMachine { 
    public NetworkMonitor(Context context, INetworkMonitorCallbacks cb, Network network,

            SharedLog validationLog, @NonNull NetworkStackServiceManager serviceManager) {

        this(context, cb, network, new IpConnectivityLog(), validationLog, serviceManager,

                Dependencies.DEFAULT, new DataStallStatsUtils(),

                getTcpSocketTrackerOrNull(context, network));

                Dependencies.DEFAULT, getTcpSocketTrackerOrNull(context, network));

    }



    @VisibleForTesting

    public NetworkMonitor(Context context, INetworkMonitorCallbacks cb, Network network,

            IpConnectivityLog logger, SharedLog validationLogs,

            @NonNull NetworkStackServiceManager serviceManager, Dependencies deps,

            DataStallStatsUtils detectionStatsUtils, @Nullable TcpSocketTracker tst) {

            @Nullable TcpSocketTracker tst) {

        // Add suffix indicating which NetworkMonitor we're talking about.

        super(TAG + "/" + network.toString());
@@ -460,7 +458,6 @@ public class NetworkMonitor extends StateMachine { 
        mCallback = cb;

        mCallbackVersion = getCallbackVersion(cb);

        mDependencies = deps;

        mDetectionStatsUtils = detectionStatsUtils;

        mNetwork = network;

        mCleartextDnsNetwork = deps.getPrivateDnsBypassNetwork(network);

        mTelephonyManager = (TelephonyManager) context.getSystemService(Context.TELEPHONY_SERVICE);
@@ -897,7 +894,8 @@ public class NetworkMonitor extends StateMachine { 
        final int[] transports = mNetworkCapabilities.getTransportTypes();



        for (int i = 0; i < transports.length; i++) {

            DataStallStatsUtils.write(buildDataStallDetectionStats(transports[i]), result);

            final DataStallDetectionStats stats = buildDataStallDetectionStats(transports[i]);

            mDependencies.writeDataStallDetectionStats(stats, result);

        }

        mCollectDataStallMetrics = false;

    }
@@ -2419,6 +2417,18 @@ public class NetworkMonitor extends StateMachine { 
            return NetworkStackUtils.isFeatureEnabled(context, namespace, name, defaultEnabled);

        }



        /**

         * Collect data stall detection level information for each transport type. Write metrics

         * data to statsd pipeline.

         * @param stats a {@link DataStallDetectionStats} that contains the detection level

         *              information.

         * @para result the network reevaluation result.

         */

        public void writeDataStallDetectionStats(@NonNull final DataStallDetectionStats stats,

                @NonNull final CaptivePortalProbeResult result) {

            DataStallStatsUtils.write(stats, result);

        }



        public static final Dependencies DEFAULT = new Dependencies();

    }

tests/unit/src/com/android/server/connectivity/NetworkMonitorTest.java

+31 −4
Original line number Diff line number Diff line
@@ -524,7 +524,7 @@ public class NetworkMonitorTest { 


        WrappedNetworkMonitor() {

            super(mContext, mCallbacks, mNetwork, mLogger, mValidationLogger, mServiceManager,

                    mDependencies, mDataStallStatsUtils, mTst);

                    mDependencies, mTst);

        }



        @Override
@@ -1332,21 +1332,48 @@ public class NetworkMonitorTest { 
    }



    @Test

    public void testDataStall_StallSuspectedAndSendMetrics() throws IOException {

    public void testDataStall_StallDnsSuspectedAndSendMetrics() throws IOException {

        // Connect a VALID network to simulate the data stall detection because data stall

        // evaluation will only start from validated state.

        setStatus(mHttpsConnection, 204);

        WrappedNetworkMonitor wrappedMonitor = makeNotMeteredNetworkMonitor();

        wrappedMonitor.notifyNetworkConnected(TEST_LINK_PROPERTIES, METERED_CAPABILITIES);

        verifyNetworkTested(VALIDATION_RESULT_VALID);

        // Setup dns data stall signal.

        wrappedMonitor.setLastProbeTime(SystemClock.elapsedRealtime() - 1000);

        makeDnsTimeoutEvent(wrappedMonitor, 5);

        assertTrue(wrappedMonitor.isDataStall());

        verify(mDataStallStatsUtils, times(1)).write(makeEmptyDataStallDetectionStats(), any());

        // Trigger a dns signal to start evaluate data stall and upload metrics.

        wrappedMonitor.notifyDnsResponse(RETURN_CODE_DNS_TIMEOUT);

        // Setup information to prevent null data and cause NPE during testing.

        when(mTelephony.getDataNetworkType()).thenReturn(TelephonyManager.NETWORK_TYPE_LTE);

        when(mTelephony.getNetworkOperator()).thenReturn(TEST_MCCMNC);

        when(mTelephony.getSimOperator()).thenReturn(TEST_MCCMNC);

        // Verify data sent as expectation.

        final DataStallDetectionStats stats = wrappedMonitor.buildDataStallDetectionStats(

                NetworkCapabilities.TRANSPORT_CELLULAR);

        final ArgumentCaptor<CaptivePortalProbeResult> probeResultCaptor =

                ArgumentCaptor.forClass(CaptivePortalProbeResult.class);

        verify(mDependencies, timeout(HANDLER_TIMEOUT_MS).times(1))

                .writeDataStallDetectionStats(eq(stats), probeResultCaptor.capture());

        assertTrue(probeResultCaptor.getValue().isSuccessful());

    }



    @Test

    public void testDataStall_NoStallSuspectedAndSendMetrics() throws IOException {

        // Connect a VALID network to simulate the data stall detection because data stall

        // evaluation will only start from validated state.

        setStatus(mHttpsConnection, 204);

        WrappedNetworkMonitor wrappedMonitor = makeNotMeteredNetworkMonitor();

        wrappedMonitor.notifyNetworkConnected(TEST_LINK_PROPERTIES, METERED_CAPABILITIES);

        verifyNetworkTested(VALIDATION_RESULT_VALID);

        // Setup no data stall dns signal.

        wrappedMonitor.setLastProbeTime(SystemClock.elapsedRealtime() - 1000);

        makeDnsTimeoutEvent(wrappedMonitor, 3);

        assertFalse(wrappedMonitor.isDataStall());

        verify(mDataStallStatsUtils, never()).write(makeEmptyDataStallDetectionStats(), any());

        // Trigger a dns signal to start evaluate data stall.

        wrappedMonitor.notifyDnsResponse(RETURN_CODE_DNS_SUCCESS);

        verify(mDependencies, never()).writeDataStallDetectionStats(any(), any());

    }



    @Test