Diff
Modified: trunk/Tools/ChangeLog (100874 => 100875)
--- trunk/Tools/ChangeLog 2011-11-20 23:55:31 UTC (rev 100874)
+++ trunk/Tools/ChangeLog 2011-11-21 01:26:57 UTC (rev 100875)
@@ -1,5 +1,22 @@
2011-11-20 Ojan Vafai <[email protected]>
+ Change the final place where we use version 3 of the results json output
+ https://bugs.webkit.org/show_bug.cgi?id=72838
+
+ Reviewed by Adam Barth.
+
+ This converts the json from being a flat map of test name --> results
+ to being hierarchical by directory. This will make the json files
+ considerably smaller.
+
+ Also cleaned up some functions that were returning/checking boolean values
+ that were always True.
+
+ * TestResultServer/model/jsonresults.py:
+ * TestResultServer/model/jsonresults_unittest.py:
+
+2011-11-20 Ojan Vafai <[email protected]>
+
Include the master name when querying the test results server.
This will soon be required since leaving the master name out mean
that multiple files could match the query.
Modified: trunk/Tools/TestResultServer/model/jsonresults.py (100874 => 100875)
--- trunk/Tools/TestResultServer/model/jsonresults.py 2011-11-20 23:55:31 UTC (rev 100874)
+++ trunk/Tools/TestResultServer/model/jsonresults.py 2011-11-21 01:26:57 UTC (rev 100875)
@@ -44,12 +44,41 @@
JSON_RESULTS_PASS = "P"
JSON_RESULTS_NO_DATA = "N"
JSON_RESULTS_MIN_TIME = 1
-JSON_RESULTS_VERSION = 3
JSON_RESULTS_HIERARCHICAL_VERSION = 4
JSON_RESULTS_MAX_BUILDS = 750
JSON_RESULTS_MAX_BUILDS_SMALL = 200
+def _add_path_to_trie(path, value, trie):
+ if not "/" in path:
+ trie[path] = value
+ return
+
+ directory, slash, rest = path.partition("/")
+ if not directory in trie:
+ trie[directory] = {}
+ _add_path_to_trie(rest, value, trie[directory])
+
+
+def _trie_json_tests(tests):
+ """Breaks a test name into chunks by directory and puts the test time as a value in the lowest part, e.g.
+ foo/bar/baz.html: VALUE1
+ foo/bar/baz1.html: VALUE2
+
+ becomes
+ foo: {
+ bar: {
+ baz.html: VALUE1,
+ baz1.html: VALUE2
+ }
+ }
+ """
+ trie = {}
+ for test, value in tests.iteritems():
+ _add_path_to_trie(test, value, trie)
+ return trie
+
+
class JsonResults(object):
@classmethod
def _strip_prefix_suffix(cls, data):
@@ -78,16 +107,12 @@
@classmethod
def _merge_json(cls, aggregated_json, incremental_json, num_runs):
- if not cls._merge_non_test_data(aggregated_json, incremental_json, num_runs):
- return False
-
+ cls._merge_non_test_data(aggregated_json, incremental_json, num_runs)
incremental_tests = incremental_json[JSON_RESULTS_TESTS]
if incremental_tests:
aggregated_tests = aggregated_json[JSON_RESULTS_TESTS]
cls._merge_tests(aggregated_tests, incremental_tests, num_runs)
- return True
-
@classmethod
def _merge_non_test_data(cls, aggregated_json, incremental_json, num_runs):
incremental_builds = incremental_json[JSON_RESULTS_BUILD_NUMBERS]
@@ -101,8 +126,6 @@
# Merge this build into aggreagated results.
cls._merge_one_build(aggregated_json, incremental_json, index, num_runs)
- return True
-
@classmethod
def _merge_one_build(cls, aggregated_json, incremental_json, incremental_index, num_runs):
for key in incremental_json.keys():
@@ -119,24 +142,33 @@
@classmethod
def _merge_tests(cls, aggregated_json, incremental_json, num_runs):
- all_tests = set(aggregated_json.iterkeys()) | set(incremental_json.iterkeys())
+ all_tests = set(aggregated_json.iterkeys())
+ if incremental_json:
+ all_tests |= set(incremental_json.iterkeys())
+
for test_name in all_tests:
- if test_name in aggregated_json:
- aggregated_test = aggregated_json[test_name]
- if test_name in incremental_json:
- incremental_test = incremental_json[test_name]
- results = incremental_test[JSON_RESULTS_RESULTS]
- times = incremental_test[JSON_RESULTS_TIMES]
- else:
- results = [[1, JSON_RESULTS_NO_DATA]]
- times = [[1, 0]]
+ if test_name not in aggregated_json:
+ aggregated_json[test_name] = incremental_json[test_name]
+ continue
- cls._insert_item_run_length_encoded(results, aggregated_test[JSON_RESULTS_RESULTS], num_runs)
- cls._insert_item_run_length_encoded(times, aggregated_test[JSON_RESULTS_TIMES], num_runs)
- cls._normalize_results_json(test_name, aggregated_json, num_runs)
+ if JSON_RESULTS_RESULTS not in aggregated_json[test_name]:
+ incremental_sub_result = incremental_json[test_name] if test_name in incremental_json else None
+ cls._merge_tests(aggregated_json[test_name], incremental_sub_result, num_runs)
+ continue
+
+ if incremental_json and test_name in incremental_json:
+ incremental_test = incremental_json[test_name]
+ results = incremental_test[JSON_RESULTS_RESULTS]
+ times = incremental_test[JSON_RESULTS_TIMES]
else:
- aggregated_json[test_name] = incremental_json[test_name]
+ results = [[1, JSON_RESULTS_NO_DATA]]
+ times = [[1, 0]]
+ aggregated_test = aggregated_json[test_name]
+ cls._insert_item_run_length_encoded(results, aggregated_test[JSON_RESULTS_RESULTS], num_runs)
+ cls._insert_item_run_length_encoded(times, aggregated_test[JSON_RESULTS_TIMES], num_runs)
+ cls._normalize_results_json(test_name, aggregated_json, num_runs)
+
@classmethod
def _insert_item_run_length_encoded(cls, incremental_item, aggregated_item, num_runs):
for item in incremental_item:
@@ -175,27 +207,13 @@
return len(results) == 1 and results[0][1] == type
@classmethod
- def _flatten_json_tests(cls, json, prefix=None):
- """Flattens a trie directory structure of tests into a flat structure.
- """
- result = {}
- for name, test in json.iteritems():
- if prefix:
- fullname = prefix + "/" + name
- else:
- fullname = name
-
- if "results" in test:
- result[fullname] = test
- else:
- result.update(cls._flatten_json_tests(test, fullname))
-
- return result
-
- @classmethod
def _remove_gtest_modifiers(cls, builder, json):
tests = json[builder][JSON_RESULTS_TESTS]
new_tests = {}
+ # FIXME: This is wrong. If the test exists in the incremental results as both values, then one will overwrite the other.
+ # We should instead pick the one that doesn't have NO_DATA as its value.
+ # Alternately we could fix this by having the JSON generation code on the buildbot only include the test
+ # that was actually run.
for name, test in tests.iteritems():
new_name = name.replace('.FLAKY_', '.', 1)
new_name = new_name.replace('.FAILS_', '.', 1)
@@ -221,13 +239,10 @@
logging.error("Missing build number in json results.")
return False
- # FIXME(aboxhall): Once the dashboard can read hierarchical JSON, both
- # incremental and aggregated JSON can be hierarchical, with no need to
- # flatten here.
- if version == JSON_RESULTS_HIERARCHICAL_VERSION:
- flattened_tests = cls._flatten_json_tests(results_for_builder[JSON_RESULTS_TESTS])
- json[builder][JSON_RESULTS_TESTS] = flattened_tests
- json[JSON_RESULTS_VERSION_KEY] = JSON_RESULTS_VERSION
+ # FIXME: Once all the bots have cycled, we can remove this code since all the results will be heirarchical.
+ if version < JSON_RESULTS_HIERARCHICAL_VERSION:
+ json[builder][JSON_RESULTS_TESTS] = _trie_json_tests(results_for_builder[JSON_RESULTS_TESTS])
+ json[JSON_RESULTS_VERSION_KEY] = JSON_RESULTS_HIERARCHICAL_VERSION
return True
@@ -260,13 +275,12 @@
logging.info("Merging json results...")
try:
- if not cls._merge_json(aggregated_json[builder], incremental_json[builder], num_runs):
- return None
+ cls._merge_json(aggregated_json[builder], incremental_json[builder], num_runs)
except Exception, err:
logging.error("Failed to merge json results: %s", str(err))
return None
- aggregated_json[JSON_RESULTS_VERSION_KEY] = JSON_RESULTS_VERSION
+ aggregated_json[JSON_RESULTS_VERSION_KEY] = JSON_RESULTS_HIERARCHICAL_VERSION
return cls._generate_file_data(aggregated_json, sort_keys)
Modified: trunk/Tools/TestResultServer/model/jsonresults_unittest.py (100874 => 100875)
--- trunk/Tools/TestResultServer/model/jsonresults_unittest.py 2011-11-20 23:55:31 UTC (rev 100874)
+++ trunk/Tools/TestResultServer/model/jsonresults_unittest.py 2011-11-21 01:26:57 UTC (rev 100875)
@@ -113,10 +113,8 @@
json = json.replace("[TESTDATA_CHROMEREVISION]", ",".join(chrome_revision))
json = json.replace("[TESTDATA_TIMES]", ",".join(times))
- if "version" in test_data:
- json = json.replace("[VERSION]", str(test_data["version"]))
- else:
- json = json.replace("[VERSION]", "3")
+ version = str(test_data["version"]) if "version" in test_data else "4"
+ json = json.replace("[VERSION]", version)
json_tests = []
for (name, test) in sorted(tests.iteritems()):
@@ -556,15 +554,23 @@
self._test_merge(
# Aggregated results
{"builds": ["2", "1"],
- "tests": {"foo/001.html": {
+ "tests": {"bar/003.html": {
+ "results": "[25,\"F\"]",
+ "times": "[25,0]"},
+ "foo/001.html": {
"results": "[50,\"F\"]",
"times": "[50,0]"},
"foo/002.html": {
"results": "[100,\"I\"]",
- "times": "[100,0]"}}},
+ "times": "[100,0]"}},
+ "version": 3},
# Incremental results
{"builds": ["3"],
- "tests": {"foo": {
+ "tests": {"baz": {
+ "004.html": {
+ "results": "[1,\"I\"]",
+ "times": "[1,0]"}},
+ "foo": {
"001.html": {
"results": "[1,\"F\"]",
"times": "[1,0]"},
@@ -574,13 +580,22 @@
"version": 4},
# Expected results
{"builds": ["3", "2", "1"],
- "tests": {"foo/001.html": {
- "results": "[51,\"F\"]",
- "times": "[51,0]"},
- "foo/002.html": {
- "results": "[101,\"I\"]",
- "times": "[101,0]"}},
- "version": 3})
+ "tests": {"bar": {
+ "003.html": {
+ "results": "[1,\"N\"],[25,\"F\"]",
+ "times": "[26,0]"}},
+ "baz": {
+ "004.html": {
+ "results": "[1,\"I\"]",
+ "times": "[1,0]"}},
+ "foo": {
+ "001.html": {
+ "results": "[51,\"F\"]",
+ "times": "[51,0]"},
+ "002.html": {
+ "results": "[101,\"I\"]",
+ "times": "[101,0]"}}},
+ "version": 4})
# FIXME(aboxhall): Add some tests for xhtml/svg test results.
@@ -612,7 +627,8 @@
"foo.FAILS_bar3": {
"results": "[100,\"I\"]",
"times": "[100,0]"},
- }},
+ },
+ "version": 3},
# Incremental results
{"builds": ["3"],
"tests": {"foo.FLAKY_bar": {
@@ -648,7 +664,7 @@
"foo.bar4": {
"results": "[1,\"I\"]",
"times": "[1,0]"}},
- "version": 3})
+ "version": 4})
if __name__ == '__main__':
unittest.main()
