Move base_dir to user_story_set, remove file_path.

Check against .py instead of .pyc

Fix the reason for revert of https://codereview.chromium.org/946473002/

BUG=454531
Committed: https://crrev.com/501f28dee50f6d130127b62ac3a34f5af89cacad
Cr-Commit-Position: refs/heads/master@{#318497}

[aiolos (Not reviewing), 2015-02-23 22:46:29]

aiolos@chromium.org changed reviewers:
+ dtu@chromium.org, nednguyen@google.com

[nednguyen, 2015-02-23 23:46:15]

lgtm

[dtu, 2015-02-23 23:56:19]

lgtm with nits

https://codereview.chromium.org/954563002/diff/1/tools/telemetry/telemetry/pa...
File tools/telemetry/telemetry/page/page_set.py (right):

https://codereview.chromium.org/954563002/diff/1/tools/telemetry/telemetry/pa...
tools/telemetry/telemetry/page/page_set.py:28: # sure that we are passing a
directory, not a file.
Not clear to me this comment means that file_path is going to become base_dir.
also nit: no apostrophes

https://codereview.chromium.org/954563002/diff/1/tools/telemetry/telemetry/pa...
File tools/telemetry/telemetry/page/page_unittest.py (right):

https://codereview.chromium.org/954563002/diff/1/tools/telemetry/telemetry/pa...
tools/telemetry/telemetry/page/page_unittest.py:57: file_path =
os.path.dirname(base_dir) + '/otherdir/file.html'
nit: use os.path.join

https://codereview.chromium.org/954563002/diff/1/tools/telemetry/telemetry/us...
File tools/telemetry/telemetry/user_story/user_story_set_unittest.py (right):

https://codereview.chromium.org/954563002/diff/1/tools/telemetry/telemetry/us...
tools/telemetry/telemetry/user_story/user_story_set_unittest.py:5: import
inspect
nit: Extra import

https://codereview.chromium.org/954563002/diff/1/tools/telemetry/telemetry/us...
tools/telemetry/telemetry/user_story/user_story_set_unittest.py:19: def
__init__(self):
Don't need constructor, just add a comment saying what this class is for.

[aiolos (Not reviewing), 2015-02-26 23:22:48]

https://codereview.chromium.org/954563002/diff/1/tools/telemetry/telemetry/pa...
File tools/telemetry/telemetry/page/page_set.py (right):

https://codereview.chromium.org/954563002/diff/1/tools/telemetry/telemetry/pa...
tools/telemetry/telemetry/page/page_set.py:28: # sure that we are passing a
directory, not a file.
On 2015/02/23 23:56:19, dtu wrote:
> Not clear to me this comment means that file_path is going to become base_dir.
> also nit: no apostrophes

Done.

https://codereview.chromium.org/954563002/diff/1/tools/telemetry/telemetry/pa...
File tools/telemetry/telemetry/page/page_unittest.py (right):

https://codereview.chromium.org/954563002/diff/1/tools/telemetry/telemetry/pa...
tools/telemetry/telemetry/page/page_unittest.py:57: file_path =
os.path.dirname(base_dir) + '/otherdir/file.html'
On 2015/02/23 23:56:19, dtu wrote:
> nit: use os.path.join

Done.

https://codereview.chromium.org/954563002/diff/1/tools/telemetry/telemetry/us...
File tools/telemetry/telemetry/user_story/user_story_set_unittest.py (right):

https://codereview.chromium.org/954563002/diff/1/tools/telemetry/telemetry/us...
tools/telemetry/telemetry/user_story/user_story_set_unittest.py:5: import
inspect
On 2015/02/23 23:56:19, dtu wrote:
> nit: Extra import

Done.

https://codereview.chromium.org/954563002/diff/1/tools/telemetry/telemetry/us...
tools/telemetry/telemetry/user_story/user_story_set_unittest.py:19: def
__init__(self):
On 2015/02/23 23:56:19, dtu wrote:
> Don't need constructor, just add a comment saying what this class is for.

And I just noticed that there's a testing UserStorySet class right above this
already... :P

[dtu, 2015-02-26 23:25:17]

Still lgtm

[aiolos (Not reviewing), 2015-02-26 23:26:20]

The CQ bit was checked by aiolos@chromium.org

[aiolos (Not reviewing), 2015-02-26 23:26:21]

The patchset sent to the CQ was uploaded after l-g-t-m from nednguyen@google.com
Link to the patchset: https://codereview.chromium.org/954563002/#ps20001 (title:
"nits")

[commit-bot: I haz the power, 2015-02-26 23:26:54]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/954563002/20001

[commit-bot: I haz the power, 2015-02-27 01:21:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-02-27 01:21:36]

Failed to apply patch for tools/telemetry/telemetry/page/page_set.py:
While running git apply --index -3 -p1;
  error: patch failed: tools/telemetry/telemetry/page/page_set.py:50
  Falling back to three-way merge...
  Applied patch to 'tools/telemetry/telemetry/page/page_set.py' with conflicts.
  U tools/telemetry/telemetry/page/page_set.py

Patch:       tools/telemetry/telemetry/page/page_set.py
Index: tools/telemetry/telemetry/page/page_set.py
diff --git a/tools/telemetry/telemetry/page/page_set.py
b/tools/telemetry/telemetry/page/page_set.py
index
a4ddc1e4206950125cceea161d2f59636235fcd0..0216241ae9a88c935d658aa5cec823e892e5fc61
100644
--- a/tools/telemetry/telemetry/page/page_set.py
+++ b/tools/telemetry/telemetry/page/page_set.py
@@ -24,19 +24,16 @@ class PageSet(user_story_set.UserStorySet):
                serving_dirs=None, bucket=None):
     # The default value of file_path is location of the file that define this
     # page set instance's class.
-    # TODO(chrishenry): Move this logic to user_story_set. Consider passing
-    # a base_dir directly. Alternatively, kill this and rely on the default
-    # behavior of using the instance's class file location.
-    if file_path is None:
-      file_path = inspect.getfile(self.__class__)
-      # Turn pyc file into py files if we can
-      if file_path.endswith('.pyc') and os.path.exists(file_path[:-1]):
-        file_path = file_path[:-1]
-    self.file_path = file_path
+    # TODO(aiolos): When migrating page_sets over to user_story_sets, make
+    # sure that we are passing a valid directory path in to base_dir, and not
+    # a file path like we curerntly do in some cases for file_path.
+    dir_name = file_path
+    if file_path and os.path.isfile(file_path):
+      dir_name = os.path.dirname(file_path)
 
     super(PageSet, self).__init__(
         archive_data_file=archive_data_file, cloud_storage_bucket=bucket,
-        serving_dirs=serving_dirs)
+        base_dir=dir_name, serving_dirs=serving_dirs)
 
     # These attributes can be set dynamically by the page set.
     self.user_agent_type = user_agent_type
@@ -50,13 +47,6 @@ class PageSet(user_story_set.UserStorySet):
     assert user_story.page_set is self
     super(PageSet, self).AddUserStory(user_story)
 
-  @property
-  def base_dir(self):
-    if os.path.isfile(self.file_path):
-      return os.path.dirname(self.file_path)
-    else:
-      return self.file_path
-
   def ReorderPageSet(self, results_file):
     """Reorders this page set based on the results of a past run."""
     page_set_dict = {}

[commit-bot: I haz the power, 2015-02-27 01:22:26]

The CQ bit was unchecked by commit-bot@chromium.org

[aiolos (Not reviewing), 2015-02-27 17:46:31]

Patchset #3 (id:40001) has been deleted

[aiolos (Not reviewing), 2015-02-27 18:12:48]

The CQ bit was checked by aiolos@chromium.org

[aiolos (Not reviewing), 2015-02-27 18:12:48]

The patchset sent to the CQ was uploaded after l-g-t-m from
nednguyen@google.com, dtu@chromium.org
Link to the patchset: https://codereview.chromium.org/954563002/#ps60001 (title:
"Rebase")

[aiolos (Not reviewing), 2015-02-27 18:13:00]

The CQ bit was unchecked by aiolos@chromium.org

[aiolos (Not reviewing), 2015-02-27 18:20:22]

https://codereview.chromium.org/954563002/diff/20001/tools/telemetry/telemetr...
File tools/telemetry/telemetry/page/page_set.py (right):

https://codereview.chromium.org/954563002/diff/20001/tools/telemetry/telemetr...
tools/telemetry/telemetry/page/page_set.py:49: 
This was done in https://codereview.chromium.org/965543002/ That patch landed
over 17 hours ago, and hasn't been reverted. Anyone know why this is popping up
as a change in this CL? It doesn't show in my diff locally against TOT.

[aiolos (Not reviewing), 2015-02-27 18:21:10]

The CQ bit was checked by aiolos@chromium.org

[commit-bot: I haz the power, 2015-02-27 18:22:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/954563002/60001

[commit-bot: I haz the power, 2015-02-27 19:54:16]

Committed patchset #3 (id:60001)

[commit-bot: I haz the power, 2015-02-27 19:54:53]

Patchset 3 (id:??) landed as
https://crrev.com/501f28dee50f6d130127b62ac3a34f5af89cacad
Cr-Commit-Position: refs/heads/master@{#318497}