Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
588903002:
Add extensions_unittests suite to more memory bots. (Closed)
DescriptionAdd extensions_unittests suite to more memory bots.
BUG=348066
TEST=None
R=bruening@chromium.org,dpranke@chromium.org,jamescook@chromium.org
Committed: https://crrev.com/fef6d69e61888ebe0b4c763d47931b0dfbe7840f
Cr-Commit-Position: refs/heads/master@{#296461}
Patch Set 1 #
Total comments: 1
Patch Set 2 : rm from LSan for now #
Total comments: 1
Messages
Total messages: 18 (2 generated)
Derek, please, make sure this is fine. Thanks!
lgtm
LGTM assuming these tests have been tested under asan and lsan to ensure no problems.
Derek, could you try locally for me? I haven't test it under asan or lsan. I don't know how to setup that. And it will probably take more time than I have, since I don't own a fast machine with lots of RAM like a z620 you guys have or access to goma. :(
On 2014/09/21 17:04:58, tfarina wrote: > Derek, could you try locally for me? > > I haven't test it under asan or lsan. I don't know how to setup that. And it > will probably take more time than I have, since I don't own a fast machine with > lots of RAM like a z620 you guys have or access to goma. :( I just tried it, extensions_unittests passes under Asan on Linux64. However, there are a large number of leaks reported by LSan, mostly things in Mojo code. I recommend adding to Asan now and I can file bugs on the LSan issues.
tfarina@chromium.org changed reviewers: + jamescook@chromium.org
James, I removed it from chromium.memory.json. Was that what you were suggesting? If so, please push to CQ! Thanks,
dpranke, question? https://codereview.chromium.org/588903002/diff/1/testing/buildbot/chromium_me... File testing/buildbot/chromium_memory_trybot.json (right): https://codereview.chromium.org/588903002/diff/1/testing/buildbot/chromium_me... testing/buildbot/chromium_memory_trybot.json:50: "platforms": ["linux", "win", "mac"], dpranke, does he need this list? extensions_unittests can also run on Chrome OS, but not Android or iOS. I'm guessing a blank platforms list == linux, mac, chromeos, maybe win?
On 2014/09/23 16:16:38, James Cook wrote: > dpranke, question? > > https://codereview.chromium.org/588903002/diff/1/testing/buildbot/chromium_me... > File testing/buildbot/chromium_memory_trybot.json (right): > > https://codereview.chromium.org/588903002/diff/1/testing/buildbot/chromium_me... > testing/buildbot/chromium_memory_trybot.json:50: "platforms": ["linux", "win", > "mac"], > dpranke, does he need this list? extensions_unittests can also run on Chrome > OS, but not Android or iOS. I'm guessing a blank platforms list == linux, mac, > chromeos, maybe win? From what I can see from looking at the code, if 'platforms' is not specified, we'll try to run it everywhere, so it would get picked up on Android and iOS, yes. If you want it to run on chromeos, I think you need to specify it explicitly; linux won't be enough.
On 2014/09/23 20:05:49, Dirk Pranke wrote: > On 2014/09/23 16:16:38, James Cook wrote: > > dpranke, question? > > > > > https://codereview.chromium.org/588903002/diff/1/testing/buildbot/chromium_me... > > File testing/buildbot/chromium_memory_trybot.json (right): > > > > > https://codereview.chromium.org/588903002/diff/1/testing/buildbot/chromium_me... > > testing/buildbot/chromium_memory_trybot.json:50: "platforms": ["linux", "win", > > "mac"], > > dpranke, does he need this list? extensions_unittests can also run on Chrome > > OS, but not Android or iOS. I'm guessing a blank platforms list == linux, > mac, > > chromeos, maybe win? > > From what I can see from looking at the code, if 'platforms' is not specified, > we'll try to run it everywhere, so it would get picked up on Android and iOS, > yes. > > If you want it to run on chromeos, I think you need to specify it explicitly; > linux won't be enough. Do you know if the platform for chromeos is "chromeos" or "chromiumos"? I don't see any other examples of a chromeos platform filter in that directory and I'm not sure where the script that reads that data lives.
On 2014/09/23 21:58:03, James Cook OOO until 9-29 wrote: > On 2014/09/23 20:05:49, Dirk Pranke wrote: > > On 2014/09/23 16:16:38, James Cook wrote: > > > dpranke, question? > > > > > > > > > https://codereview.chromium.org/588903002/diff/1/testing/buildbot/chromium_me... > > > File testing/buildbot/chromium_memory_trybot.json (right): > > > > > > > > > https://codereview.chromium.org/588903002/diff/1/testing/buildbot/chromium_me... > > > testing/buildbot/chromium_memory_trybot.json:50: "platforms": ["linux", > "win", > > > "mac"], > > > dpranke, does he need this list? extensions_unittests can also run on > Chrome > > > OS, but not Android or iOS. I'm guessing a blank platforms list == linux, > > mac, > > > chromeos, maybe win? > > > > From what I can see from looking at the code, if 'platforms' is not specified, > > we'll try to run it everywhere, so it would get picked up on Android and iOS, > > yes. > > > > If you want it to run on chromeos, I think you need to specify it explicitly; > > linux won't be enough. > > Do you know if the platform for chromeos is "chromeos" or "chromiumos"? I don't > see any other examples of a chromeos platform filter in that directory and I'm > not sure where the script that reads that data lives. I believe it is "chromeos". I think the values are being checked in: https://code.google.com/p/chromium/codesearch?q=chromium_modules%2Fconfig.py#... phajdan.jr can correct me if I'm wrong ...
On 2014/09/23 23:14:58, Dirk Pranke wrote: > On 2014/09/23 21:58:03, James Cook OOO until 9-29 wrote: > > On 2014/09/23 20:05:49, Dirk Pranke wrote: > > > On 2014/09/23 16:16:38, James Cook wrote: > > > > dpranke, question? > > > > > > > > > > > > > > https://codereview.chromium.org/588903002/diff/1/testing/buildbot/chromium_me... > > > > File testing/buildbot/chromium_memory_trybot.json (right): > > > > > > > > > > > > > > https://codereview.chromium.org/588903002/diff/1/testing/buildbot/chromium_me... > > > > testing/buildbot/chromium_memory_trybot.json:50: "platforms": ["linux", > > "win", > > > > "mac"], > > > > dpranke, does he need this list? extensions_unittests can also run on > > Chrome > > > > OS, but not Android or iOS. I'm guessing a blank platforms list == linux, > > > mac, > > > > chromeos, maybe win? > > > > > > From what I can see from looking at the code, if 'platforms' is not > specified, > > > we'll try to run it everywhere, so it would get picked up on Android and > iOS, > > > yes. > > > > > > If you want it to run on chromeos, I think you need to specify it > explicitly; > > > linux won't be enough. > > > > Do you know if the platform for chromeos is "chromeos" or "chromiumos"? I > don't > > see any other examples of a chromeos platform filter in that directory and I'm > > not sure where the script that reads that data lives. > > I believe it is "chromeos". I think the values are being checked in: > > https://code.google.com/p/chromium/codesearch?q=chromium_modules%2Fconfig.py#... > > phajdan.jr can correct me if I'm wrong ... When I changed this last, "linux" included chromeos; I had to add another key ("chromium_configs") to allow for just chromeos configurations.
https://codereview.chromium.org/588903002/diff/20001/testing/buildbot/chromiu... File testing/buildbot/chromium_memory_trybot.json (right): https://codereview.chromium.org/588903002/diff/20001/testing/buildbot/chromiu... testing/buildbot/chromium_memory_trybot.json:50: "platforms": ["linux", "win", "mac"], I think Pawel's message here -> https://codereview.chromium.org/257873004/#msg4, clarifies this.
lgtm
The CQ bit was checked by tfarina@chromium.org
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/588903002/20001
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as 4252b7e22c6a86afc956d1f453877f08b0d32cf5
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/fef6d69e61888ebe0b4c763d47931b0dfbe7840f Cr-Commit-Position: refs/heads/master@{#296461} |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
