Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
6544018:
kernel_BootMessagesServer: initial add of new test (Closed)
|
Created:
9 years, 10 months ago by Mandeep Singh Baines Modified:
9 years, 7 months ago CC:
chromium-os-reviews_chromium.org, truty+cc_chromium.org, sosa+cc_chromium.org, seano+cc_chromium.org, ericli, petkov+cc_chromium.org Visibility:
Public. |
Descriptionkernel_BootMessagesServer: initial add of new test
This tests verifies that there are no new kernel dmesg warnings.
BUG=12280
TEST=Verified PASS and FAIL.
Change-Id: Iff2b1f6de7ec435928170393ec85e7b8b9201b23
Committed: http://chrome-svn/viewvc/chromeos?view=rev&revision=142ac8d
Patch Set 1 #
Total comments: 9
Patch Set 2 : Fix per review. #Patch Set 3 : update copyright #Patch Set 4 : fix copyright #Patch Set 5 : Fix per review. #Patch Set 6 : Fix spacing. #
Messages
Total messages: 6 (0 generated)
Did you consider making the dmesg parser piece into a common library by itself? That way any test can use it to find out if there were any dmesg errors/warnings while that test ran. And, if we want to be more ambitious, have it run for all chromeos tests automatically :) http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... File server/site_tests/kernel_BootMessagesServer/control (right): http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... server/site_tests/kernel_BootMessagesServer/control:1: # Copyright (c) 2009 The Chromium OS Authors. All rights reserved. 2011? http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... File server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py (right): http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:1: # Copyright (c) 2009 The Chromium OS Authors. All rights reserved. 2011? http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:16: "pnp 00:01: io resource (0x164e-0x164f) overlaps 0000:00:1c.0 BAR 7 (0x1000-0x1fff), disabling", > 80 chars http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:24: "EXT3-fs warning: maximal mount count reached, running e2fsck is recommended", > 80 chars http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:25: "i2c i2c-2: The new_device interface is still experimental and may change in a near future", > 80 chars http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:26: "industrialio: module is from the staging directory, the quality is unknown, you have been warned.", > 80 chars http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:27: "tsl2563: module is from the staging directory, the quality is unknown, you have been warned.", > 80 chars http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:28: ] Do you want to make the warnings whitelist platform specific? The ARM whitelist will be very different. http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:57: for line in dmesg.split('\n'): use dmesg.splitlines instead?
Fixed. PTAL. snanda@chromium.org (snanda@chromium.org) wrote: > Did you consider making the dmesg parser piece into a common library > by itself? I made it in a function. I'll need to think about how to make it common. OK, if I do this later as it is quite involved. > That way any test can use it to find out if there were any dmesg > errors/warnings > while that test ran. And, if we want to be more ambitious, have it > run for all > chromeos tests automatically :) > > > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > File server/site_tests/kernel_BootMessagesServer/control (right): > > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > server/site_tests/kernel_BootMessagesServer/control:1: # Copyright (c) > 2009 The Chromium OS Authors. All rights reserved. > 2011? > Fixed. > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > File > server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py > (right): > > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:1: > # Copyright (c) 2009 The Chromium OS Authors. All rights reserved. > 2011? > Fixed. > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:16: > "pnp 00:01: io resource (0x164e-0x164f) overlaps 0000:00:1c.0 BAR 7 > (0x1000-0x1fff), disabling", > > 80 chars > Fixed. > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:24: > "EXT3-fs warning: maximal mount count reached, running e2fsck is > recommended", > > 80 chars > Fixed. > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:25: > "i2c i2c-2: The new_device interface is still experimental and may > change in a near future", > > 80 chars > Fixed. > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:26: > "industrialio: module is from the staging directory, the quality is > unknown, you have been warned.", > > 80 chars > Fixed. > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:27: > "tsl2563: module is from the staging directory, the quality is unknown, > you have been warned.", > > 80 chars > Fixed. > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:28: > ] > Do you want to make the warnings whitelist platform specific? The ARM > whitelist will be very different. > There will be some overlap. Still trying to decide whether we should have one whitelist, one per arch, or one per platform. OK if I punt on this for now? > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:57: > for line in dmesg.split('\n'): > use dmesg.splitlines instead? > Fixed. > http://codereview.chromium.org/6544018/
On 2011/02/18 18:26:36, Mandeep Singh Baines wrote: > Fixed. PTAL. > > mailto:snanda@chromium.org (mailto:snanda@chromium.org) wrote: > > Did you consider making the dmesg parser piece into a common library > > by itself? > > I made it in a function. I'll need to think about how to make it common. > OK, if I do this later as it is quite involved. Would prefer to move it into a common lib before checking in (you may want to look at client/common_lib/site_power_status.py for a reference). But am OK with LGTMing it as is for now. Up to you. > > > That way any test can use it to find out if there were any dmesg > > errors/warnings > > while that test ran. And, if we want to be more ambitious, have it > > run for all > > chromeos tests automatically :) > > > > > > > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > > File server/site_tests/kernel_BootMessagesServer/control (right): > > > > > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > > server/site_tests/kernel_BootMessagesServer/control:1: # Copyright (c) > > 2009 The Chromium OS Authors. All rights reserved. > > 2011? > > > > Fixed. > > > > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > > File > > server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py > > (right): > > > > > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > > server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:1: > > # Copyright (c) 2009 The Chromium OS Authors. All rights reserved. > > 2011? > > > > Fixed. > > > > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > > server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:16: > > "pnp 00:01: io resource (0x164e-0x164f) overlaps 0000:00:1c.0 BAR 7 > > (0x1000-0x1fff), disabling", > > > 80 chars > > > > Fixed. > > > > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > > server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:24: > > "EXT3-fs warning: maximal mount count reached, running e2fsck is > > recommended", > > > 80 chars > > > > Fixed. > > > > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > > server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:25: > > "i2c i2c-2: The new_device interface is still experimental and may > > change in a near future", > > > 80 chars > > > > Fixed. > > > > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > > server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:26: > > "industrialio: module is from the staging directory, the quality is > > unknown, you have been warned.", > > > 80 chars > > > > Fixed. > > > > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > > server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:27: > > "tsl2563: module is from the staging directory, the quality is unknown, > > you have been warned.", > > > 80 chars > > > > Fixed. > > > > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > > server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:28: > > ] > > Do you want to make the warnings whitelist platform specific? The ARM > > whitelist will be very different. > > > > There will be some overlap. Still trying to decide whether we should have > one whitelist, one per arch, or one per platform. OK if I punt on this for now? > > > > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > > server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:57: > > for line in dmesg.split('\n'): > > use dmesg.splitlines instead? > > > > Fixed. > > > http://codereview.chromium.org/6544018/
Fixed. PTAL. snanda@chromium.org (snanda@chromium.org) wrote: > On 2011/02/18 18:26:36, Mandeep Singh Baines wrote: > >Fixed. PTAL. > > >mailto:snanda@chromium.org (mailto:snanda@chromium.org) wrote: > >> Did you consider making the dmesg parser piece into a common library > >> by itself? > > >I made it in a function. I'll need to think about how to make it common. > >OK, if I do this later as it is quite involved. > > Would prefer to move it into a common lib before checking in (you > may want to > look at client/common_lib/site_power_status.py for a reference). > But am OK with > LGTMing it as is for now. Up to you. > > > >> That way any test can use it to find out if there were any dmesg > >> errors/warnings > >> while that test ran. And, if we want to be more ambitious, have it > >> run for all > >> chromeos tests automatically :) > >> > >> > >> > > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > >> File server/site_tests/kernel_BootMessagesServer/control (right): > >> > >> > > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > >> server/site_tests/kernel_BootMessagesServer/control:1: # Copyright (c) > >> 2009 The Chromium OS Authors. All rights reserved. > >> 2011? > >> > > >Fixed. > > >> > > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > >> File > >> server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py > >> (right): > >> > >> > > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > >> server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:1: > >> # Copyright (c) 2009 The Chromium OS Authors. All rights reserved. > >> 2011? > >> > > >Fixed. > > >> > > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > >> server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:16: > >> "pnp 00:01: io resource (0x164e-0x164f) overlaps 0000:00:1c.0 BAR 7 > >> (0x1000-0x1fff), disabling", > >> > 80 chars > >> > > >Fixed. > > >> > > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > >> server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:24: > >> "EXT3-fs warning: maximal mount count reached, running e2fsck is > >> recommended", > >> > 80 chars > >> > > >Fixed. > > >> > > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > >> server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:25: > >> "i2c i2c-2: The new_device interface is still experimental and may > >> change in a near future", > >> > 80 chars > >> > > >Fixed. > > >> > > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > >> server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:26: > >> "industrialio: module is from the staging directory, the quality is > >> unknown, you have been warned.", > >> > 80 chars > >> > > >Fixed. > > >> > > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > >> server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:27: > >> "tsl2563: module is from the staging directory, the quality is unknown, > >> you have been warned.", > >> > 80 chars > >> > > >Fixed. > > >> > > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > >> server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:28: > >> ] > >> Do you want to make the warnings whitelist platform specific? The ARM > >> whitelist will be very different. > >> > > >There will be some overlap. Still trying to decide whether we should have > >one whitelist, one per arch, or one per platform. OK if I punt on this for > now? > > >> > > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > >> server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:57: > >> for line in dmesg.split('\n'): > >> use dmesg.splitlines instead? > >> > > >Fixed. > > >> http://codereview.chromium.org/6544018/ > > > > http://codereview.chromium.org/6544018/
LGTM. Thanks! On 2011/02/18 19:05:53, Mandeep Singh Baines wrote: > Fixed. PTAL. > > mailto:snanda@chromium.org (mailto:snanda@chromium.org) wrote: > > On 2011/02/18 18:26:36, Mandeep Singh Baines wrote: > > >Fixed. PTAL. > > > > >mailto:snanda@chromium.org (mailto:snanda@chromium.org) wrote: > > >> Did you consider making the dmesg parser piece into a common library > > >> by itself? > > > > >I made it in a function. I'll need to think about how to make it common. > > >OK, if I do this later as it is quite involved. > > > > Would prefer to move it into a common lib before checking in (you > > may want to > > look at client/common_lib/site_power_status.py for a reference). > > But am OK with > > LGTMing it as is for now. Up to you. > > > > > > >> That way any test can use it to find out if there were any dmesg > > >> errors/warnings > > >> while that test ran. And, if we want to be more ambitious, have it > > >> run for all > > >> chromeos tests automatically :) > > >> > > >> > > >> > > > > > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > > >> File server/site_tests/kernel_BootMessagesServer/control (right): > > >> > > >> > > > > > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > > >> server/site_tests/kernel_BootMessagesServer/control:1: # Copyright (c) > > >> 2009 The Chromium OS Authors. All rights reserved. > > >> 2011? > > >> > > > > >Fixed. > > > > >> > > > > > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > > >> File > > >> server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py > > >> (right): > > >> > > >> > > > > > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > > >> server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:1: > > >> # Copyright (c) 2009 The Chromium OS Authors. All rights reserved. > > >> 2011? > > >> > > > > >Fixed. > > > > >> > > > > > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > > >> > server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:16: > > >> "pnp 00:01: io resource (0x164e-0x164f) overlaps 0000:00:1c.0 BAR 7 > > >> (0x1000-0x1fff), disabling", > > >> > 80 chars > > >> > > > > >Fixed. > > > > >> > > > > > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > > >> > server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:24: > > >> "EXT3-fs warning: maximal mount count reached, running e2fsck is > > >> recommended", > > >> > 80 chars > > >> > > > > >Fixed. > > > > >> > > > > > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > > >> > server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:25: > > >> "i2c i2c-2: The new_device interface is still experimental and may > > >> change in a near future", > > >> > 80 chars > > >> > > > > >Fixed. > > > > >> > > > > > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > > >> > server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:26: > > >> "industrialio: module is from the staging directory, the quality is > > >> unknown, you have been warned.", > > >> > 80 chars > > >> > > > > >Fixed. > > > > >> > > > > > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > > >> > server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:27: > > >> "tsl2563: module is from the staging directory, the quality is unknown, > > >> you have been warned.", > > >> > 80 chars > > >> > > > > >Fixed. > > > > >> > > > > > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > > >> > server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:28: > > >> ] > > >> Do you want to make the warnings whitelist platform specific? The ARM > > >> whitelist will be very different. > > >> > > > > >There will be some overlap. Still trying to decide whether we should have > > >one whitelist, one per arch, or one per platform. OK if I punt on this for > > now? > > > > >> > > > > > http://codereview.chromium.org/6544018/diff/1/server/site_tests/kernel_BootMe... > > >> > server/site_tests/kernel_BootMessagesServer/kernel_BootMessagesServer.py:57: > > >> for line in dmesg.split('\n'): > > >> use dmesg.splitlines instead? > > >> > > > > >Fixed. > > > > >> http://codereview.chromium.org/6544018/ > > > > > > > > http://codereview.chromium.org/6544018/ |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
