From a71fdb9ef758e2407cdb93de5c2279ec0d79566c Mon Sep 17 00:00:00 2001 From: Adam Bookatz Date: Fri, 8 May 2020 16:31:01 -0700 Subject: [PATCH] UserLifecycleTests handles crashes Previously, if a test failed or crashed, no results would be returned. And if the test 'succeeded', but with the wrong data (e.g. the secondary user actually failed to be created), the results would simply display bad data. Here, we make it so that *any* test failure will officially report a value of -10ms. -10ms was chosen since it is clearly 'wrong' yet won't skew the dashboard (since the value is not far from 0). The actual runs will be allowed to proceed, so that more detailed information will be available in the logs. But the final reported Benchmark data will be a simple -10ms. At this point, we take the approach that if one of the four iterations fail, the entire test is declared to have failed. This is to highlight failures, since we don't know their frequency yet. We may change this in the future. Test: atest UserLifecycleTests (and trigger some failures) Bug: 137127051 Change-Id: Iab4e4d9c5194976af8e53bc8808a4a16918e0865 --- .../android/multiuser/BenchmarkResults.java | 25 ++++++++ .../multiuser/BenchmarkResultsReporter.java | 64 ++++++++++++++++--- .../android/multiuser/BenchmarkRunner.java | 33 +++++++++- .../android/multiuser/UserLifecycleTests.java | 8 ++- 4 files changed, 115 insertions(+), 15 deletions(-) diff --git a/apct-tests/perftests/multiuser/src/android/multiuser/BenchmarkResults.java b/apct-tests/perftests/multiuser/src/android/multiuser/BenchmarkResults.java index c1362dc3a6a71..cd3c11c6bcbfc 100644 --- a/apct-tests/perftests/multiuser/src/android/multiuser/BenchmarkResults.java +++ b/apct-tests/perftests/multiuser/src/android/multiuser/BenchmarkResults.java @@ -22,6 +22,9 @@ import java.util.Collections; import java.util.concurrent.TimeUnit; public class BenchmarkResults { + /** If the test fails, output this value as a signal of the failure. */ + public static final long DECLARED_VALUE_IF_ERROR_MS = -10; + private final ArrayList mResults = new ArrayList<>(); public void addDuration(long duration) { @@ -42,6 +45,28 @@ public class BenchmarkResults { return stats; } + /** + * Same as {@link #getStatsToReport()} but for failure, + * using {@link #DECLARED_VALUE_IF_ERROR_MS}. + */ + public static Bundle getFailedStatsToReport() { + final Bundle stats = new Bundle(); + stats.putDouble("Mean (ms)", DECLARED_VALUE_IF_ERROR_MS); + return stats; + } + + /** + * Same as {@link #getStatsToLog()} but for failure, + * using {@link #DECLARED_VALUE_IF_ERROR_MS}. + */ + public static Bundle getFailedStatsToLog() { + final Bundle stats = new Bundle(); + stats.putDouble("Mean (ms)", DECLARED_VALUE_IF_ERROR_MS); + stats.putDouble("Median (ms)", DECLARED_VALUE_IF_ERROR_MS); + stats.putDouble("Sigma (ms)", DECLARED_VALUE_IF_ERROR_MS); + return stats; + } + public ArrayList getAllDurations() { return mResults; } diff --git a/apct-tests/perftests/multiuser/src/android/multiuser/BenchmarkResultsReporter.java b/apct-tests/perftests/multiuser/src/android/multiuser/BenchmarkResultsReporter.java index ba33e6439fbd8..b6f13fd65d569 100644 --- a/apct-tests/perftests/multiuser/src/android/multiuser/BenchmarkResultsReporter.java +++ b/apct-tests/perftests/multiuser/src/android/multiuser/BenchmarkResultsReporter.java @@ -15,6 +15,8 @@ */ package android.multiuser; +import static android.multiuser.BenchmarkResults.DECLARED_VALUE_IF_ERROR_MS; + import android.app.Activity; import android.app.Instrumentation; import android.os.Bundle; @@ -40,28 +42,70 @@ public class BenchmarkResultsReporter implements TestRule { return new Statement() { @Override public void evaluate() throws Throwable { - base.evaluate(); - final Bundle stats = mRunner.getStatsToReport(); - final String summary = getSummaryString(description.getMethodName(), - mRunner.getStatsToLog()); - logSummary(description.getTestClass().getSimpleName(), summary, - mRunner.getAllDurations()); - stats.putString(Instrumentation.REPORT_KEY_STREAMRESULT, summary); - InstrumentationRegistry.getInstrumentation().sendStatus( - Activity.RESULT_OK, stats); + final String tag = description.getTestClass().getSimpleName(); + final String methodName = description.getMethodName(); + Throwable error = null; + + try { + base.evaluate(); + error = mRunner.getErrorOrNull(); + } catch (Exception e) { + error = e; + } + + if (error != null) { + Log.e(tag, "Test " + methodName + " failed.", error); + Log.d(tag, "Logcat displays the results ignoring the fact that it failed;\n" + + "however, fake results of " + DECLARED_VALUE_IF_ERROR_MS + "ms " + + "will be reported to the instrumentation caller to signify failure."); + } + + final String summary = getSummaryString(methodName, mRunner.getStatsToLog()); + logSummary(tag, summary, mRunner.getAllDurations()); + + Bundle stats; + if (error == null) { + stats = mRunner.getStatsToReport(); + stats.putString(Instrumentation.REPORT_KEY_STREAMRESULT, summary); + } else { + stats = BenchmarkResults.getFailedStatsToReport(); + final String failSummary = getSummaryString(methodName, + BenchmarkResults.getFailedStatsToLog()); + stats.putString(Instrumentation.REPORT_KEY_STREAMRESULT, failSummary); + } + InstrumentationRegistry.getInstrumentation().sendStatus(Activity.RESULT_OK, stats); + + if (error != null) { + throw error; + } } }; } + /** + * Prints, for example: + * UserLifecycleTests: (summary string) + * UserLifecycleTests: 1->101 + * UserLifecycleTests: 2->102 + * UserLifecycleTests: 3->103 + * UserLifecycleTests: 4->102 + */ private void logSummary(String tag, String summary, ArrayList durations) { final StringBuilder sb = new StringBuilder(summary); final int size = durations.size(); for (int i = 0; i < size; ++i) { - sb.append("\n").append(i).append("->").append(durations.get(i)); + sb.append("\n").append(i+1).append("->").append(durations.get(i)); } Log.d(tag, sb.toString()); } + /** + * For example: + * testName + * Sigma (ms): 1 + * Mean (ms): 2 + * Median (ms): 3 + */ private String getSummaryString(String testName, Bundle stats) { final StringBuilder sb = new StringBuilder(); sb.append("\n\n").append(getKey(testName)); diff --git a/apct-tests/perftests/multiuser/src/android/multiuser/BenchmarkRunner.java b/apct-tests/perftests/multiuser/src/android/multiuser/BenchmarkRunner.java index 7b65bfa02a185..8305d3f648a3f 100644 --- a/apct-tests/perftests/multiuser/src/android/multiuser/BenchmarkRunner.java +++ b/apct-tests/perftests/multiuser/src/android/multiuser/BenchmarkRunner.java @@ -15,6 +15,7 @@ */ package android.multiuser; +import android.annotation.Nullable; import android.os.Bundle; import android.os.SystemClock; import android.perftests.utils.ShellHelper; @@ -35,12 +36,14 @@ public class BenchmarkRunner { private final BenchmarkResults mResults = new BenchmarkResults(); private int mState = NOT_STARTED; // Current benchmark state. - private int mIteration; + private int mIteration = 1; public long mStartTimeNs; public long mPausedDurationNs; public long mPausedTimeNs; + private Throwable mFirstFailure = null; + public boolean keepRunning() { switch (mState) { case NOT_STARTED: @@ -61,7 +64,7 @@ public class BenchmarkRunner { private boolean startNextTestRun() { mResults.addDuration(System.nanoTime() - mStartTimeNs - mPausedDurationNs); - if (mIteration == NUM_ITERATIONS) { + if (mIteration == NUM_ITERATIONS + 1) { mState = FINISHED; return false; } else { @@ -104,4 +107,30 @@ public class BenchmarkRunner { public ArrayList getAllDurations() { return mResults.getAllDurations(); } + + /** Returns which iteration (starting at 1) the Runner is currently on. */ + public int getIteration() { + return mIteration; + } + + /** + * Marks the test run as failed, along with a message of why. + * Only the first fail message is retained. + */ + public void markAsFailed(Throwable err) { + if (mFirstFailure == null) { + mFirstFailure = err; + } + } + + /** Gets the failure message if the test failed; otherwise {@code null}. */ + public @Nullable Throwable getErrorOrNull() { + if (mFirstFailure != null) { + return mFirstFailure; + } + if (mState != FINISHED) { + return new AssertionError("BenchmarkRunner state is not FINISHED."); + } + return null; + } } \ No newline at end of file diff --git a/apct-tests/perftests/multiuser/src/android/multiuser/UserLifecycleTests.java b/apct-tests/perftests/multiuser/src/android/multiuser/UserLifecycleTests.java index e042782af3661..405810b767961 100644 --- a/apct-tests/perftests/multiuser/src/android/multiuser/UserLifecycleTests.java +++ b/apct-tests/perftests/multiuser/src/android/multiuser/UserLifecycleTests.java @@ -17,6 +17,7 @@ package android.multiuser; import static org.junit.Assume.assumeTrue; +import android.annotation.NonNull; import android.app.ActivityManager; import android.app.ActivityTaskManager; import android.app.AppGlobals; @@ -767,13 +768,14 @@ public class UserLifecycleTests { } } - private void attestTrue(String message, boolean assertion) { + private void attestTrue(@NonNull String message, boolean assertion) { if (!assertion) { - Log.w(TAG, message); + Log.e(TAG, "Test failed on iteration #" + mRunner.getIteration() + ": " + message); + mRunner.markAsFailed(new AssertionError(message)); } } - private void attestFalse(String message, boolean assertion) { + private void attestFalse(@NonNull String message, boolean assertion) { attestTrue(message, !assertion); } }