|
|
Created:
6 years, 8 months ago by nodir Modified:
6 years, 7 months ago CC:
chromium-reviews Base URL:
https://chromium.googlesource.com/chromium/src.git@master Visibility:
Public. |
DescriptionAdded arm test spec.
Will be used by chromium_trybot_arm recipe.
R=mseaborn@chromium.org, phajdan.jr@chromium.org
BUG=359338
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=269655
Patch Set 1 #
Total comments: 2
Patch Set 2 : renamed the file to chromium_arm.json #
Total comments: 1
Patch Set 3 : added a description and removed trailing commands because json.load couldn't parse them #
Total comments: 2
Patch Set 4 : updated description #Messages
Total messages: 19 (0 generated)
As said before, I don't recommend this solution. Why not just ensure that only tests you're interested in get compiled? I'm really confused how running on arm would imply only running tests having NaCl in their name.
I thought the plan was to first get the ARM Chromium buildbots/trybots using isolate (which involved using recipes), running all the tests? And then only after that we'd look into how to get them to run a subset of tests?
On 2014/04/24 10:59:42, Paweł Hajdan Jr. wrote: > As said before, I don't recommend this solution. Why not just ensure that only > tests you're interested in get compiled? I assumed we are in agreement that getting rid of test-filter in chromium tests is a separate task and involves not only browser tests. If you find the concept of test filter wrong, please replace it with something better. > I'm really confused how running on arm would imply only running tests having > NaCl in their name. AFAIU, this is what was requested in the bug https://code.google.com/p/chromium/issues/detail?id=359338
On 2014/04/24 15:22:44, Mark Seaborn wrote: > I thought the plan was to first get the ARM Chromium buildbots/trybots using > isolate (which involved using recipes), running all the tests? Yes, and this doesn't contradict with this file. The recipe I am adapting for arm is chromium_trybot.py which reads chromium_trybot_arm.json: https://code.google.com/p/chromium/codesearch#chromium/tools/build/scripts/sl... I am modifying it so it reads chromium_trybot_arm.json when running on linux_arm_cross_compile builder. Also I supply test_isoliation_mode=archive, so the tests will be automatically archived and uploaded to the isolate server. > And then only after that we'd look into how to get them to run a subset of > tests? This part is already implemented in chromium_trybot.py so I am just reusing this functionality.
On 2014/04/24 17:21:58, nodir wrote: > On 2014/04/24 15:22:44, Mark Seaborn wrote: > > I thought the plan was to first get the ARM Chromium buildbots/trybots using > > isolate (which involved using recipes), running all the tests? > > Yes, and this doesn't contradict with this file. The recipe I am adapting for > arm is chromium_trybot.py which reads chromium_trybot_arm.json: > https://code.google.com/p/chromium/codesearch#chromium/tools/build/scripts/sl... > > I am modifying it so it reads chromium_trybot_arm.json when running on > linux_arm_cross_compile builder. Also I supply test_isoliation_mode=archive, so > the tests will be automatically archived and uploaded to the isolate server. OK, if the infrastructure already exists to allow the test subset to be defined in the Chromium repo, that sounds OK then. https://codereview.chromium.org/246623009/diff/1/testing/buildbot/chromium_tr... File testing/buildbot/chromium_trybot_arm.json (right): https://codereview.chromium.org/246623009/diff/1/testing/buildbot/chromium_tr... testing/buildbot/chromium_trybot_arm.json:1: [ Presumably this shouldn't have "trybot" in the name, if we want the same subset of tests to be used on the ARM buildbots too? https://codereview.chromium.org/246623009/diff/1/testing/buildbot/chromium_tr... testing/buildbot/chromium_trybot_arm.json:4: "args": "--gtest-filter='*NaCl*'", Can you add a comment explaining why we want this subset of tests on ARM, please?
On 2014/04/24 17:18:24, nodir wrote: > On 2014/04/24 10:59:42, Paweł Hajdan Jr. wrote: > > As said before, I don't recommend this solution. Why not just ensure that only > > tests you're interested in get compiled? > > I assumed we are in agreement that getting rid of test-filter in chromium tests > is a separate task and involves not only browser tests. If you find the concept > of test filter wrong, please replace it with something better. Right, so I think I've mentioned at least some of these before. 1) Why not run all of browser_tests? 2) Why not only compile what needs to run? 3) Why not provide a command-line flag (which you can also do from the .json files) that indicates arm environment, and then have the *launcher* understand that flag? > > I'm really confused how running on arm would imply only running tests having > > NaCl in their name. > > AFAIU, this is what was requested in the bug > https://code.google.com/p/chromium/issues/detail?id=359338 Please correct me if I'm wrong, but my interpretation is running at least these tests. I don't see an indication that no other tests should be run.
On 25 April 2014 06:38, <phajdan.jr@chromium.org> wrote: > On 2014/04/24 17:18:24, nodir wrote: > >> On 2014/04/24 10:59:42, Paweł Hajdan Jr. wrote: >> > As said before, I don't recommend this solution. Why not just ensure >> that only > > > tests you're interested in get compiled? >> > > I assumed we are in agreement that getting rid of test-filter in chromium >> tests > > is a separate task and involves not only browser tests. If you find the >> concept > > of test filter wrong, please replace it with something better. >> > > Right, so I think I've mentioned at least some of these before. > > 1) Why not run all of browser_tests? > Because ARM hardware is fairly slow, and we want these ARM bots to have a reasonable cycle time. We particularly want to run tests that cover features containing a lot of architecture-specific code, such as the NaCl sandboxes. > 2) Why not only compile what needs to run? > That seems like it would be more complicated. Wouldn't that involve a lot of Gyp hackery? Cross-compile time is not the issue at the moment because the compile runs on a fast x86 machine, and we can do incremental builds, unlike for testing. So we don't need to reduce the compile time for browser_tests particularly. > 3) Why not provide a command-line flag (which you can also do from the > .json > files) that indicates arm environment, and then have the *launcher* > understand > that flag? That also seems like it would be more complicated. What would be the pros and cons of using an approach other than using "--gtest_filter"? Cheers, Mark To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
PTAL
LGTM https://codereview.chromium.org/246623009/diff/20001/testing/buildbot/chromiu... File testing/buildbot/chromium_arm.json (right): https://codereview.chromium.org/246623009/diff/20001/testing/buildbot/chromiu... testing/buildbot/chromium_arm.json:4: "args": "--gtest-filter='*NaCl*'", Can you add a comment saying why we want to run this subset of tests? Since this is a JSON file, you might need to add a "comment" field instead of a real comment.
The CQ bit was checked by nodir@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/nodir@chromium.org/246623009/40001
https://codereview.chromium.org/246623009/diff/40001/testing/buildbot/chromiu... File testing/buildbot/chromium_arm.json (right): https://codereview.chromium.org/246623009/diff/40001/testing/buildbot/chromiu... testing/buildbot/chromium_arm.json:3: "description": ["These tests will be run on an ARM device, which is farily", "fairly" https://codereview.chromium.org/246623009/diff/40001/testing/buildbot/chromiu... testing/buildbot/chromium_arm.json:4: "slow, that's why we run a much smaller set of tests"], Maybe also say that we run NaCl tests specifically because NaCl contains a lot of architecture-specific code?
Done
BTW, to clarify, does this also turn off all test suites aside from browser_tests? There are some test suites we particularly want (such as sandbox unit tests), though we can add those back once the ARM bots are working reliably.
On 2014/05/09 22:05:46, Mark Seaborn wrote: > BTW, to clarify, does this also turn off all test suites aside from > browser_tests? Yes > There are some test suites we particularly want (such as sandbox unit tests), > though we can add those back once the ARM bots are working reliably. Yes. You will be able to add them later.
The CQ bit was checked by nodir@chromium.org
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/nodir@chromium.org/246623009/60001
Message was sent while issue was closed.
Change committed as 269655 |