Index: scripts/slave/recipe_modules/swarming/api.py |
diff --git a/scripts/slave/recipe_modules/swarming/api.py b/scripts/slave/recipe_modules/swarming/api.py |
index da5afa4fa99aee6f42037ffb3c88659aba0d93e0..0aa2b99960d287bf68b12bf95f25771b855bb46d 100644 |
--- a/scripts/slave/recipe_modules/swarming/api.py |
+++ b/scripts/slave/recipe_modules/swarming/api.py |
@@ -770,6 +770,77 @@ class SwarmingApi(recipe_api.RecipeApi): |
merged_results[key][add_key] = chartjson_results_json[key][add_key] |
return merged_results |
+ def _merge_simplified_test_result_format(self, shard_results_list): |
+ merged_results = { |
+ 'successes': [], |
+ 'failures': [], |
+ 'valid': True, |
+ } |
+ for results_json in shard_results_list: |
+ for key in merged_results: |
+ if key in results_json: |
+ if isinstance(merged_results[key], list): |
+ merged_results[key].extend(results_json[key]) |
+ elif isinstance(merged_results[key], bool): |
+ merged_results[key] = merged_results[key] and results_json[key] |
+ else: |
+ raise recipe_api.InfraFailure( |
+ 'Unknown key type ' + type(merged_results[key]) + |
+ ' when handling key ' + key + '.') # pragma: no cover |
+ return merged_results |
+ |
+ def _merge_json_test_result_format(self, shard_results_list): |
+ merged_results = { |
+ 'tests': {}, |
+ 'interrupted': false, |
+ 'path_delimiter': '', |
+ 'version': 3, |
+ 'seconds_since_epoch': float('inf'), |
+ 'num_failures_by_type': { |
+ } |
+ } |
+ for result_json in shard_results_list: |
+ if not ('tests' in result_json and |
+ 'interrupted' in result_json and |
+ 'path_delimiter' in result_json and |
+ 'version' in result_json and |
+ 'seconds_since_epoch' in result_json and |
+ 'num_failures_by_type' in result_json): |
+ raise recipe_api.InfraFailure('Invalid json test results') |
Sergiy Byelozyorov
2016/09/29 21:36:22
I would double-check if everybody implements the f
nednguyen
2016/10/04 00:10:11
Hmhh, the spec say these fields are required?
htt
Sergiy Byelozyorov
2016/10/04 08:54:52
Sure. I just meant that you should make sure it do
Dirk Pranke
2016/10/04 20:04:02
We should try to follow the spec and update the sp
|
+ |
+ # Traverse the results_json's test trie & merged_results's test tries in |
+ # DFS order & add the n to merged['tests']. |
+ curr_result_nodes_queue = [results_json['tests']] |
+ merged_results_nodes_queue = [merged_results['tests']] |
+ while nodes_queue: |
Sergiy Byelozyorov
2016/09/29 21:36:22
I don't see nodes_queue defined anywhere... does t
nednguyen
2016/10/04 00:10:11
Yeah, I wasn't sure how to test these. Done.
|
+ curr_node = curr_result_nodes_queue.pop() |
+ merged_node = merged_results_nodes_queue.pop() |
+ for k, v in curr_node.iteritems(): |
+ if k in merged_node: |
+ curr_result_nodes_queue.push(v) |
+ merged_results_nodes_queue.push(merged_node[k]) |
+ else: |
+ merged_node[k] = v |
+ |
+ # Update the rest of the fields for merged_results. |
+ merged_results['interrupted'] |= results_json['interrupted'] |
+ if not merged_results['path_delimiter']: |
+ merged_results['path_delimiter'] = results_json['path_delimiter'] |
+ elif merged_results['path_delimiter'] != results_json['path_delimiter']: |
+ raise Exception( |
+ 'Incosistent path delimiter: %s %s' % |
+ (merged_results['path_delimiter'], |
+ results_json['path_delimiter'])) |
+ if results_json['version'] != 3: |
+ raise recipe_api.InfraFailure( |
+ 'Only version 3 of json test result format is supported') |
+ merged_results['seconds_since_epoch'] = min( |
+ merged_results['seconds_since_epoch'], |
+ results_json['seconds_since_epoch']) |
Sergiy Byelozyorov
2016/09/29 21:36:22
Any particular reason to pick the smaller of the t
nednguyen
2016/10/04 00:10:11
I honestly not sure what is the reason for this fi
Sergiy Byelozyorov
2016/10/04 08:54:52
AFAIK, this is when the tests were run... probably
nednguyen
2016/10/04 13:30:29
Ah, I just think that the earliest seconds_since_e
|
+ for result_type, count in results_json['num_failures_by_type']: |
+ if not result_type in merged_results['num_failures_by_type']: |
Sergiy Byelozyorov
2016/09/29 21:36:22
IMHO this is more Pythonic:
merged_results['num_f
nednguyen
2016/10/04 00:10:11
Done.
|
+ merged_results['num_failures_by_type'][result_type] = 0 |
+ merged_results['num_failures_by_type'][result_type] += count |
def _merge_isolated_script_shards(self, task, step_result): |
# This code is unfortunately specialized to the "simplified" |
@@ -782,11 +853,7 @@ class SwarmingApi(recipe_api.RecipeApi): |
# switch the isolated tests over when adding sharding support. |
# |
# These are the only keys we pay attention to in the output JSON. |
Ken Russell (switch to Gerrit)
2016/09/30 23:37:53
Please move this comment to the top of _merge_simp
nednguyen
2016/10/04 00:10:11
Done.
|
- merged_results = { |
- 'successes': [], |
- 'failures': [], |
- 'valid': True, |
- } |
+ shard_results_list = [] |
for i in xrange(task.shards): |
path = self.m.path.join(str(i), 'output.json') |
if path not in step_result.raw_io.output_dir: |
@@ -794,19 +861,15 @@ class SwarmingApi(recipe_api.RecipeApi): |
results_raw = step_result.raw_io.output_dir[path] |
try: |
results_json = self.m.json.loads(results_raw) |
+ shard_results_list.append(results_json) |
except Exception as e: |
raise Exception('error decoding JSON results from shard #%d' % i) |
- for key in merged_results: |
- if key in results_json: |
- if isinstance(merged_results[key], list): |
- merged_results[key].extend(results_json[key]) |
- elif isinstance(merged_results[key], bool): |
- merged_results[key] = merged_results[key] and results_json[key] |
- else: |
- raise recipe_api.InfraFailure( |
- 'Unknown key type ' + type(merged_results[key]) + |
- ' when handling key ' + key + '.') # pragma: no cover |
- return merged_results |
+ if not shard_results_list: |
eyaich1
2016/09/29 14:55:23
no need for this check you create it above
Sergiy Byelozyorov
2016/09/29 21:36:22
But it can be empty, can't it?
|
+ raise Exception('No shard results is created') |
Ken Russell (switch to Gerrit)
2016/09/30 23:37:53
Grammar: remove "is".
nednguyen
2016/10/04 00:10:11
Done.
|
+ elif 'seconds_since_epoch' in shard_results_list[0]: |
eyaich1
2016/09/29 14:55:23
is this the distinguishing factor between the resu
nednguyen
2016/10/04 00:10:11
This is the distinguished factor between the simpl
|
+ return self._merge_json_test_result_format[shard_results_list] |
+ else: |
+ return self._merge_simplified_json_format(shard_results_list) |
def _isolated_script_collect_step(self, task, **kwargs): |
step_test_data = kwargs.pop('step_test_data', None) |