From d7ad0bb8137d631fd4262375ddc390a98bb3cade Mon Sep 17 00:00:00 2001 From: Michael Comella Date: Fri, 9 Apr 2021 09:58:53 -0700 Subject: [PATCH] [fenix] For https://github.com/mozilla-mobile/fenix/issues/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 88ca04a9b6..21fdeab7e3 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 63ac009124..fd98d2b23f 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))