[System Health] Create first System Health media user stories.

tools/perf/page_sets/system_health/media_stories.py

+# Copyright 2016 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+from page_sets.system_health import platforms
+from page_sets.system_health import system_health_story
+
+from page_sets.login_helpers import google_login
+from page_sets.login_helpers import pandora_login
+
+
+class _MediaStory(system_health_story.SystemHealthStory):
+  """Abstract base class for media System Health user stories."""
+
+  ABSTRACT_STORY = True
+  SEARCH_IDENTIFIER = NotImplemented
+  PLAY_IDENTIFIER = NotImplemented

[charliea (OOO until 10-5), 2016/09/14 23:55:03]

I think "element" might be more accurate than "identifier"

[rnephew (Reviews Here), 2016/09/20 21:09:54]

Done.

+  STOP_IDENTIFIER = NotImplemented
+  SEARCH_QUERY = NotImplemented

[petrcermak, 2016/09/14 16:42:57]

I don't think there's any value in having this abstract class constant. Since
you just pass it to an abstract method, you might as well just remove the
parameter from the abstract method. This just adds code complexity without
reducing code duplication. The same applies to SEARCH_IDENTIFIER. You could just
define a _SearchMedia(action_runner) abstract method and let the subclasses do
the job.

[charliea (OOO until 10-5), 2016/09/14 23:55:03]

+1

[rnephew (Reviews Here), 2016/09/20 21:09:55]

Done.

+  PLAY_DURATION = 10

[nednguyen, 2016/09/14 16:57:41]

Can we change this to 20s?

[rnephew (Reviews Here), 2016/09/20 21:09:54]

Done.

+
+
+  def RunPageInteractions(self, action_runner):
+    self._SearchMedia(action_runner, self.SEARCH_IDENTIFIER,
+                      self.SEARCH_QUERY)
+    # Start media.
+    self._WaitForAndClickElement(action_runner, self.PLAY_IDENTIFIER)
+    action_runner.Wait(self.PLAY_DURATION)
+    # Stop media.
+    self._WaitForAndClickElement(action_runner, self.STOP_IDENTIFIER)
+
+  def _SearchMedia(self, action_runner, element_function, query):

[petrcermak, 2016/09/14 16:42:57]

What does "element_function" refer to? The input box? The button to be pressed
after typing the query? Please make the variable name more descriptive (e.g.
input_element_function or button_element_function).

[charliea (OOO until 10-5), 2016/09/14 23:55:03]

I think _NavigateToMedia might be a more accurate name, given how it's used.

[rnephew (Reviews Here), 2016/09/20 21:09:55]

Done.

[rnephew (Reviews Here), 2016/09/20 21:09:55]

Done.

+    raise NotImplementedError
+
+  def _WaitForAndClickElement(self, action_runner, element_function):
+    action_runner.WaitForElement(element_function=element_function)
+    action_runner.ClickElement(element_function=element_function)
+
+
+class _MediaDesktopStory(_MediaStory):
+  ABSTRACT_STORY = True
+  SUPPORTED_PLATFORMS = platforms.DESKTOP_ONLY
+
+  def _SearchMedia(self, action_runner, element_function, query):

[petrcermak, 2016/09/14 16:42:57]

no need to re-define the method

[rnephew (Reviews Here), 2016/09/20 21:09:54]

Pylint disagrees. It complains that an abstract method isn't being defined here.

+    raise NotImplementedError
+
+
+################################################################################
+# Audio stories

[petrcermak, 2016/09/14 16:42:57]

nit: missing period (for consistency with other *_stories.py files)

[rnephew (Reviews Here), 2016/09/20 21:09:54]

Done.

+################################################################################
+
+
+class GooglePlayMusicDesktopStory(_MediaDesktopStory):
+  NAME = 'play:media:google_play_music'
+  URL = 'https://music.google.com'
+
+  SEARCH_IDENTIFIER = 'document.querySelector(".title.fade-out.tooltip")'
+  PLAY_IDENTIFIER = (
+      'document.getElementsByClassName("x-scope paper-fab-0")[0]')
+  STOP_IDENTIFIER = (
+      'document.getElementsByClassName("style-scope sj-play-button")[0]')
+  NAVIGATE_IDENTIFIER = (
+      'document.getElementsByClassName("description tooltip fade-out")[0]')
+
+  def _Login(self, action_runner):
+    google_login.LoginGoogleAccount(action_runner, 'googletest',
+                                    self.credentials_path)
+
+  def _SearchMedia(self, action_runner, element_function, _):
+    # Clicks on Today's top hits. (SEARCH_IDENTIFIER)

[petrcermak, 2016/09/14 16:42:57]

This is a proof that passing SEARCH_IDENTIFIER through an argument doesn't make
sense (because you already know that it will be passed).

[rnephew (Reviews Here), 2016/09/20 21:09:54]

Done.

+    self._WaitForAndClickElement(action_runner, element_function)
+    # Clicks on playlist. (NAVIGATE_IDENTIFIER)

[petrcermak, 2016/09/14 16:42:57]

nit: no need for "(NAVIGATE_IDENTIFIER)" here. The next line tells the story ;-)

(the same applies to line 71)

[rnephew (Reviews Here), 2016/09/20 21:09:54]

Done.

+    self._WaitForAndClickElement(action_runner, self.NAVIGATE_IDENTIFIER)

[nednguyen, 2016/09/14 17:02:03]

The timing of google play is in "<span id="time-container-current"
aria-label="Current track time: 19 seconds">0:19</span>"

[rnephew (Reviews Here), 2016/09/20 21:09:55]

Done.

+
+
+class SoundCloudDesktopStory(_MediaDesktopStory):
+  NAME = 'play:media:soundcloud'
+  URL = 'https://soundcloud.com'
+
+  SEARCH_IDENTIFIER = 'document.getElementsByClassName("headerSearch")[0]'
+  SEARCH_QUERY = 'SSmooth Jazz' # First S for some reason gets ommited.
+  PLAY_IDENTIFIER = ('document.getElementsByClassName("sc-button-play'
+                     ' playButton sc-button sc-button-xlarge")[0]')
+  STOP_IDENTIFIER = ('document.getElementsByClassName("playControl '
+                     'playControls__icon sc-ir playing")[0]')
+
+  def _SearchMedia(self, action_runner, element_function, query):
+    self._WaitForAndClickElement(action_runner, element_function)
+    action_runner.Wait(1) # Add 1 second wait to make the browsing realistic.
+    action_runner.EnterText(query)
+    action_runner.PressKey('Return')
+
+
+class PandoraDesktopStory(_MediaDesktopStory):

[petrcermak, 2016/09/14 16:42:57]

Note that at this point, this class could just subclass directly from
system_health_story.SystemHealthStory because it doesn't use any of the
_MediaStory code. To address this, I suggest that you update
_MediaStory.RunPageInteractions to do the following (same for STOP_IDENTIFIER):

  ...
  if self.PLAY_IDENFIER:
    self._WaitForAndClickElement(action_runner, self.PLAY_IDENTIFIER)
  ...

If you do this, there will be no need to override RunPageInteractions at all
(which will render my next comment irrelevant).

[rnephew (Reviews Here), 2016/09/20 21:09:55]

Done.

+  NAME='play:media:pandora'
+  URL = 'https://pandora.com'
+
+  SEARCH_IDENTIFIER = 'document.getElementsByClassName("searchInput")[0]'
+  SEARCH_QUERY = None
+  PLAY_IDENTIFIER = None
+  STOP_IDENTIFIER = 'document.getElementsByClassName("pauseButton")[0]'
+
+  def _Login(self, action_runner):
+    pandora_login.LoginAccount(action_runner, 'pandora', self.credentials_path)
+
+  def _SearchMedia(self, action_runner, element_function, query):
+    pass # Audio autoplays on Pandora, no need to search.
+
+  def RunPageInteractions(self, action_runner):
+    self._SearchMedia(action_runner, self.SEARCH_IDENTIFIER,

[petrcermak, 2016/09/14 16:42:57]

Why do you do this? You're just calling an empty function.

[rnephew (Reviews Here), 2016/09/20 21:09:55]

Acknowledged.

+                      self.SEARCH_QUERY)
+    action_runner.Wait(self.PLAY_DURATION)

[nednguyen, 2016/09/14 17:02:03]

The elapse time of the player is reflected through "<div
class="elapsedTime">0:07</div> ". Can you modify the wait condition to check for
this to be at least PLAY_DURATION?

[rnephew (Reviews Here), 2016/09/20 21:09:54]

Done, but one concern here is that, at least on GPM, the timer is janky. It
regularly will hang and update 5+ seconds at a time. Example: 1,2,7,9,10,15,21

+    self._WaitForAndClickElement(action_runner, self.STOP_IDENTIFIER)