Skip to content

Commit

Permalink
Clovis: unittesting fix and some general tweaks.
Browse files Browse the repository at this point in the history
The root problem was that dependency_graph_unittest called
request_dependencies_lens.TestReqeusts.CreateLoadingTrace, and then changed some
of the timing of those requests. As those requests were global, that meant that
if a different unittest (eg, prefetch_view_unittest) happened to use the same
TestRequests, it would see the changed timing, and so run differently than if it
were run in isolation.

A nice way to fix that is to serialize and deserialize the trace, which exposed
some other holes in our organization, namely abstract classes that defined empty
methods instead of abstract methods (in the python world, that means using
"pass" instead of "raise NotImplementedError") and then some other gaps in our
serialization methods.

In order to diagnose & fix this, run_tests was extended to allow for multiple
tests to be specified. This does not change existing behavior when a single
argument is passed to run_tests, and does the right thing for multiple arguments
instead of silently ignoring them.

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

Cr-Commit-Position: refs/heads/master@{#387590}
  • Loading branch information
mattcary authored and Commit bot committed Apr 15, 2016
1 parent fc8ea0d commit 046e2f3
Show file tree
Hide file tree
Showing 6 changed files with 44 additions and 10 deletions.
14 changes: 9 additions & 5 deletions tools/android/loading/devtools_monitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -390,14 +390,14 @@ def Handle(self, method, msg):
event_name: (str) Event name, as registered.
event: (dict) complete event.
"""
pass
raise NotImplementedError


class Track(Listener):
"""Collects data from a DevTools server."""
def GetEvents(self):
"""Returns a list of collected events, finalizing the state if necessary."""
pass
raise NotImplementedError

def ToJsonDict(self):
"""Serializes to a dictionary, to be dumped as JSON.
Expand All @@ -406,10 +406,10 @@ def ToJsonDict(self):
A dict that can be dumped by the json module, and loaded by
FromJsonDict().
"""
pass
raise NotImplementedError

@classmethod
def FromJsonDict(cls, json_dict):
def FromJsonDict(cls, _json_dict):
"""Returns a Track instance constructed from data dumped by
Track.ToJsonDict().
Expand All @@ -419,4 +419,8 @@ def FromJsonDict(cls, json_dict):
Returns:
a Track instance.
"""
pass
# There is no sensible way to deserialize this abstract class, but
# subclasses are not required to define a deserialization method. For
# example, for testing we have a FakeRequestTrack which is never
# deserialized; instead fake instances are deserialized as RequestTracks.
assert False
6 changes: 4 additions & 2 deletions tools/android/loading/loading_trace.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ def ToJsonDict(self):
result = {self._URL_KEY: self.url, self._METADATA_KEY: self.metadata,
self._PAGE_KEY: self.page_track.ToJsonDict(),
self._REQUEST_KEY: self.request_track.ToJsonDict(),
self._TRACING_KEY: self.tracing_track.ToJsonDict()}
self._TRACING_KEY: (self.tracing_track.ToJsonDict()
if self.tracing_track else None)}
return result

def ToJsonFile(self, json_path):
Expand Down Expand Up @@ -111,7 +112,8 @@ def Slim(self):
self._tracing_track = None

def _RestoreTracingTrack(self):
assert self._tracing_json_str
if not self._tracing_json_str:
return None
self._tracing_track = tracing.TracingTrack.FromJsonDict(
json.loads(self._tracing_json_str))
self._tracing_json_str = None
5 changes: 4 additions & 1 deletion tools/android/loading/request_dependencies_lens_unittest.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,14 @@ class TestRequests(object):

@classmethod
def CreateLoadingTrace(cls, trace_events=None):
return test_utils.LoadingTraceFromEvents(
trace = test_utils.LoadingTraceFromEvents(
[cls.FIRST_REDIRECT_REQUEST, cls.SECOND_REDIRECT_REQUEST,
cls.REDIRECTED_REQUEST, cls.REQUEST, cls.JS_REQUEST, cls.JS_REQUEST_2,
cls.JS_REQUEST_OTHER_FRAME, cls.JS_REQUEST_UNRELATED_FRAME],
cls.PAGE_EVENTS, trace_events)
# Serialize and deserialize so that clients can change events without
# affecting future tests.
return LoadingTrace.FromJsonDict(trace.ToJsonDict())


class RequestDependencyLensTestCase(unittest.TestCase):
Expand Down
10 changes: 8 additions & 2 deletions tools/android/loading/run_tests
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,15 @@ if __name__ == '__main__':

suite = unittest.TestSuite()
loader = unittest.TestLoader()
pattern = '*%s*_unittest.py' % ('' if len(sys.argv) < 2 else sys.argv[1])
root_dir = os.path.dirname(os.path.realpath(__file__))
suite.addTests(loader.discover(start_dir=root_dir, pattern=pattern))
if len(sys.argv) < 2:
cases = loader.discover(start_dir=root_dir, pattern='*_unittest.py')
else:
cases = []
for module in sys.argv[1:]:
pattern = '{}_unittest.py'.format(module)
cases.extend(loader.discover(start_dir=root_dir, pattern=pattern))
suite.addTests(cases)
res = unittest.TextTestRunner(verbosity=2).run(suite)
if res.wasSuccessful():
sys.exit(0)
Expand Down
17 changes: 17 additions & 0 deletions tools/android/loading/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,20 @@ def __init__(self, events):
super(FakeRequestTrack, self).__init__(None)
self._events = [self._RewriteEvent(e) for e in events]

def Handle(self, _method, _msg):
assert False # Should never be called.

def GetEvents(self):
return self._events

def ToJsonDict(self):
cls = request_track.RequestTrack
return {cls._EVENTS_KEY: [
rq.ToJsonDict() for rq in self.GetEvents()],
cls._METADATA_KEY: {
cls._DUPLICATES_KEY: 0,
cls._INCONSISTENT_INITIATORS_KEY: 0}}

def _RewriteEvent(self, event):
# This modifies the instance used across tests, so this method
# must be idempotent.
Expand All @@ -33,6 +44,9 @@ def __init__(self, events):
super(FakePageTrack, self).__init__(None)
self._events = events

def Handle(self, _method, _msg):
assert False # Should never be called.

def GetEvents(self):
return self._events

Expand All @@ -42,6 +56,9 @@ def GetMainFrameId(self):
assert event['method'] == page_track.PageTrack.FRAME_STARTED_LOADING
return event['frame_id']

def ToJsonDict(self):
return {'events': [event for event in self._events]}


def MakeRequestWithTiming(
url, source_url, timing_dict, magic_content_type=False,
Expand Down
2 changes: 2 additions & 0 deletions tools/android/loading/tracing.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,8 @@ def Filter(self, pid=None, tid=None, categories=None):

@classmethod
def FromJsonDict(cls, json_dict):
if not json_dict:
return None
assert 'events' in json_dict
events = [Event(e) for e in json_dict['events']]
tracing_track = TracingTrack(None)
Expand Down

0 comments on commit 046e2f3

Please sign in to comment.