From ed1f38611f9c11a6f8e9d25cf220f63bf628173b Mon Sep 17 00:00:00 2001 From: Michael Comella Date: Fri, 9 Apr 2021 09:58:53 -0700 Subject: [PATCH] For #18836: shorten isColdStart... and rm questionable test. The test failed with the rewrite of the code because it violates one of our assumptions that only one Activity will be started. However, since it doesn't rely on observed behavior and we made up the events, it's value is questionable so it seems okay to remove, especially for the gain of conciseness in the code. --- .../fenix/perf/StartupStateProvider.kt | 13 ++----- .../fenix/perf/StartupStateProviderTest.kt | 36 ------------------- 2 files changed, 2 insertions(+), 47 deletions(-) diff --git a/app/src/main/java/org/mozilla/fenix/perf/StartupStateProvider.kt b/app/src/main/java/org/mozilla/fenix/perf/StartupStateProvider.kt index 88ca04a9b..21fdeab7e 100644 --- a/app/src/main/java/org/mozilla/fenix/perf/StartupStateProvider.kt +++ b/app/src/main/java/org/mozilla/fenix/perf/StartupStateProvider.kt @@ -44,20 +44,11 @@ class StartupStateProvider( return false } - val firstActivityStartedIndex = startupLog.log.indexOfFirst { it is LogEntry.ActivityStarted } - if (firstActivityStartedIndex < 0) { - return false // if no activities are started, then we haven't started up yet. - } - - val firstActivityStartedAndAfter = startupLog.log.subList(firstActivityStartedIndex, startupLog.log.size) - val isFirstActivityStartedStillForegrounded = firstActivityStartedAndAfter == listOf( + val isLastStartedActivityStillStarted = startupLog.log.takeLast(2) == listOf( LogEntry.ActivityStarted(activityClass), LogEntry.AppStarted ) - - val hasAppBeenStopped = startupLog.log.contains(LogEntry.AppStopped) - - return isFirstActivityStartedStillForegrounded && !hasAppBeenStopped + return !startupLog.log.contains(LogEntry.AppStopped) && isLastStartedActivityStillStarted } /** diff --git a/app/src/test/java/org/mozilla/fenix/perf/StartupStateProviderTest.kt b/app/src/test/java/org/mozilla/fenix/perf/StartupStateProviderTest.kt index 63ac00912..fd98d2b23 100644 --- a/app/src/test/java/org/mozilla/fenix/perf/StartupStateProviderTest.kt +++ b/app/src/test/java/org/mozilla/fenix/perf/StartupStateProviderTest.kt @@ -89,42 +89,6 @@ class StartupStateProviderTest { assertFalse(provider.isColdStartForStartedActivity(homeActivityClass)) } - @Test - fun `GIVEN the app started for an activity WHEN multiple activities are started but not stopped (maybe impossible) THEN start up is not cold`() { - fun assertIsNotCold() { assertFalse(provider.isColdStartForStartedActivity(homeActivityClass)) } - - // Since we've never observed this, there are multiple ways the events could - // theoretically be ordered: we try a few. - logEntries.addAll(listOf( - LogEntry.ActivityCreated(irActivityClass), - LogEntry.ActivityStarted(irActivityClass), - LogEntry.AppStarted, - LogEntry.ActivityCreated(homeActivityClass), - LogEntry.ActivityStarted(homeActivityClass) - )) - assertIsNotCold() - - logEntries.clear() - logEntries.addAll(listOf( - LogEntry.ActivityCreated(irActivityClass), - LogEntry.ActivityStarted(irActivityClass), - LogEntry.ActivityCreated(homeActivityClass), - LogEntry.ActivityStarted(homeActivityClass), - LogEntry.AppStarted - )) - assertIsNotCold() - - logEntries.clear() - logEntries.addAll(listOf( - LogEntry.ActivityCreated(irActivityClass), - LogEntry.ActivityCreated(homeActivityClass), - LogEntry.ActivityStarted(irActivityClass), - LogEntry.ActivityStarted(homeActivityClass), - LogEntry.AppStarted - )) - assertIsNotCold() - } - @Test fun `GIVEN the app started for an activity WHEN an activity hasn't been created yet THEN start up is not cold`() { assertFalse(provider.isColdStartForStartedActivity(homeActivityClass))