Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
1692253003:
[Android] Add compile target -> old name map for instrumentation tests. (Closed)
|
Created:
4 years, 10 months ago by jbudorick Modified:
4 years, 9 months ago 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. |
Description[Android] Add compile target -> old name map for instrumentation tests.
This also reverts commit 97e7b89174c495ca53d07c60ec1dd0a3b2e75e8a.
BUG=525873
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=299050
Patch Set 1 #Patch Set 2 : return of the main target #
Total comments: 10
Patch Set 3 : stip comments #Messages
Total messages: 15 (7 generated)
jbudorick@chromium.org changed reviewers: + stip@chromium.org
let's abandon the CamelCase suite names instead of moving them into src :/
jbudorick@chromium.org changed reviewers: + martiniss@chromium.org
revised w/ main targets, ptal
lgtm as long as we're aligned for getting all info out of this thing you'll need an OWNER as well https://codereview.chromium.org/1692253003/diff/20001/scripts/slave/recipe_mo... File scripts/slave/recipe_modules/chromium_tests/steps.py (left): https://codereview.chromium.org/1692253003/diff/20001/scripts/slave/recipe_mo... scripts/slave/recipe_modules/chromium_tests/steps.py:1527: ANDROID_INSTRUMENTATION_TARGET_MAP = { I pulled these out of the class because I needed to write a second, swarmed class which had to read this data. I guess if we switch to non-camel I won't need this though. https://codereview.chromium.org/1692253003/diff/20001/scripts/slave/recipe_mo... File scripts/slave/recipe_modules/chromium_tests/steps.py (right): https://codereview.chromium.org/1692253003/diff/20001/scripts/slave/recipe_mo... scripts/slave/recipe_modules/chromium_tests/steps.py:1581: _DEFAULT_SUITES_BY_TARGET = { Ideally we can delete once we're only using snake_case? https://codereview.chromium.org/1692253003/diff/20001/scripts/slave/recipe_mo... scripts/slave/recipe_modules/chromium_tests/steps.py:1588: 'system_webview_shell_layout_test_apk': _DEFAULT_SUITES['SystemWebViewShellLayoutTest'], nit: 80 https://codereview.chromium.org/1692253003/diff/20001/scripts/slave/recipe_mo... scripts/slave/recipe_modules/chromium_tests/steps.py:1598: or AndroidInstrumentationTest._DEFAULT_SUITES_BY_TARGET.get(name) 80 https://codereview.chromium.org/1692253003/diff/20001/scripts/slave/recipe_mo... scripts/slave/recipe_modules/chromium_tests/steps.py:1601: compile_targets = [suite_defaults['compile_target']] this will raise an exception if the target isn't in the map. maybe that's what we want?
jbudorick@chromium.org changed reviewers: + phajdan.jr@chromium.org
+phajdan.jr for chromium_tests OWNERS https://codereview.chromium.org/1692253003/diff/20001/scripts/slave/recipe_mo... File scripts/slave/recipe_modules/chromium_tests/steps.py (left): https://codereview.chromium.org/1692253003/diff/20001/scripts/slave/recipe_mo... scripts/slave/recipe_modules/chromium_tests/steps.py:1527: ANDROID_INSTRUMENTATION_TARGET_MAP = { On 2016/03/01 15:54:52, stip (damaged and oooish) wrote: > I pulled these out of the class because I needed to write a second, swarmed > class which had to read this data. I guess if we switch to non-camel I won't > need this though. yeah, I was thinking that you'd be able to just do swarming_target = 'run_%' % name once name is the compile target rather than camel case. https://codereview.chromium.org/1692253003/diff/20001/scripts/slave/recipe_mo... File scripts/slave/recipe_modules/chromium_tests/steps.py (right): https://codereview.chromium.org/1692253003/diff/20001/scripts/slave/recipe_mo... scripts/slave/recipe_modules/chromium_tests/steps.py:1581: _DEFAULT_SUITES_BY_TARGET = { On 2016/03/01 15:54:52, stip (damaged and oooish) wrote: > Ideally we can delete once we're only using snake_case? yeah, I've only got both here for the src-side transition. https://codereview.chromium.org/1692253003/diff/20001/scripts/slave/recipe_mo... scripts/slave/recipe_modules/chromium_tests/steps.py:1588: 'system_webview_shell_layout_test_apk': _DEFAULT_SUITES['SystemWebViewShellLayoutTest'], On 2016/03/01 15:54:52, stip (damaged and oooish) wrote: > nit: 80 Done. https://codereview.chromium.org/1692253003/diff/20001/scripts/slave/recipe_mo... scripts/slave/recipe_modules/chromium_tests/steps.py:1598: or AndroidInstrumentationTest._DEFAULT_SUITES_BY_TARGET.get(name) On 2016/03/01 15:54:52, stip (damaged and oooish) wrote: > 80 argh, I've become dependent on having the linter in the upload checks. https://codereview.chromium.org/1692253003/diff/20001/scripts/slave/recipe_mo... scripts/slave/recipe_modules/chromium_tests/steps.py:1601: compile_targets = [suite_defaults['compile_target']] On 2016/03/01 15:54:52, stip (damaged and oooish) wrote: > this will raise an exception if the target isn't in the map. maybe that's what > we want? That's exactly what I want. I don't think there's a good way for us to recover if we can't figure out what to build.
jbudorick@chromium.org changed reviewers: + sergeyberezin@chromium.org
+sergeyberezin for PST OWNERS
LGTM
The CQ bit was checked by jbudorick@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from stip@chromium.org Link to the patchset: https://codereview.chromium.org/1692253003/#ps40001 (title: "stip comments")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1692253003/40001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1692253003/40001
Message was sent while issue was closed.
Description was changed from ========== [Android] Add compile target -> old name map for instrumentation tests. This also reverts commit 97e7b89174c495ca53d07c60ec1dd0a3b2e75e8a. BUG=525873 ========== to ========== [Android] Add compile target -> old name map for instrumentation tests. This also reverts commit 97e7b89174c495ca53d07c60ec1dd0a3b2e75e8a. BUG=525873 Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=299050 ==========
Message was sent while issue was closed.
Committed patchset #3 (id:40001) as http://src.chromium.org/viewvc/chrome?view=rev&revision=299050 |
