Commit 14042b49 authored by jam's avatar jam Committed by Commit bot

Revert of Move base_dir to user_story_set, remove file_path. (patchset #4...

Revert of Move base_dir to user_story_set, remove file_path. (patchset #4 id:60001 of https://codereview.chromium.org/946473002/)

Reason for revert:
breaks on main waterfall, i.e.
http://build.chromium.org/p/chromium.win/builders/Win%207%20Tests%20x64%20%281%29/builds/21577/steps/telemetry_unittests/logs/stdio

  AssertionError: 'E:\\b\\build\\slave\\Win_7_Tests_x64__1_\\build\\src\\tools\\telemetry\\telemetry\\user_story\\user_story_set_unittest.pyc' != 'E:\\b\\build\\slave\\Win_7_Tests_x64__1_\\build\\src\\tools\\telemetry\\telemetry\\user_story\\user_story_set_unittest.py'

not sure why this fails on main waterfall but not trybots?

Original issue's description:
> Move base_dir to user_story_set, remove file_path.
>
> BUG=454531
>
> Committed: https://crrev.com/65ff7d22f8986ab2832045af09160dc753e40c5f
> Cr-Commit-Position: refs/heads/master@{#317632}

TBR=chrishenry@google.com,dtu@chromium.org,sullivan@chromium.org,nednguyen@google.com,aiolos@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=454531

Review URL: https://codereview.chromium.org/951593003

Cr-Commit-Position: refs/heads/master@{#317665}
parent d32a8c3a
......@@ -24,15 +24,19 @@ 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(aiolos): When migrating page_set's over to user_story_set's, make
# sure that we are passing a directory, not a file.
dir_name = file_path
if file_path and os.path.isfile(file_path):
dir_name = os.path.dirname(file_path)
# 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
super(PageSet, self).__init__(
archive_data_file=archive_data_file, cloud_storage_bucket=bucket,
base_dir=dir_name, serving_dirs=serving_dirs)
serving_dirs=serving_dirs)
# These attributes can be set dynamically by the page set.
self.user_agent_type = user_agent_type
......@@ -46,6 +50,13 @@ 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 = {}
......
......@@ -2,7 +2,6 @@
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
import inspect
import os
import unittest
......@@ -53,11 +52,9 @@ class TestPage(unittest.TestCase):
pages)
def testGetUrlBaseDirAndFileForUrlBaseDir(self):
base_dir = os.path.dirname(__file__)
file_path = os.path.dirname(base_dir) + '/otherdir/file.html'
ps = page_set.PageSet(file_path=base_dir, serving_dirs=['../somedir/'])
ps = page_set.PageSet(file_path='basedir/', serving_dirs=['../somedir/'])
ps.AddUserStory(page.Page('file://../otherdir/file.html', ps, ps.base_dir))
self.assertPathEqual(ps[0].file_path, file_path)
self.assertPathEqual(ps[0].file_path, 'otherdir/file.html')
def testDisplayUrlForHttp(self):
ps = page_set.PageSet(file_path=os.path.dirname(__file__))
......
......@@ -86,10 +86,10 @@ class PageSetSmokeTest(unittest.TestCase):
page, page_set.file_path)))
def CheckAttributesOfPageSetBasicAttributes(self, page_set):
if page_set.base_dir is not None:
if page_set.file_path is not None:
self.assertTrue(
isinstance(page_set.base_dir, str),
msg='page_set %\'s base_dir must have type string')
isinstance(page_set.file_path, str),
msg='page_set %\'s file_path must have type string')
self.assertTrue(
isinstance(page_set.archive_data_file, str),
......
......@@ -17,7 +17,7 @@ class UserStorySet(object):
"""
def __init__(self, archive_data_file='', cloud_storage_bucket=None,
base_dir=None, serving_dirs=None):
serving_dirs=None):
"""Creates a new UserStorySet.
Args:
......@@ -33,21 +33,11 @@ class UserStorySet(object):
self._wpr_archive_info = None
archive_info.AssertValidCloudStorageBucket(cloud_storage_bucket)
self._cloud_storage_bucket = cloud_storage_bucket
if base_dir:
if not os.path.isdir(base_dir):
raise ValueError('Must provide valid directory path for base_dir.')
self._base_dir = base_dir
else:
self._base_dir = os.path.dirname(inspect.getfile(self.__class__))
# Convert any relative serving_dirs to absolute paths.
self._serving_dirs = set(os.path.realpath(os.path.join(self.base_dir, d))
for d in serving_dirs or [])
@property
def file_path(self):
return inspect.getfile(self.__class__).replace('.pyc', '.py')
@property
def base_dir(self):
"""The base directory to resolve archive_data_file.
......
......@@ -2,7 +2,6 @@
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.
import inspect
import os
import unittest
......@@ -15,11 +14,6 @@ class UserStorySetFoo(user_story_set.UserStorySet):
pass
class MockUserStorySet(user_story_set.UserStorySet):
def __init__(self):
super(MockUserStorySet, self).__init__()
class UserStorySetTest(unittest.TestCase):
def testUserStoryTestName(self):
......@@ -36,11 +30,6 @@ class UserStorySetTest(unittest.TestCase):
self.assertTrue(os.path.isdir(base_dir))
self.assertEqual(base_dir, os.path.dirname(__file__))
def testFilePath(self):
uss = MockUserStorySet()
self.assertEqual(os.path.abspath(__file__),
uss.file_path)
def testCloudBucket(self):
blank_uss = user_story_set.UserStorySet()
self.assertEqual(blank_uss.bucket, None)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment