Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1721653003:
Make sure CHROMIUM_OUTPUT_DIR is set from runtest.py (Closed)
|
Created:
4 years, 10 months ago by agrieve Modified:
4 years, 9 months ago Reviewers:
jbudorick CC:
chromium-reviews, infra-reviews+build_chromium.org, kjellander-cc_chromium.org Base URL:
https://chromium.googlesource.com/chromium/tools/build.git@master Target Ref:
refs/heads/master Project:
build Visibility:
Public. |
DescriptionMake sure CHROMIUM_OUTPUT_DIR is set from runtest.py
BUG=594105
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=299248
Patch Set 1 #
Total comments: 2
Patch Set 2 : android only #Messages
Total messages: 15 (4 generated)
agrieve@chromium.org changed reviewers: + jbudorick@chromium.org
📻 Not sure if this is the best fix, but it's all I could figure out.
I don't think this is the right way to fix this. We should instead look at passing --output-directory directly in telemetry. If necessary, perhaps we could do this for the specific step. Also: my reviews are going to be slow (...er than normal) until Thursday, beware.
On 2016/02/22 23:04:22, jbudorick - slow til Feb 25 wrote: > I don't think this is the right way to fix this. We should instead look at > passing --output-directory directly in telemetry. If necessary, perhaps we could > do this for the specific step. > > Also: my reviews are going to be slow (...er than normal) until Thursday, > beware. I think this can be fixed by adding --chromium-output-directory to the recipe step, but I couldn't find where to do that. Any ideas?
On 2016/02/22 23:04:22, jbudorick - slow til Feb 25 wrote: > I don't think this is the right way to fix this. We should instead look at > passing --output-directory directly in telemetry. If necessary, perhaps we could > do this for the specific step. > > Also: my reviews are going to be slow (...er than normal) until Thursday, > beware. I think this can be fixed by adding --chromium-output-directory to the recipe step, but I couldn't find where to do that. Any ideas?
On 2016/02/23 01:07:45, agrieve wrote: > On 2016/02/22 23:04:22, jbudorick - slow til Feb 25 wrote: > > I don't think this is the right way to fix this. We should instead look at > > passing --output-directory directly in telemetry. If necessary, perhaps we > could > > do this for the specific step. > > > > Also: my reviews are going to be slow (...er than normal) until Thursday, > > beware. > > I think this can be fixed by adding --chromium-output-directory to the recipe > step, but I couldn't find where to do that. Any ideas? Okay, learned a bunch more about how this works. Some info: 1. Test command args are generated by running run_benchmark list, but this command doesn't know where the output directory is. 2. Test is run via run_sharded_perf_tests(), so it could set the env var there maybe? I'm not sure this is better than the current fix though (probably worse). 3. We should use an env var so that bisects still work (or else run into "unknown flag". wdyt? Is adding the env in build/scripts/slave/recipe_modules/chromium_android/api.py's run_sharded_perf_test any better?
Description was changed from ========== Make sure CHROMIUM_OUTPUT_DIR is set from runtest.py BUG=588180 ========== to ========== Make sure CHROMIUM_OUTPUT_DIR is set from runtest.py BUG=594105 ==========
On 2016/03/07 21:22:14, agrieve wrote: > On 2016/02/23 01:07:45, agrieve wrote: > > On 2016/02/22 23:04:22, jbudorick - slow til Feb 25 wrote: > > > I don't think this is the right way to fix this. We should instead look at > > > passing --output-directory directly in telemetry. If necessary, perhaps we > > could > > > do this for the specific step. > > > > > > Also: my reviews are going to be slow (...er than normal) until Thursday, > > > beware. > > > > I think this can be fixed by adding --chromium-output-directory to the recipe > > step, but I couldn't find where to do that. Any ideas? > > Okay, learned a bunch more about how this works. > > Some info: > 1. Test command args are generated by running run_benchmark list, but this > command doesn't know where the output directory is. > 2. Test is run via run_sharded_perf_tests(), so it could set the env var there > maybe? I'm not sure this is better than the current fix though (probably worse). > 3. We should use an env var so that bisects still work (or else run into > "unknown flag". > > wdyt? Is adding the env in > build/scripts/slave/recipe_modules/chromium_android/api.py's > run_sharded_perf_test any better? ping
https://codereview.chromium.org/1721653003/diff/1/scripts/slave/runtest.py File scripts/slave/runtest.py (right): https://codereview.chromium.org/1721653003/diff/1/scripts/slave/runtest.py#ne... scripts/slave/runtest.py:1860: if not os.environ.get('CHROMIUM_OUTPUT_DIR') and options.target: Can we limit this to Android?
https://codereview.chromium.org/1721653003/diff/1/scripts/slave/runtest.py File scripts/slave/runtest.py (right): https://codereview.chromium.org/1721653003/diff/1/scripts/slave/runtest.py#ne... scripts/slave/runtest.py:1860: if not os.environ.get('CHROMIUM_OUTPUT_DIR') and options.target: On 2016/03/11 16:28:14, jbudorick wrote: > Can we limit this to Android? Done.
lgtm
The CQ bit was checked by agrieve@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1721653003/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1721653003/20001
Message was sent while issue was closed.
Description was changed from ========== Make sure CHROMIUM_OUTPUT_DIR is set from runtest.py BUG=594105 ========== to ========== Make sure CHROMIUM_OUTPUT_DIR is set from runtest.py BUG=594105 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=299248 ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as http://src.chromium.org/viewvc/chrome?view=rev&revision=299248 |
