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

Commit 11074780 authored by Neil Fuller's avatar Neil Fuller
Browse files

Fix metrics test flakiness 

This commit makes two changes:
1) The shell commands for accessing config went directly to the
   ServiceConfigAccessor, while metrics used a cached copy of config.
   This means they may disagree under some circumstances like automated
   tests.  This change tunnels shell command queries to the strategy
   object, which generates the metrics, ensuring a higher chance of
   consistency.

2) Some types of configuration change (used during tests) were not
   causing the copy of ConfigurationInternal held by
   TimeZoneDetectorStrategyImpl to be updated. This corrects that.

This should fix automated tests that were receiving constradictory
information about whether geo detection was supported on a device from
shell command Vs metrics reporting. It is likely this is the result of a
previous test changing the supported status (probably by changing the
config of location time zone providers) and the issues fixed by points 1
and 2 (mostly 2) above causing the contradiction.

Bug: 228457046
Test: Treehugger
Change-Id: I95e79575ec102662bcbfbbab7c99c07437e3a633
parent ead48cc6

services/core/java/com/android/server/timezonedetector/ServiceConfigAccessorImpl.java

+12 −0
Original line number Diff line number Diff line
@@ -59,6 +59,8 @@ public final class ServiceConfigAccessorImpl implements ServiceConfigAccessor { 
     */

    private static final Set<String> CONFIGURATION_INTERNAL_SERVER_FLAGS_KEYS_TO_WATCH = Set.of(

            ServerFlags.KEY_LOCATION_TIME_ZONE_DETECTION_FEATURE_SUPPORTED,

            ServerFlags.KEY_PRIMARY_LTZP_MODE_OVERRIDE,

            ServerFlags.KEY_SECONDARY_LTZP_MODE_OVERRIDE,

            ServerFlags.KEY_LOCATION_TIME_ZONE_DETECTION_RUN_IN_BACKGROUND_ENABLED,

            ServerFlags.KEY_ENHANCED_METRICS_COLLECTION_ENABLED,

            ServerFlags.KEY_LOCATION_TIME_ZONE_DETECTION_SETTING_ENABLED_DEFAULT,
@@ -443,6 +445,9 @@ public final class ServiceConfigAccessorImpl implements ServiceConfigAccessor { 
        mTestPrimaryLocationTimeZoneProviderMode =

                mTestPrimaryLocationTimeZoneProviderPackageName == null

                        ? PROVIDER_MODE_DISABLED : PROVIDER_MODE_ENABLED;

        // Changing this state can affect the content of ConfigurationInternal, so listeners need to

        // be informed.

        mContext.getMainThreadHandler().post(this::handleConfigurationInternalChangeOnMainThread);

    }



    @Override
@@ -469,6 +474,9 @@ public final class ServiceConfigAccessorImpl implements ServiceConfigAccessor { 
        mTestSecondaryLocationTimeZoneProviderMode =

                mTestSecondaryLocationTimeZoneProviderPackageName == null

                        ? PROVIDER_MODE_DISABLED : PROVIDER_MODE_ENABLED;

        // Changing this state can affect the content of ConfigurationInternal, so listeners need to

        // be informed.

        mContext.getMainThreadHandler().post(this::handleConfigurationInternalChangeOnMainThread);

    }



    @Override
@@ -573,6 +581,10 @@ public final class ServiceConfigAccessorImpl implements ServiceConfigAccessor { 
        mTestSecondaryLocationTimeZoneProviderPackageName = null;

        mTestSecondaryLocationTimeZoneProviderMode = null;

        mRecordStateChangesForTests = false;



        // Changing LTZP config can affect the content of ConfigurationInternal, so listeners

        // need to be informed.

        mContext.getMainThreadHandler().post(this::handleConfigurationInternalChangeOnMainThread);

    }



    private boolean isTelephonyFallbackSupported() {

services/core/java/com/android/server/timezonedetector/TimeZoneDetectorService.java

+2 −2
Original line number Diff line number Diff line
@@ -336,13 +336,13 @@ public final class TimeZoneDetectorService extends ITimeZoneDetectorService.Stub 
    boolean isTelephonyTimeZoneDetectionSupported() {

        enforceManageTimeZoneDetectorPermission();



        return mServiceConfigAccessor.isTelephonyTimeZoneDetectionFeatureSupported();

        return mTimeZoneDetectorStrategy.isTelephonyTimeZoneDetectionSupported();

    }



    boolean isGeoTimeZoneDetectionSupported() {

        enforceManageTimeZoneDetectorPermission();



        return mServiceConfigAccessor.isGeoTimeZoneDetectionFeatureSupported();

        return mTimeZoneDetectorStrategy.isGeoTimeZoneDetectionSupported();

    }



    /**

services/core/java/com/android/server/timezonedetector/TimeZoneDetectorStrategy.java

+6 −0
Original line number Diff line number Diff line
@@ -124,4 +124,10 @@ public interface TimeZoneDetectorStrategy extends Dumpable { 
    /** Generates a state snapshot for metrics. */

    @NonNull

    MetricsTimeZoneDetectorState generateMetricsState();



    /** Returns {@code true} if the device supports telephony time zone detection. */

    boolean isTelephonyTimeZoneDetectionSupported();



    /** Returns {@code true} if the device supports geolocation time zone detection. */

    boolean isGeoTimeZoneDetectionSupported();

}

services/core/java/com/android/server/timezonedetector/TimeZoneDetectorStrategyImpl.java

+14 −0
Original line number Diff line number Diff line
@@ -396,6 +396,20 @@ public final class TimeZoneDetectorStrategyImpl implements TimeZoneDetectorStrat 
                getLatestGeolocationSuggestion());

    }



    @Override

    public boolean isTelephonyTimeZoneDetectionSupported() {

        synchronized (this) {

            return mCurrentConfigurationInternal.isTelephonyDetectionSupported();

        }

    }



    @Override

    public boolean isGeoTimeZoneDetectionSupported() {

        synchronized (this) {

            return mCurrentConfigurationInternal.isGeoDetectionSupported();

        }

    }



    private static int scoreTelephonySuggestion(@NonNull TelephonyTimeZoneSuggestion suggestion) {

        int score;

        if (suggestion.getZoneId() == null) {

services/tests/servicestests/src/com/android/server/timezonedetector/FakeTimeZoneDetectorStrategy.java

+10 −0
Original line number Diff line number Diff line
@@ -60,6 +60,16 @@ class FakeTimeZoneDetectorStrategy implements TimeZoneDetectorStrategy { 
        throw new UnsupportedOperationException();

    }



    @Override

    public boolean isTelephonyTimeZoneDetectionSupported() {

        throw new UnsupportedOperationException();

    }



    @Override

    public boolean isGeoTimeZoneDetectionSupported() {

        throw new UnsupportedOperationException();

    }



    @Override

    public void dump(IndentingPrintWriter pw, String[] args) {

        mDumpCalled = true;