Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2251863003:
Add tombstones as part of json-results-file for android instr tests. (Closed)
|
Created:
4 years, 4 months ago by BigBossZhiling Modified:
4 years, 4 months ago Reviewers:
jbudorick CC:
chromium-reviews, jbudorick+watch_chromium.org, mikecase+watch_chromium.org Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionAdd tombstones as part of json-results-file for android instr tests.
When a testcase crashes, a tombstone will be created. However in the
json-results-file, the tombstone information was not included. In this
cl, tombstones becomes a part of json-results-file.
BUG=631213
Committed: https://crrev.com/6fd77f58bc7711f0b4e701b1d3562076c1756e49
Cr-Commit-Position: refs/heads/master@{#413646}
Patch Set 1 #
Total comments: 6
Patch Set 2 : moved tombstone to base test result #
Total comments: 4
Patch Set 3 : address comments #
Total comments: 2
Patch Set 4 : minor fixes #
Messages
Total messages: 22 (8 generated)
Description was changed from ========== Add tombstones as part of json-results-file for android instr tests. When a testcase crashes, a tombstone will be created. However in the json-results-file, the tombstone information was not included. In this cl, tombstones becomes a part of json-results-file. BUG=631213 ========== to ========== Add tombstones as part of json-results-file for android instr tests. When a testcase crashes, a tombstone will be created. However in the json-results-file, the tombstone information was not included. In this cl, tombstones becomes a part of json-results-file. BUG=631213 ==========
hzl@google.com changed reviewers: + jbudorick@chromium.org
https://codereview.chromium.org/2251863003/diff/1/build/android/pylib/instrum... File build/android/pylib/instrumentation/test_result.py (left): https://codereview.chromium.org/2251863003/diff/1/build/android/pylib/instrum... build/android/pylib/instrumentation/test_result.py:31: self._tombstones = None I think this should stay None. Write an empty string to the JSON if it's None when you go to create the JSON. https://codereview.chromium.org/2251863003/diff/1/build/android/pylib/results... File build/android/pylib/results/json_results.py (right): https://codereview.chromium.org/2251863003/diff/1/build/android/pylib/results... build/android/pylib/results/json_results.py:105: 'output_snippet_base64:': '',} Move the } back down, please. https://codereview.chromium.org/2251863003/diff/1/build/android/pylib/results... build/android/pylib/results/json_results.py:106: if type(r) == test_result.InstrumentationTestResult: We shouldn't be querying the derived type. If you want tombstones in the result, add a GetTombstones method to base_test_result. We'll probably want it later for gtests anyway.
https://codereview.chromium.org/2251863003/diff/1/build/android/pylib/instrum... File build/android/pylib/instrumentation/test_result.py (left): https://codereview.chromium.org/2251863003/diff/1/build/android/pylib/instrum... build/android/pylib/instrumentation/test_result.py:31: self._tombstones = None On 2016/08/17 04:14:30, jbudorick wrote: > I think this should stay None. Write an empty string to the JSON if it's None > when you go to create the JSON. Done. https://codereview.chromium.org/2251863003/diff/1/build/android/pylib/results... File build/android/pylib/results/json_results.py (right): https://codereview.chromium.org/2251863003/diff/1/build/android/pylib/results... build/android/pylib/results/json_results.py:105: 'output_snippet_base64:': '',} On 2016/08/17 04:14:30, jbudorick wrote: > Move the } back down, please. Done. https://codereview.chromium.org/2251863003/diff/1/build/android/pylib/results... build/android/pylib/results/json_results.py:106: if type(r) == test_result.InstrumentationTestResult: On 2016/08/17 04:14:30, jbudorick wrote: > We shouldn't be querying the derived type. If you want tombstones in the result, > add a GetTombstones method to base_test_result. We'll probably want it later for > gtests anyway. Done.
https://codereview.chromium.org/2251863003/diff/20001/build/android/pylib/res... File build/android/pylib/results/json_results.py (right): https://codereview.chromium.org/2251863003/diff/20001/build/android/pylib/res... build/android/pylib/results/json_results.py:104: } left four spaces https://codereview.chromium.org/2251863003/diff/20001/build/android/pylib/res... build/android/pylib/results/json_results.py:105: if r.GetTombstones(): You can consolidate this by just adding 'tombstones': r.GetTombstones() or '' to the dict declaration.
https://codereview.chromium.org/2251863003/diff/20001/build/android/pylib/res... File build/android/pylib/results/json_results.py (right): https://codereview.chromium.org/2251863003/diff/20001/build/android/pylib/res... build/android/pylib/results/json_results.py:104: } On 2016/08/18 19:36:18, jbudorick wrote: > left four spaces Done. https://codereview.chromium.org/2251863003/diff/20001/build/android/pylib/res... build/android/pylib/results/json_results.py:105: if r.GetTombstones(): On 2016/08/18 19:36:18, jbudorick wrote: > You can consolidate this by just adding > > 'tombstones': r.GetTombstones() or '' > > to the dict declaration. Done.
lgtm w/ nit https://codereview.chromium.org/2251863003/diff/40001/build/android/pylib/res... File build/android/pylib/results/json_results.py (right): https://codereview.chromium.org/2251863003/diff/40001/build/android/pylib/res... build/android/pylib/results/json_results.py:105: result_dict['tombstones'] = r.GetTombstones() or '' nit: you can do this directly in the declaration: result_dict = { # other fields 'tombstones': r.GetTombstones() or '' }
https://codereview.chromium.org/2251863003/diff/40001/build/android/pylib/res... File build/android/pylib/results/json_results.py (right): https://codereview.chromium.org/2251863003/diff/40001/build/android/pylib/res... build/android/pylib/results/json_results.py:105: result_dict['tombstones'] = r.GetTombstones() or '' On 2016/08/19 01:03:53, jbudorick wrote: > nit: you can do this directly in the declaration: > > result_dict = { > # other fields > 'tombstones': r.GetTombstones() or '' > } Done.
The CQ bit was checked by hzl@google.com
The patchset sent to the CQ was uploaded after l-g-t-m from jbudorick@chromium.org Link to the patchset: https://codereview.chromium.org/2251863003/#ps60001 (title: "minor fixes")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
The CQ bit was checked by hzl@google.com
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
Message was sent while issue was closed.
Description was changed from ========== Add tombstones as part of json-results-file for android instr tests. When a testcase crashes, a tombstone will be created. However in the json-results-file, the tombstone information was not included. In this cl, tombstones becomes a part of json-results-file. BUG=631213 ========== to ========== Add tombstones as part of json-results-file for android instr tests. When a testcase crashes, a tombstone will be created. However in the json-results-file, the tombstone information was not included. In this cl, tombstones becomes a part of json-results-file. BUG=631213 ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001)
Message was sent while issue was closed.
Description was changed from ========== Add tombstones as part of json-results-file for android instr tests. When a testcase crashes, a tombstone will be created. However in the json-results-file, the tombstone information was not included. In this cl, tombstones becomes a part of json-results-file. BUG=631213 ========== to ========== Add tombstones as part of json-results-file for android instr tests. When a testcase crashes, a tombstone will be created. However in the json-results-file, the tombstone information was not included. In this cl, tombstones becomes a part of json-results-file. BUG=631213 Committed: https://crrev.com/6fd77f58bc7711f0b4e701b1d3562076c1756e49 Cr-Commit-Position: refs/heads/master@{#413646} ==========
Message was sent while issue was closed.
Patchset 4 (id:??) landed as https://crrev.com/6fd77f58bc7711f0b4e701b1d3562076c1756e49 Cr-Commit-Position: refs/heads/master@{#413646} |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
