From 46333daeb1b588b1732087f1b37b1ac61fdc17f5 Mon Sep 17 00:00:00 2001 From: atrost Date: Thu, 10 Oct 2019 14:24:21 +0100 Subject: Add a unit test for ChangeReporter. Separate the decision on whether to log to two separate ones - logcat and statsLog to allow separate policies. Also add startLogAll and endLogAll in preparation for an adb command that will enable/disable logging to logcat every time. Test: atest ChangeReporterTest Bug: 138374585 Bug: 140910281 Change-Id: Ie49368b838a19845f51a2670035f611d3e4b9a1b --- .../android/internal/compat/ChangeReporter.java | 79 ++++++++++++-- core/tests/PlatformCompatFramework/Android.bp | 14 +++ .../PlatformCompatFramework/AndroidManifest.xml | 11 ++ core/tests/PlatformCompatFramework/OWNERS | 7 ++ .../internal/compat/ChangeReporterTest.java | 115 +++++++++++++++++++++ 5 files changed, 217 insertions(+), 9 deletions(-) create mode 100644 core/tests/PlatformCompatFramework/Android.bp create mode 100644 core/tests/PlatformCompatFramework/AndroidManifest.xml create mode 100644 core/tests/PlatformCompatFramework/OWNERS create mode 100644 core/tests/PlatformCompatFramework/src/com/android/internal/compat/ChangeReporterTest.java diff --git a/core/java/com/android/internal/compat/ChangeReporter.java b/core/java/com/android/internal/compat/ChangeReporter.java index 72b0ad7a02bc..e0eb9af8b228 100644 --- a/core/java/com/android/internal/compat/ChangeReporter.java +++ b/core/java/com/android/internal/compat/ChangeReporter.java @@ -21,6 +21,7 @@ import android.util.Slog; import android.util.StatsLog; import com.android.internal.annotations.GuardedBy; +import com.android.internal.annotations.VisibleForTesting; import java.util.HashMap; import java.util.HashSet; @@ -37,7 +38,7 @@ public final class ChangeReporter { private static final String TAG = "CompatibilityChangeReporter"; private int mSource; - private final class ChangeReport { + private static final class ChangeReport { long mChangeId; int mState; @@ -65,9 +66,13 @@ public final class ChangeReporter { @GuardedBy("mReportedChanges") private final Map> mReportedChanges; + // When true will of every time to debug (logcat). + private boolean mDebugLogAll; + public ChangeReporter(int source) { mSource = source; mReportedChanges = new HashMap<>(); + mDebugLogAll = false; } /** @@ -79,20 +84,76 @@ public final class ChangeReporter { * @param state of the reported change - enabled/disabled/only logged */ public void reportChange(int uid, long changeId, int state) { - ChangeReport report = new ChangeReport(changeId, state); + if (shouldWriteToStatsLog(uid, changeId, state)) { + StatsLog.write(StatsLog.APP_COMPATIBILITY_CHANGE_REPORTED, uid, changeId, + state, mSource); + } + if (shouldWriteToDebug(uid, changeId, state)) { + debugLog(uid, changeId, state); + } + markAsReported(uid, new ChangeReport(changeId, state)); + } + + /** + * Start logging all the time to logcat. + */ + public void startDebugLogAll() { + mDebugLogAll = true; + } + + /** + * Stop logging all the time to logcat. + */ + public void stopDebugLogAll() { + mDebugLogAll = false; + } + + + /** + * Returns whether the next report should be logged to statsLog. + * + * @param uid affected by the change + * @param changeId the reported change id + * @param state of the reported change - enabled/disabled/only logged + * @return true if the report should be logged + */ + @VisibleForTesting + public boolean shouldWriteToStatsLog(int uid, long changeId, int state) { + return !isAlreadyReported(uid, new ChangeReport(changeId, state)); + } + + /** + * Returns whether the next report should be logged to logcat. + * + * @param uid affected by the change + * @param changeId the reported change id + * @param state of the reported change - enabled/disabled/only logged + * @return true if the report should be logged + */ + @VisibleForTesting + public boolean shouldWriteToDebug(int uid, long changeId, int state) { + return mDebugLogAll || !isAlreadyReported(uid, new ChangeReport(changeId, state)); + } + + private boolean isAlreadyReported(int uid, ChangeReport report) { + synchronized (mReportedChanges) { + Set reportedChangesForUid = mReportedChanges.get(uid); + if (reportedChangesForUid == null) { + return false; + } else { + return reportedChangesForUid.contains(report); + } + } + } + + private void markAsReported(int uid, ChangeReport report) { synchronized (mReportedChanges) { Set reportedChangesForUid = mReportedChanges.get(uid); if (reportedChangesForUid == null) { mReportedChanges.put(uid, new HashSet()); reportedChangesForUid = mReportedChanges.get(uid); } - if (!reportedChangesForUid.contains(report)) { - debugLog(uid, changeId, state); - StatsLog.write(StatsLog.APP_COMPATIBILITY_CHANGE_REPORTED, uid, changeId, - state, mSource); - reportedChangesForUid.add(report); - } - + reportedChangesForUid.add(report); } } diff --git a/core/tests/PlatformCompatFramework/Android.bp b/core/tests/PlatformCompatFramework/Android.bp new file mode 100644 index 000000000000..33802650fa12 --- /dev/null +++ b/core/tests/PlatformCompatFramework/Android.bp @@ -0,0 +1,14 @@ +android_test { + name: "PlatformCompatFrameworkTests", + // Include all test java files. + srcs: ["src/**/*.java"], + libs: [ + "android.test.runner", + "android.test.base", + ], + static_libs: [ + "junit", + "androidx.test.rules", + ], + platform_apis: true, +} diff --git a/core/tests/PlatformCompatFramework/AndroidManifest.xml b/core/tests/PlatformCompatFramework/AndroidManifest.xml new file mode 100644 index 000000000000..4a6383b77000 --- /dev/null +++ b/core/tests/PlatformCompatFramework/AndroidManifest.xml @@ -0,0 +1,11 @@ + + + + + + + + + \ No newline at end of file diff --git a/core/tests/PlatformCompatFramework/OWNERS b/core/tests/PlatformCompatFramework/OWNERS new file mode 100644 index 000000000000..2b7cdb0cbce9 --- /dev/null +++ b/core/tests/PlatformCompatFramework/OWNERS @@ -0,0 +1,7 @@ +# Use this reviewer by default. +platform-compat-eng+reviews@google.com + +andreionea@google.com +atrost@google.com +mathewi@google.com +satayev@google.com diff --git a/core/tests/PlatformCompatFramework/src/com/android/internal/compat/ChangeReporterTest.java b/core/tests/PlatformCompatFramework/src/com/android/internal/compat/ChangeReporterTest.java new file mode 100644 index 000000000000..09bbe37bf292 --- /dev/null +++ b/core/tests/PlatformCompatFramework/src/com/android/internal/compat/ChangeReporterTest.java @@ -0,0 +1,115 @@ +/* + * Copyright (C) 2019 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 com.android.internal.compat; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import org.junit.Test; + +public class ChangeReporterTest { + @Test + public void testStatsLogOnce() { + ChangeReporter reporter = new ChangeReporter(0); + int myUid = 1022, otherUid = 1023; + long myChangeId = 500L, otherChangeId = 600L; + int myState = 1, otherState = 2; + + assertTrue(reporter.shouldWriteToStatsLog(myUid, myChangeId, myState)); + reporter.reportChange(myUid, myChangeId, myState); + + // Same report will not be logged again. + assertFalse(reporter.shouldWriteToStatsLog(myUid, myChangeId, myState)); + // Other reports will be logged. + assertTrue(reporter.shouldWriteToStatsLog(otherUid, myChangeId, myState)); + assertTrue(reporter.shouldWriteToStatsLog(myUid, otherChangeId, myState)); + assertTrue(reporter.shouldWriteToStatsLog(myUid, myChangeId, otherState)); + } + + @Test + public void testStatsLogAfterReset() { + ChangeReporter reporter = new ChangeReporter(0); + int myUid = 1022; + long myChangeId = 500L; + int myState = 1; + + assertTrue(reporter.shouldWriteToStatsLog(myUid, myChangeId, myState)); + reporter.reportChange(myUid, myChangeId, myState); + + // Same report will not be logged again. + assertFalse(reporter.shouldWriteToStatsLog(myUid, myChangeId, myState)); + reporter.resetReportedChanges(myUid); + + // Same report will be logged again after reset. + assertTrue(reporter.shouldWriteToStatsLog(myUid, myChangeId, myState)); + } + + @Test + public void testDebugLogOnce() { + ChangeReporter reporter = new ChangeReporter(0); + int myUid = 1022, otherUid = 1023; + long myChangeId = 500L, otherChangeId = 600L; + int myState = 1, otherState = 2; + + assertTrue(reporter.shouldWriteToDebug(myUid, myChangeId, myState)); + reporter.reportChange(myUid, myChangeId, myState); + + // Same report will not be logged again. + assertFalse(reporter.shouldWriteToDebug(myUid, myChangeId, myState)); + // Other reports will be logged. + assertTrue(reporter.shouldWriteToDebug(otherUid, myChangeId, myState)); + assertTrue(reporter.shouldWriteToDebug(myUid, otherChangeId, myState)); + assertTrue(reporter.shouldWriteToDebug(myUid, myChangeId, otherState)); + } + + @Test + public void testDebugLogAfterReset() { + ChangeReporter reporter = new ChangeReporter(0); + int myUid = 1022; + long myChangeId = 500L; + int myState = 1; + + assertTrue(reporter.shouldWriteToDebug(myUid, myChangeId, myState)); + reporter.reportChange(myUid, myChangeId, myState); + + // Same report will not be logged again. + assertFalse(reporter.shouldWriteToDebug(myUid, myChangeId, myState)); + reporter.resetReportedChanges(myUid); + + // Same report will be logged again after reset. + assertTrue(reporter.shouldWriteToDebug(myUid, myChangeId, myState)); + } + + @Test + public void testDebugLogWithLogAll() { + ChangeReporter reporter = new ChangeReporter(0); + int myUid = 1022; + long myChangeId = 500L; + int myState = 1; + + assertTrue(reporter.shouldWriteToDebug(myUid, myChangeId, myState)); + reporter.reportChange(myUid, myChangeId, myState); + + reporter.startDebugLogAll(); + // Same report will be logged again. + assertTrue(reporter.shouldWriteToDebug(myUid, myChangeId, myState)); + assertTrue(reporter.shouldWriteToDebug(myUid, myChangeId, myState)); + + reporter.stopDebugLogAll(); + assertFalse(reporter.shouldWriteToDebug(myUid, myChangeId, myState)); + } +} -- cgit v1.2.3-59-g8ed1b