Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1971433002:
ABANDONED [Android] Expose each try result in test results JSON. (Closed)
DescriptionABANDONED [Android] Expose each try result in test results JSON.
BUG=609588
Patch Set 1 #
Total comments: 6
Patch Set 2 : rebase #Patch Set 3 : fix instrumentation test handling #Patch Set 4 : fix non-hashable test case #Patch Set 5 : fixes: use last result in an iteration, only use iteration_results in exit code determination #
Messages
Total messages: 30 (15 generated)
jbudorick@chromium.org changed reviewers: + mikecase@chromium.org, rnephew@chromium.org
ping
Some questions about how results are stored. https://codereview.chromium.org/1971433002/diff/1/build/android/pylib/local/d... File build/android/pylib/local/device/local_device_test_run.py (right): https://codereview.chromium.org/1971433002/diff/1/build/android/pylib/local/d... build/android/pylib/local/device/local_device_test_run.py:134: results.append(try_results) Should this be results.extend(try_results)? Im kinda pretty confused how all these results are structured actually. I think it would be clearer if we kept all the results for each single test in a separate list like.... results['testname'].append(try_results['testname']) https://codereview.chromium.org/1971433002/diff/1/build/android/test_runner.py File build/android/test_runner.py (right): https://codereview.chromium.org/1971433002/diff/1/build/android/test_runner.p... build/android/test_runner.py:867: iteration_results.AddTestRunResults(r) I find this pretty confusing. So, just to make sure I am getting it right.... raw_results is a list of list of results gotten from retries. So like... raw_results = [['Test1 Try1', 'Test2 Try1'], ['Test2 Try 2']] Then results is a just a super extended list of these lists like.... results = [['Test1 Try1', 'Test2 Try1'], ['Test2 Try 2'], //repition 1 ['Test1 Try1', 'Test2 Try1'], ['Test1 Try2', 'Test2 Try 2']] //repition 2 If this is the case, maybe add some more comments. And maybe some names like... repetition_results instead of results or something. Also, would it make more sense to store it more like how the JSON results is stored { 'repitition1': { 'test1': ['try1', 'try2'], 'test2': ['try1'] } 'repitition2': { 'test1': ['try1', 'try2'], 'test2': ['try1'] } }
https://codereview.chromium.org/1971433002/diff/1/build/android/pylib/local/d... File build/android/pylib/local/device/local_device_test_run.py (right): https://codereview.chromium.org/1971433002/diff/1/build/android/pylib/local/d... build/android/pylib/local/device/local_device_test_run.py:134: results.append(try_results) On 2016/05/13 17:32:13, mikecase wrote: > Should this be results.extend(try_results)? Im kinda pretty confused how all No, try_results is a single instance of base_test_result.TestRunResults > these results are structured actually. I think it would be clearer if we kept > all the results for each single test in a separate list like.... > > results['testname'].append(try_results['testname']) Something like that would be neat as part of a larger rewrite of how we handle results (and, probably, pending tests). I don't have the time to do that at the moment. https://codereview.chromium.org/1971433002/diff/1/build/android/test_runner.py File build/android/test_runner.py (right): https://codereview.chromium.org/1971433002/diff/1/build/android/test_runner.p... build/android/test_runner.py:867: iteration_results.AddTestRunResults(r) On 2016/05/13 17:32:13, mikecase wrote: > I find this pretty confusing. So, just to make sure I am getting it right.... > > raw_results is a list of list of results gotten from retries. So like... > raw_results = [['Test1 Try1', 'Test2 Try1'], ['Test2 Try 2']] No, raw_results is a list of base_test_result.TestRunResults objects. > > Then results is a just a super extended list of these lists like.... > > results = [['Test1 Try1', 'Test2 Try1'], ['Test2 Try 2'], //repition 1 > ['Test1 Try1', 'Test2 Try1'], ['Test1 Try2', 'Test2 Try 2']] > //repition 2 wrong for the same reason as above > > If this is the case, maybe add some more comments. And maybe some names like... > repetition_results instead of results or something. > > > Also, would it make more sense to store it more like how the JSON results is > stored > > { > 'repitition1': { > 'test1': ['try1', 'try2'], > 'test2': ['try1'] > } > 'repitition2': { > 'test1': ['try1', 'try2'], > 'test2': ['try1'] > } > }
code looks good lgtm. Just make sure (if you haven't already) that the json results is formatted how you want. https://codereview.chromium.org/1971433002/diff/1/build/android/test_runner.py File build/android/test_runner.py (right): https://codereview.chromium.org/1971433002/diff/1/build/android/test_runner.p... build/android/test_runner.py:863: results.extend(raw_results) My only question I guess would be will the GenerateJson function put each repetition in a separate 'iteration_results' dict (I assume we want that)? Since you are just extending the results list here for all the repetitions my suspicion would be that it won't do that. https://code.google.com/p/chromium/codesearch#chromium/src/build/android/pyli...
https://codereview.chromium.org/1971433002/diff/1/build/android/test_runner.py File build/android/test_runner.py (right): https://codereview.chromium.org/1971433002/diff/1/build/android/test_runner.p... build/android/test_runner.py:863: results.extend(raw_results) On 2016/05/13 17:45:34, mikecase wrote: > My only question I guess would be will the GenerateJson function put each > repetition in a separate 'iteration_results' dict (I assume we want that)? > > Since you are just extending the results list here for all the repetitions my > suspicion would be that it won't do that. > > https://code.google.com/p/chromium/codesearch#chromium/src/build/android/pyli... It should: https://code.google.com/p/chromium/codesearch#chromium/src/build/android/pyli...
The CQ bit was checked by jbudorick@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from mikecase@chromium.org Link to the patchset: https://codereview.chromium.org/1971433002/#ps20001 (title: "rebase")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1971433002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1971433002/20001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
The CQ bit was checked by jbudorick@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from mikecase@chromium.org Link to the patchset: https://codereview.chromium.org/1971433002/#ps40001 (title: "fix instrumentation test handling")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1971433002/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1971433002/40001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
The CQ bit was checked by jbudorick@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from mikecase@chromium.org Link to the patchset: https://codereview.chromium.org/1971433002/#ps60001 (title: "fix non-hashable test case")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1971433002/60001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1971433002/60001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
The CQ bit was checked by jbudorick@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from mikecase@chromium.org Link to the patchset: https://codereview.chromium.org/1971433002/#ps80001 (title: "fixes: use last result in an iteration, only use iteration_results in exit code determination")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1971433002/80001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1971433002/80001
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: linux_android_rel_ng on tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
Closing this out for restructuring & rereview.
Description was changed from ========== [Android] Expose each try result in test results JSON. BUG=609588 ========== to ========== ABANDONED [Android] Expose each try result in test results JSON. BUG=609588 ==========
Description was changed from ========== ABANDONED [Android] Expose each try result in test results JSON. BUG=609588 ========== to ========== ABANDONED [Android] Expose each try result in test results JSON. BUG=609588 ========== |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
