Make test failure messages more verbose (and thus useful to testing operators).

BUG=chrome-os-partner:505

[Hung-Te, 2010-08-04 13:00:47]

Hey, tammo - do you think we are really switching to zh-CN (Simplified Chinese)?
Is it time to support multiple localizations?

Also I'm wondering if it would be a good idea to keep using Traditional Chinese
in source code, and use some translator programs (eg, the one Wikipedia uses) to
allow UI changing UI language on-the-fly.

http://codereview.chromium.org/2819079/diff/7001/8001
File client/site_tests/factory_Camera/factory_Camera.py (right):

http://codereview.chromium.org/2819079/diff/7001/8001#newcode191
client/site_tests/factory_Camera/factory_Camera.py:191: raise
error.TestFail('品管人員懷疑攝影鏡頭故障，請檢修')
We can't have only Chinese messages - you should always keep an English version
description.

Also - I thought this ticket wants to use Simplified Chinese?

http://codereview.chromium.org/2819079/diff/7001/8001#newcode193
client/site_tests/factory_Camera/factory_Camera.py:193: raise
error.TestFail('LED 測試不通過，請檢修')
same as above. Also I think you can hint that this LED is Camera LED (since
there's another "LED Test" which refers to side-LEDs)

http://codereview.chromium.org/2819079/diff/7001/8003
File client/site_tests/factory_Display/factory_Display.py (right):

http://codereview.chromium.org/2819079/diff/7001/8003#newcode187
client/site_tests/factory_Display/factory_Display.py:187: raise
error.TestFail('以下圖樣測試未通過: %s' %
Please also keep English messages

http://codereview.chromium.org/2819079/diff/7001/8005
File client/site_tests/factory_ExternalStorage/factory_ExternalStorage.py
(right):

http://codereview.chromium.org/2819079/diff/7001/8005#newcode97
client/site_tests/factory_ExternalStorage/factory_ExternalStorage.py:97: if
len(diff) > 1 or self._target_device not in diff:
logic also changed here - is this a bug fix?

http://codereview.chromium.org/2819079/diff/7001/8006
File client/site_tests/factory_Keyboard/factory_Keyboard.py (right):

http://codereview.chromium.org/2819079/diff/7001/8006#newcode48
client/site_tests/factory_Keyboard/factory_Keyboard.py:48: return
'沒有偵測到下列按鍵，鍵盤可能故障，請檢修: %s' % ', '.join(missing_keys)
we also need English messages :)

http://codereview.chromium.org/2819079/diff/7001/8007
File client/site_tests/factory_Leds/factory_Leds.py (right):

http://codereview.chromium.org/2819079/diff/7001/8007#newcode211
client/site_tests/factory_Leds/factory_Leds.py:211: raise
error.TestFail('以下圖樣測試未通過: %s' %
please also provide English messages

http://codereview.chromium.org/2819079/diff/7001/8008
File client/site_tests/factory_Touchpad/factory_Touchpad.py (right):

http://codereview.chromium.org/2819079/diff/7001/8008#newcode80
client/site_tests/factory_Touchpad/factory_Touchpad.py:80:
missing.append('沒有偵測到下列 motion sectors 被觸及，請檢修 [%s]' %
Please add English messages

http://codereview.chromium.org/2819079/diff/7001/8008#newcode85
client/site_tests/factory_Touchpad/factory_Touchpad.py:85:
missing.append('沒有偵測到下列 scroll segments 被觸及，請檢修 [%s]' %
Please add English messages

http://codereview.chromium.org/2819079/diff/7001/8009
File client/site_tests/suite_Factory/control (right):

http://codereview.chromium.org/2819079/diff/7001/8009#newcode117
client/site_tests/suite_Factory/control:117: label_zw='侧面发光二极管',
As I remember, the label on screen cannot take words longer than 4
wide-characters, so this may exceed display...

http://codereview.chromium.org/2819079/diff/7001/8009#newcode134
client/site_tests/suite_Factory/control:134: label_zw='扬声器',
The test here includes both speaker and headsets, while 扬声器 usually refers to
headset only? (I'm not sure if this applies to Simplified Chinese)

http://codereview.chromium.org/2819079/diff/7001/8009#newcode174
client/site_tests/suite_Factory/control:174: label_zw='开发人员恢复模式',
As I remember, the label on screen cannot take words longer than 4
wide-characters, so this may exceed display...

[clchiou, 2010-08-04 18:14:00]

On 2010/08/04 13:00:47, Hung-Te wrote:
> Hey, tammo - do you think we are really switching to zh-CN (Simplified
Chinese)?

I am sorry that I didn't update the issue tracker, Hung-Te.  It was
my misunderstanding that I was assigned to translate all messages into
simplified Chinese.  In fact, this issue is asking factory tests to
provide verbose messages when tests fail and let factory assembly line
operators decide further actions.

This is why messages are mixed with simplified and tradition Chinese.
It was my bad.

At this point, in my opinion, providing internationalization is not on
our high priority, but would be nice to have since many OEMs have factories
in east Europe and Mexico.  On the other hand, I think you are right that
I should not delete English messages since not every developer can read
Chinese.

So in short, I am now putting back English messages but I will leave the
mixed Chinese situation unchanged.

> Is it time to support multiple localizations?
> 
> Also I'm wondering if it would be a good idea to keep using Traditional
Chinese
> in source code, and use some translator programs (eg, the one Wikipedia uses)
to
> allow UI changing UI language on-the-fly.
> 
> http://codereview.chromium.org/2819079/diff/7001/8001
> File client/site_tests/factory_Camera/factory_Camera.py (right):
> 
> http://codereview.chromium.org/2819079/diff/7001/8001#newcode191
> client/site_tests/factory_Camera/factory_Camera.py:191: raise
> error.TestFail('品管人員懷疑攝影鏡頭故障，請檢修')
> We can't have only Chinese messages - you should always keep an English
version
> description.
> 
> Also - I thought this ticket wants to use Simplified Chinese?
> 
> http://codereview.chromium.org/2819079/diff/7001/8001#newcode193
> client/site_tests/factory_Camera/factory_Camera.py:193: raise
> error.TestFail('LED 測試不通過，請檢修')
> same as above. Also I think you can hint that this LED is Camera LED (since
> there's another "LED Test" which refers to side-LEDs)
> 
> http://codereview.chromium.org/2819079/diff/7001/8003
> File client/site_tests/factory_Display/factory_Display.py (right):
> 
> http://codereview.chromium.org/2819079/diff/7001/8003#newcode187
> client/site_tests/factory_Display/factory_Display.py:187: raise
> error.TestFail('以下圖樣測試未通過: %s' %
> Please also keep English messages
> 
> http://codereview.chromium.org/2819079/diff/7001/8005
> File client/site_tests/factory_ExternalStorage/factory_ExternalStorage.py
> (right):
> 
> http://codereview.chromium.org/2819079/diff/7001/8005#newcode97
> client/site_tests/factory_ExternalStorage/factory_ExternalStorage.py:97: if
> len(diff) > 1 or self._target_device not in diff:
> logic also changed here - is this a bug fix?
> 
> http://codereview.chromium.org/2819079/diff/7001/8006
> File client/site_tests/factory_Keyboard/factory_Keyboard.py (right):
> 
> http://codereview.chromium.org/2819079/diff/7001/8006#newcode48
> client/site_tests/factory_Keyboard/factory_Keyboard.py:48: return
> '沒有偵測到下列按鍵，鍵盤可能故障，請檢修: %s' % ', '.join(missing_keys)
> we also need English messages :)
> 
> http://codereview.chromium.org/2819079/diff/7001/8007
> File client/site_tests/factory_Leds/factory_Leds.py (right):
> 
> http://codereview.chromium.org/2819079/diff/7001/8007#newcode211
> client/site_tests/factory_Leds/factory_Leds.py:211: raise
> error.TestFail('以下圖樣測試未通過: %s' %
> please also provide English messages
> 
> http://codereview.chromium.org/2819079/diff/7001/8008
> File client/site_tests/factory_Touchpad/factory_Touchpad.py (right):
> 
> http://codereview.chromium.org/2819079/diff/7001/8008#newcode80
> client/site_tests/factory_Touchpad/factory_Touchpad.py:80:
> missing.append('沒有偵測到下列 motion sectors 被觸及，請檢修 [%s]' %
> Please add English messages
> 
> http://codereview.chromium.org/2819079/diff/7001/8008#newcode85
> client/site_tests/factory_Touchpad/factory_Touchpad.py:85:
> missing.append('沒有偵測到下列 scroll segments 被觸及，請檢修 [%s]' %
> Please add English messages
> 
> http://codereview.chromium.org/2819079/diff/7001/8009
> File client/site_tests/suite_Factory/control (right):
> 
> http://codereview.chromium.org/2819079/diff/7001/8009#newcode117
> client/site_tests/suite_Factory/control:117: label_zw='侧面发光二极管',
> As I remember, the label on screen cannot take words longer than 4
> wide-characters, so this may exceed display...
> 
> http://codereview.chromium.org/2819079/diff/7001/8009#newcode134
> client/site_tests/suite_Factory/control:134: label_zw='扬声器',
> The test here includes both speaker and headsets, while 扬声器 usually refers to
> headset only? (I'm not sure if this applies to Simplified Chinese)
> 
> http://codereview.chromium.org/2819079/diff/7001/8009#newcode174
> client/site_tests/suite_Factory/control:174: label_zw='开发人员恢复模式',
> As I remember, the label on screen cannot take words longer than 4
> wide-characters, so this may exceed display...

[clchiou, 2010-08-04 20:27:42]

Hi Hung-Te,

I've addressed your comments.
Thanks for reviewing.

http://codereview.chromium.org/2819079/diff/7001/8001
File client/site_tests/factory_Camera/factory_Camera.py (right):

http://codereview.chromium.org/2819079/diff/7001/8001#newcode191
client/site_tests/factory_Camera/factory_Camera.py:191: raise
error.TestFail('品管人員懷疑攝影鏡頭故障，請檢修')
Done.

http://codereview.chromium.org/2819079/diff/7001/8001#newcode193
client/site_tests/factory_Camera/factory_Camera.py:193: raise
error.TestFail('LED 測試不通過，請檢修')
Done.

http://codereview.chromium.org/2819079/diff/7001/8003
File client/site_tests/factory_Display/factory_Display.py (right):

http://codereview.chromium.org/2819079/diff/7001/8003#newcode187
client/site_tests/factory_Display/factory_Display.py:187: raise
error.TestFail('以下圖樣測試未通過: %s' %
Done.

http://codereview.chromium.org/2819079/diff/7001/8005
File client/site_tests/factory_ExternalStorage/factory_ExternalStorage.py
(right):

http://codereview.chromium.org/2819079/diff/7001/8005#newcode97
client/site_tests/factory_ExternalStorage/factory_ExternalStorage.py:97: if
len(diff) > 1 or self._target_device not in diff:
I rewrite this piece of code for clarifying our intention here.

On 2010/08/04 13:00:47, Hung-Te wrote:
> logic also changed here - is this a bug fix?

http://codereview.chromium.org/2819079/diff/7001/8006
File client/site_tests/factory_Keyboard/factory_Keyboard.py (right):

http://codereview.chromium.org/2819079/diff/7001/8006#newcode48
client/site_tests/factory_Keyboard/factory_Keyboard.py:48: return
'沒有偵測到下列按鍵，鍵盤可能故障，請檢修: %s' % ', '.join(missing_keys)
Done.

http://codereview.chromium.org/2819079/diff/7001/8007
File client/site_tests/factory_Leds/factory_Leds.py (right):

http://codereview.chromium.org/2819079/diff/7001/8007#newcode211
client/site_tests/factory_Leds/factory_Leds.py:211: raise
error.TestFail('以下圖樣測試未通過: %s' %
Done.

http://codereview.chromium.org/2819079/diff/7001/8008
File client/site_tests/factory_Touchpad/factory_Touchpad.py (right):

http://codereview.chromium.org/2819079/diff/7001/8008#newcode80
client/site_tests/factory_Touchpad/factory_Touchpad.py:80:
missing.append('沒有偵測到下列 motion sectors 被觸及，請檢修 [%s]' %
Done.

http://codereview.chromium.org/2819079/diff/7001/8008#newcode85
client/site_tests/factory_Touchpad/factory_Touchpad.py:85:
missing.append('沒有偵測到下列 scroll segments 被觸及，請檢修 [%s]' %
Done.

http://codereview.chromium.org/2819079/diff/7001/8008#newcode88
client/site_tests/factory_Touchpad/factory_Touchpad.py:88:
missing.append('沒有偵測到左鍵被按下，請檢修')
Added English message, too.

http://codereview.chromium.org/2819079/diff/7001/8009
File client/site_tests/suite_Factory/control (right):

http://codereview.chromium.org/2819079/diff/7001/8009#newcode117
client/site_tests/suite_Factory/control:117: label_zw='侧面发光二极管',
Shortened.

http://codereview.chromium.org/2819079/diff/7001/8009#newcode134
client/site_tests/suite_Factory/control:134: label_zw='扬声器',
I googled images for 扬声器, which showed speakers, so I thought this was a proper
translation.  I also tried Bing, and the result was the same.

http://codereview.chromium.org/2819079/diff/7001/8009#newcode174
client/site_tests/suite_Factory/control:174: label_zw='开发人员恢复模式',
Shortened.

[Hung-Te, 2010-08-05 04:00:34]

> In fact, this issue is asking factory tests to
> provide verbose messages when tests fail and let 
> factory assembly line operators decide further actions.

Hmmm. In this case, since newly added messages are still Traditional Chinese,
why not revert the Simplified ones in suite_Factory/control back to Traditional
Chinese?

http://codereview.chromium.org/2819079/diff/7001/8009
File client/site_tests/suite_Factory/control (right):

http://codereview.chromium.org/2819079/diff/7001/8009#newcode134
client/site_tests/suite_Factory/control:134: label_zw='扬声器',
Sorry I made a typo previously - what I wanted to say is 扬声器 usually refers to
only speakers while this test includes both headsets and speakers.

Maybe a term like 音源裝置 would describe the real targets better.

http://codereview.chromium.org/2819079/diff/7001/8009#newcode174
client/site_tests/suite_Factory/control:174: label_zw='开发人员恢复模式',
恢复模式 discards the "developer button" tests. Also if you want to bypass the
developer stuff, maybe 還原 is better than 恢复?

[clchiou, 2010-08-05 21:37:47]

Hi Hung-Te,

Thanks for the timely review.
I revert suite_Factory/control and rewrite some of messages
based on your comments.

Sorry for multiple patch sets.
I am still exploring git functions.

On 2010/08/05 04:00:34, Hung-Te wrote:
> > In fact, this issue is asking factory tests to
> > provide verbose messages when tests fail and let 
> > factory assembly line operators decide further actions.
> 
> Hmmm. In this case, since newly added messages are still Traditional Chinese,
> why not revert the Simplified ones in suite_Factory/control back to
Traditional
> Chinese?
> 
> http://codereview.chromium.org/2819079/diff/7001/8009
> File client/site_tests/suite_Factory/control (right):
> 
> http://codereview.chromium.org/2819079/diff/7001/8009#newcode134
> client/site_tests/suite_Factory/control:134: label_zw='扬声器',
> Sorry I made a typo previously - what I wanted to say is 扬声器 usually refers to
> only speakers while this test includes both headsets and speakers.
> 
> Maybe a term like 音源裝置 would describe the real targets better.
> 
> http://codereview.chromium.org/2819079/diff/7001/8009#newcode174
> client/site_tests/suite_Factory/control:174: label_zw='开发人员恢复模式',
> 恢复模式 discards the "developer button" tests. Also if you want to bypass the
> developer stuff, maybe 還原 is better than 恢复?

[Tammo Spalink, 2010-08-06 04:35:56]

it looks like all the simplified chinese is back to traditional, right?

we can tackle real internationalization later, low priority for now

http://codereview.chromium.org/2819079/diff/16003/7003
File client/site_tests/factory_DeveloperRecovery/factory_DeveloperRecovery.py
(right):

http://codereview.chromium.org/2819079/diff/16003/7003#newcode280
client/site_tests/factory_DeveloperRecovery/factory_DeveloperRecovery.py:280:
'Test program does not have correspond hardware '
i suggest "test missing corresponding hardware for %s"

http://codereview.chromium.org/2819079/diff/16003/7005
File client/site_tests/factory_EnableWriteProtect/factory_EnableWriteProtect.py
(right):

http://codereview.chromium.org/2819079/diff/16003/7005#newcode54
client/site_tests/factory_EnableWriteProtect/factory_EnableWriteProtect.py:54:
raise error.TestError('ERROR: cannot select target %s' %
nothing here?

http://codereview.chromium.org/2819079/diff/16003/7006
File client/site_tests/factory_ExternalStorage/factory_ExternalStorage.py
(right):

http://codereview.chromium.org/2819079/diff/16003/7006#newcode97
client/site_tests/factory_ExternalStorage/factory_ExternalStorage.py:97: if
len(diff) > 1 or self._target_device not in diff:
was the original condition wrong?  it looks to me like this could be rolled into
the enclosing condition...

[Hung-Te, 2010-08-06 09:34:12]

http://codereview.chromium.org/2819079/diff/16003/7005
File client/site_tests/factory_EnableWriteProtect/factory_EnableWriteProtect.py
(right):

http://codereview.chromium.org/2819079/diff/16003/7005#newcode54
client/site_tests/factory_EnableWriteProtect/factory_EnableWriteProtect.py:54:
raise error.TestError('ERROR: cannot select target %s' %
Since this py file was originally by me... I can provide some candidates to
these messages. However since most of them should be internal error, I think
it's OK if you ignore and don't translate most of them, except the "ERROR:
cannot enable write protection" which is the only error that operator may get.

錯誤: 無法選取快閃記憶體目標 %s

http://codereview.chromium.org/2819079/diff/16003/7005#newcode60
client/site_tests/factory_EnableWriteProtect/factory_EnableWriteProtect.py:60:
raise error.TestError('Cannot read valid flash rom data.')
無法讀取快閃記憶體資料

http://codereview.chromium.org/2819079/diff/16003/7005#newcode64
client/site_tests/factory_EnableWriteProtect/factory_EnableWriteProtect.py:64:
raise error.TestError('Cannot detect flash rom layout.')
無法偵測快閃記憶體配置結構

http://codereview.chromium.org/2819079/diff/16003/7005#newcode70
client/site_tests/factory_EnableWriteProtect/factory_EnableWriteProtect.py:70:
raise error.TestError("INTERNAL ERROR: Must be 1 RO section")
內部錯誤: 須要單一個唯讀區段

http://codereview.chromium.org/2819079/diff/16003/7005#newcode72
client/site_tests/factory_EnableWriteProtect/factory_EnableWriteProtect.py:72:
raise error.TestError('ERROR: cannot enable write protection.')
錯誤: 無法啟用寫入保護

http://codereview.chromium.org/2819079/diff/16003/7005#newcode76
client/site_tests/factory_EnableWriteProtect/factory_EnableWriteProtect.py:76:
raise error.TestError('ERROR: cannot restore target.')
錯誤: 無法還原快閃記憶體目標

http://codereview.chromium.org/2819079/diff/16003/7008
File client/site_tests/factory_Leds/factory_Leds.py (right):

http://codereview.chromium.org/2819079/diff/16003/7008#newcode211
client/site_tests/factory_Leds/factory_Leds.py:211: raise error.TestFail('Some
patterns failed' \
seems like missing a \n here

http://codereview.chromium.org/2819079/diff/16003/7009
File client/site_tests/factory_Touchpad/factory_Touchpad.py (right):

http://codereview.chromium.org/2819079/diff/16003/7009#newcode81
client/site_tests/factory_Touchpad/factory_Touchpad.py:81: '沒有偵測到下列 motion
sectors 被觸及，請檢修 [%s]' %
According to the real UI, will this be better translated as:
未偵測到下列位置的觸控移動訊號

Not sure if this is better because I'm not sure what "motion sector" is, so you
can judge by yourself if you'd prefer to keep the English terms.

http://codereview.chromium.org/2819079/diff/16003/7009#newcode87
client/site_tests/factory_Touchpad/factory_Touchpad.py:87: '沒有偵測到下列 scroll
segments 被觸及，請檢修 [%s]' %
Maybe 未偵測到下列位置的觸控捲動訊號 . Same as above, I'm not quite sure what is a "scroll
segment", so please judge if keeping English is better here.

[clchiou, 2010-08-06 20:53:03]

Thanks for Hung-Te's and Tammo's help on making messages more clearer.

Any comments are welcome.

http://codereview.chromium.org/2819079/diff/16003/7003
File client/site_tests/factory_DeveloperRecovery/factory_DeveloperRecovery.py
(right):

http://codereview.chromium.org/2819079/diff/16003/7003#newcode280
client/site_tests/factory_DeveloperRecovery/factory_DeveloperRecovery.py:280:
'Test program does not have correspond hardware '
On 2010/08/06 04:35:56, tammo wrote:
> i suggest "test missing corresponding hardware for %s"
Done.

http://codereview.chromium.org/2819079/diff/16003/7005
File client/site_tests/factory_EnableWriteProtect/factory_EnableWriteProtect.py
(right):

http://codereview.chromium.org/2819079/diff/16003/7005#newcode54
client/site_tests/factory_EnableWriteProtect/factory_EnableWriteProtect.py:54:
raise error.TestError('ERROR: cannot select target %s' %
On 2010/08/06 09:34:12, Hung-Te wrote:
> Since this py file was originally by me... I can provide some candidates to
> these messages. However since most of them should be internal error, I think
> it's OK if you ignore and don't translate most of them, except the "ERROR:
> cannot enable write protection" which is the only error that operator may get.
> 
> 錯誤: 無法選取快閃記憶體目標 %s
Done.

http://codereview.chromium.org/2819079/diff/16003/7005#newcode60
client/site_tests/factory_EnableWriteProtect/factory_EnableWriteProtect.py:60:
raise error.TestError('Cannot read valid flash rom data.')
On 2010/08/06 09:34:12, Hung-Te wrote:
> 無法讀取快閃記憶體資料
Done.

http://codereview.chromium.org/2819079/diff/16003/7005#newcode64
client/site_tests/factory_EnableWriteProtect/factory_EnableWriteProtect.py:64:
raise error.TestError('Cannot detect flash rom layout.')
On 2010/08/06 09:34:12, Hung-Te wrote:
> 無法偵測快閃記憶體配置結構
Done.

http://codereview.chromium.org/2819079/diff/16003/7005#newcode70
client/site_tests/factory_EnableWriteProtect/factory_EnableWriteProtect.py:70:
raise error.TestError("INTERNAL ERROR: Must be 1 RO section")
On 2010/08/06 09:34:12, Hung-Te wrote:
> 內部錯誤: 須要單一個唯讀區段
Done.

http://codereview.chromium.org/2819079/diff/16003/7005#newcode72
client/site_tests/factory_EnableWriteProtect/factory_EnableWriteProtect.py:72:
raise error.TestError('ERROR: cannot enable write protection.')
On 2010/08/06 09:34:12, Hung-Te wrote:
> 錯誤: 無法啟用寫入保護
Done.

http://codereview.chromium.org/2819079/diff/16003/7005#newcode76
client/site_tests/factory_EnableWriteProtect/factory_EnableWriteProtect.py:76:
raise error.TestError('ERROR: cannot restore target.')
On 2010/08/06 09:34:12, Hung-Te wrote:
> 錯誤: 無法還原快閃記憶體目標
Done.

http://codereview.chromium.org/2819079/diff/16003/7006
File client/site_tests/factory_ExternalStorage/factory_ExternalStorage.py
(right):

http://codereview.chromium.org/2819079/diff/16003/7006#newcode97
client/site_tests/factory_ExternalStorage/factory_ExternalStorage.py:97: if
len(diff) > 1 or self._target_device not in diff:
On 2010/08/06 04:35:56, tammo wrote:
> was the original condition wrong?  it looks to me like this could be rolled
into
> the enclosing condition...
The original condition is correct, though I think is a little unclear about what
it is testing.
I rewrite this part in next patch to make it more clearer (in my point of view).

http://codereview.chromium.org/2819079/diff/16003/7008
File client/site_tests/factory_Leds/factory_Leds.py (right):

http://codereview.chromium.org/2819079/diff/16003/7008#newcode211
client/site_tests/factory_Leds/factory_Leds.py:211: raise error.TestFail('Some
patterns failed' \
On 2010/08/06 09:34:12, Hung-Te wrote:
> seems like missing a \n here
Done.

http://codereview.chromium.org/2819079/diff/16003/7009
File client/site_tests/factory_Touchpad/factory_Touchpad.py (right):

http://codereview.chromium.org/2819079/diff/16003/7009#newcode81
client/site_tests/factory_Touchpad/factory_Touchpad.py:81: '沒有偵測到下列 motion
sectors 被觸及，請檢修 [%s]' %
On 2010/08/06 09:34:12, Hung-Te wrote:
> According to the real UI, will this be better translated as:
> 未偵測到下列位置的觸控移動訊號
> 
> Not sure if this is better because I'm not sure what "motion sector" is, so
you
> can judge by yourself if you'd prefer to keep the English terms.
Done. This looks better.

http://codereview.chromium.org/2819079/diff/16003/7009#newcode87
client/site_tests/factory_Touchpad/factory_Touchpad.py:87: '沒有偵測到下列 scroll
segments 被觸及，請檢修 [%s]' %
On 2010/08/06 09:34:12, Hung-Te wrote:
> Maybe 未偵測到下列位置的觸控捲動訊號 . Same as above, I'm not quite sure what is a "scroll
> segment", so please judge if keeping English is better here.
Done. This looks better.

[Hung-Te, 2010-08-09 02:50:09]

LGTM

[Tammo Spalink, 2010-08-09 02:54:22]

lgtm

[Nick Sanders, 2010-08-10 07:32:35]

http://codereview.chromium.org/2819079/diff/41001/42006
File client/site_tests/factory_Keyboard/factory_Keyboard.py (right):

http://codereview.chromium.org/2819079/diff/41001/42006#newcode143
client/site_tests/factory_Keyboard/factory_Keyboard.py:143: kbd_image =
cairo.ImageSurface.create_from_png('%s.png' % layout)
Can you put try/catch around this and add "keyboard img missing" and "keyboard
binding missing" error msg

[clchiou, 2010-08-23 04:31:32]

Hi Nick,

I grep'ed source code and found two places that depends on third-party files. 
They are in keyboard and leds testing.  If I missed any places, please help me
indicate where it is.  Thank you very much.

On 2010/08/10 07:32:35, Nick Sanders wrote:
> http://codereview.chromium.org/2819079/diff/41001/42006
> File client/site_tests/factory_Keyboard/factory_Keyboard.py (right):
> 
> http://codereview.chromium.org/2819079/diff/41001/42006#newcode143
> client/site_tests/factory_Keyboard/factory_Keyboard.py:143: kbd_image =
> cairo.ImageSurface.create_from_png('%s.png' % layout)
> Can you put try/catch around this and add "keyboard img missing" and "keyboard
> binding missing" error msg

[Nick Sanders, 2010-08-23 05:38:37]

Can you download the buildbot factory image, run it on an L13 and see in
which places if fails?
These two are good, but I think there are more places where it will fail,
but grep probably can''t find them, there will also be cases where the code
assumes H2C, or is device specific, or whatever.

Thanks,
  -Nick

On Sun, Aug 22, 2010 at 9:31 PM, <clchiou@google.com> wrote:

> Hi Nick,
>
> I grep'ed source code and found two places that depends on third-party
> files.
> They are in keyboard and leds testing.  If I missed any places, please help
> me
> indicate where it is.  Thank you very much.
>
> On 2010/08/10 07:32:35, Nick Sanders wrote:
>
>> http://codereview.chromium.org/2819079/diff/41001/42006
>> File client/site_tests/factory_Keyboard/factory_Keyboard.py (right):
>>
>
>  http://codereview.chromium.org/2819079/diff/41001/42006#newcode143
>> client/site_tests/factory_Keyboard/factory_Keyboard.py:143: kbd_image =
>> cairo.ImageSurface.create_from_png('%s.png' % layout)
>> Can you put try/catch around this and add "keyboard img missing" and
>> "keyboard
>> binding missing" error msg
>>
>
>
>
> http://codereview.chromium.org/2819079/show
>

[clchiou, 2010-09-01 04:57:54]

Hi Nick,

Many thanks for the comments and sorry for the late reply.

Summary of test result of buildbot TOT factory image on L13:

1 Tests with gpio setup failure
 * factory_DeveloperRecovery and hardware_GPIOSwitches at runin test
 * Command
   /usr/sbin/gpio_setup
 * AI: add try-catch block (done)

2 Tests with amixer command failed
 * factory_ExtDisplay
 * Command
   amixer -c 0 cset name="IEC958 Playback Switch" on
   amixer -c 0 cset name="IEC958 Playback Switch" off
 * AI: add try-catch block (done)

3 Tests that have a bug
 * factory_LightSensor
 * bug: error.CmdError require 2 arguments but only 1 given
 * AI: write to original author (done)

4 Test error that I am not sure whether it is supposed to happen
 * factory_WriteGBB (at final stage1)
   TestError: Unable to find GBB file: gbb_*
 * hardware_Components and hardware_GPIOSwitches at runin test
   TestError: Cannot get section [FV_BSTUB] from flashrom
 * AI: ?

As for the item 4, these tests are Google Required Tests for Google-blessed
devices.  It is not my authority to decide these error should or should not
happen in these tests.
* If they should happen, I think the original error message is clear enough; I
could try to make it clearer, but I don't think there would be much I can do.
* If they should not happen, we should file another bug for them.

On 2010/08/23 05:38:37, Nick Sanders wrote:
> Can you download the buildbot factory image, run it on an L13 and see in
> which places if fails?
> These two are good, but I think there are more places where it will fail,
> but grep probably can''t find them, there will also be cases where the code
> assumes H2C, or is device specific, or whatever.
> 
> Thanks,
>   -Nick
> 
> On Sun, Aug 22, 2010 at 9:31 PM, <mailto:clchiou@google.com> wrote:
> 
> > Hi Nick,
> >
> > I grep'ed source code and found two places that depends on third-party
> > files.
> > They are in keyboard and leds testing.  If I missed any places, please help
> > me
> > indicate where it is.  Thank you very much.
> >
> > On 2010/08/10 07:32:35, Nick Sanders wrote:
> >
> >> http://codereview.chromium.org/2819079/diff/41001/42006
> >> File client/site_tests/factory_Keyboard/factory_Keyboard.py (right):
> >>
> >
> >  http://codereview.chromium.org/2819079/diff/41001/42006#newcode143
> >> client/site_tests/factory_Keyboard/factory_Keyboard.py:143: kbd_image =
> >> cairo.ImageSurface.create_from_png('%s.png' % layout)
> >> Can you put try/catch around this and add "keyboard img missing" and
> >> "keyboard
> >> binding missing" error msg
> >>
> >
> >
> >
> > http://codereview.chromium.org/2819079/show
> >
>

[Nick Sanders, 2010-09-01 09:46:01]

Sounds good!

On Tue, Aug 31, 2010 at 9:57 PM, <clchiou@google.com> wrote:

> Hi Nick,
>
> Many thanks for the comments and sorry for the late reply.
>
> Summary of test result of buildbot TOT factory image on L13:
>
> 1 Tests with gpio setup failure
>  * factory_DeveloperRecovery and hardware_GPIOSwitches at runin test
>  * Command
>   /usr/sbin/gpio_setup
>  * AI: add try-catch block (done)
>
> 2 Tests with amixer command failed
>  * factory_ExtDisplay
>  * Command
>   amixer -c 0 cset name="IEC958 Playback Switch" on
>   amixer -c 0 cset name="IEC958 Playback Switch" off
>  * AI: add try-catch block (done)
>
> 3 Tests that have a bug
>  * factory_LightSensor
>  * bug: error.CmdError require 2 arguments but only 1 given
>  * AI: write to original author (done)
>
> 4 Test error that I am not sure whether it is supposed to happen
>  * factory_WriteGBB (at final stage1)
>   TestError: Unable to find GBB file: gbb_*
>  * hardware_Components and hardware_GPIOSwitches at runin test
>   TestError: Cannot get section [FV_BSTUB] from flashrom
>  * AI: ?
>

For this one, as long as the test prints out these errors, it should be
fine.
The test should not pass,  however they should give good error messages.


> As for the item 4, these tests are Google Required Tests for Google-blessed
> devices.  It is not my authority to decide these error should or should not
> happen in these tests.
>

Since you are part of Google, it is actually your authority (since we are
the
ones blessing the devices). =)
But I agree with you that the errors reported are clear enough.


> * If they should happen, I think the original error message is clear
> enough; I
> could try to make it clearer, but I don't think there would be much I can
> do.
> * If they should not happen, we should file another bug for them.
>
> On 2010/08/23 05:38:37, Nick Sanders wrote:
>
>> Can you download the buildbot factory image, run it on an L13 and see in
>> which places if fails?
>> These two are good, but I think there are more places where it will fail,
>> but grep probably can''t find them, there will also be cases where the
>> code
>> assumes H2C, or is device specific, or whatever.
>>
>
>  Thanks,
>>   -Nick
>>
>
>  On Sun, Aug 22, 2010 at 9:31 PM, <mailto:clchiou@google.com> wrote:
>>
>
>  > Hi Nick,
>> >
>> > I grep'ed source code and found two places that depends on third-party
>> > files.
>> > They are in keyboard and leds testing.  If I missed any places, please
>> help
>> > me
>> > indicate where it is.  Thank you very much.
>> >
>> > On 2010/08/10 07:32:35, Nick Sanders wrote:
>> >
>> >> http://codereview.chromium.org/2819079/diff/41001/42006
>> >> File client/site_tests/factory_Keyboard/factory_Keyboard.py (right):
>> >>
>> >
>> >  http://codereview.chromium.org/2819079/diff/41001/42006#newcode143
>> >> client/site_tests/factory_Keyboard/factory_Keyboard.py:143: kbd_image =
>> >> cairo.ImageSurface.create_from_png('%s.png' % layout)
>> >> Can you put try/catch around this and add "keyboard img missing" and
>> >> "keyboard
>> >> binding missing" error msg
>> >>
>> >
>> >
>> >
>> > http://codereview.chromium.org/2819079/show
>> >
>>
>
>
>
>
> http://codereview.chromium.org/2819079/show
>

[Tom Wai-Hong Tam, 2010-09-03 01:35:11]

On Wed, Sep 1, 2010 at 2:45 AM, Nick Sanders <nsanders@chromium.org> wrote:

> Sounds good!
>
> On Tue, Aug 31, 2010 at 9:57 PM, <clchiou@google.com> wrote:
>
>> Hi Nick,
>>
>> Many thanks for the comments and sorry for the late reply.
>>
>> Summary of test result of buildbot TOT factory image on L13:
>>
>> 1 Tests with gpio setup failure
>>  * factory_DeveloperRecovery and hardware_GPIOSwitches at runin test
>>  * Command
>>   /usr/sbin/gpio_setup
>>  * AI: add try-catch block (done)
>>
>> 2 Tests with amixer command failed
>>  * factory_ExtDisplay
>>  * Command
>>   amixer -c 0 cset name="IEC958 Playback Switch" on
>>   amixer -c 0 cset name="IEC958 Playback Switch" off
>>  * AI: add try-catch block (done)
>>
>> 3 Tests that have a bug
>>  * factory_LightSensor
>>  * bug: error.CmdError require 2 arguments but only 1 given
>>  * AI: write to original author (done)
>>
>> 4 Test error that I am not sure whether it is supposed to happen
>>  * factory_WriteGBB (at final stage1)
>>   TestError: Unable to find GBB file: gbb_*
>>  * hardware_Components and hardware_GPIOSwitches at runin test
>>   TestError: Cannot get section [FV_BSTUB] from flashrom
>>  * AI: ?
>>
>
> For this one, as long as the test prints out these errors, it should be
> fine.
> The test should not pass,  however they should give good error messages.
>

For the first item, since there is no gbb yet, so the failure is expected.
We are working on it. Or you can grab one from current BIOS.

For the second iterm, the main problem of hardware_Components test failed is
you run it on legacy bios. I agree should give good error messages, like
catch the exception and raise another with better message. But I remember
Hungte had a change to ignore errors and just return an empty string, please
sync the code and try.


>
>
>> As for the item 4, these tests are Google Required Tests for
>> Google-blessed
>> devices.  It is not my authority to decide these error should or should
>> not
>> happen in these tests.
>>
>
> Since you are part of Google, it is actually your authority (since we are
> the
> ones blessing the devices). =)
> But I agree with you that the errors reported are clear enough.
>
>
>> * If they should happen, I think the original error message is clear
>> enough; I
>> could try to make it clearer, but I don't think there would be much I can
>> do.
>> * If they should not happen, we should file another bug for them.
>>
>> On 2010/08/23 05:38:37, Nick Sanders wrote:
>>
>>> Can you download the buildbot factory image, run it on an L13 and see in
>>> which places if fails?
>>> These two are good, but I think there are more places where it will fail,
>>> but grep probably can''t find them, there will also be cases where the
>>> code
>>> assumes H2C, or is device specific, or whatever.
>>>
>>
>>  Thanks,
>>>   -Nick
>>>
>>
>>  On Sun, Aug 22, 2010 at 9:31 PM, <mailto:clchiou@google.com> wrote:
>>>
>>
>>  > Hi Nick,
>>> >
>>> > I grep'ed source code and found two places that depends on third-party
>>> > files.
>>> > They are in keyboard and leds testing.  If I missed any places, please
>>> help
>>> > me
>>> > indicate where it is.  Thank you very much.
>>> >
>>> > On 2010/08/10 07:32:35, Nick Sanders wrote:
>>> >
>>> >> http://codereview.chromium.org/2819079/diff/41001/42006
>>> >> File client/site_tests/factory_Keyboard/factory_Keyboard.py (right):
>>> >>
>>> >
>>> >  http://codereview.chromium.org/2819079/diff/41001/42006#newcode143
>>> >> client/site_tests/factory_Keyboard/factory_Keyboard.py:143: kbd_image
>>> =
>>> >> cairo.ImageSurface.create_from_png('%s.png' % layout)
>>> >> Can you put try/catch around this and add "keyboard img missing" and
>>> >> "keyboard
>>> >> binding missing" error msg
>>> >>
>>> >
>>> >
>>> >
>>> > http://codereview.chromium.org/2819079/show
>>> >
>>>
>>
>>
>>
>>
>> http://codereview.chromium.org/2819079/show
>>
>
>

[clchiou, 2010-09-03 03:54:47]

Hi Tom,

Thanks for the suggestions.  I was not proposing add try-catch block for these
exceptions, but was uncertain what should be done.

On the first suggestion, thanks for updating the status of GBB.  I think it
would be okay I just leave it there until GBB is submitted, and then we could
discuss what we should do.

On the second suggestion, I tested it again with a newer TOT factory image
including Hung-Te's patch.  It still failed as expected due to the legacy BIOS
on L13, but I saw Hung-Te's patch was catching exceptions.

On 2010/09/03 01:35:11, Tom Wai-Hong Tam wrote:
> On Wed, Sep 1, 2010 at 2:45 AM, Nick Sanders <mailto:nsanders@chromium.org>
wrote:
> 
> > Sounds good!
> >
> > On Tue, Aug 31, 2010 at 9:57 PM, <mailto:clchiou@google.com> wrote:
> >
> >> Hi Nick,
> >>
> >> Many thanks for the comments and sorry for the late reply.
> >>
> >> Summary of test result of buildbot TOT factory image on L13:
> >>
> >> 1 Tests with gpio setup failure
> >>  * factory_DeveloperRecovery and hardware_GPIOSwitches at runin test
> >>  * Command
> >>   /usr/sbin/gpio_setup
> >>  * AI: add try-catch block (done)
> >>
> >> 2 Tests with amixer command failed
> >>  * factory_ExtDisplay
> >>  * Command
> >>   amixer -c 0 cset name="IEC958 Playback Switch" on
> >>   amixer -c 0 cset name="IEC958 Playback Switch" off
> >>  * AI: add try-catch block (done)
> >>
> >> 3 Tests that have a bug
> >>  * factory_LightSensor
> >>  * bug: error.CmdError require 2 arguments but only 1 given
> >>  * AI: write to original author (done)
> >>
> >> 4 Test error that I am not sure whether it is supposed to happen
> >>  * factory_WriteGBB (at final stage1)
> >>   TestError: Unable to find GBB file: gbb_*
> >>  * hardware_Components and hardware_GPIOSwitches at runin test
> >>   TestError: Cannot get section [FV_BSTUB] from flashrom
> >>  * AI: ?
> >>
> >
> > For this one, as long as the test prints out these errors, it should be
> > fine.
> > The test should not pass,  however they should give good error messages.
> >
> 
> For the first item, since there is no gbb yet, so the failure is expected.
> We are working on it. Or you can grab one from current BIOS.
> 
> For the second iterm, the main problem of hardware_Components test failed is
> you run it on legacy bios. I agree should give good error messages, like
> catch the exception and raise another with better message. But I remember
> Hungte had a change to ignore errors and just return an empty string, please
> sync the code and try.
> 
> 
> >
> >
> >> As for the item 4, these tests are Google Required Tests for
> >> Google-blessed
> >> devices.  It is not my authority to decide these error should or should
> >> not
> >> happen in these tests.
> >>
> >
> > Since you are part of Google, it is actually your authority (since we are
> > the
> > ones blessing the devices). =)
> > But I agree with you that the errors reported are clear enough.
> >
> >
> >> * If they should happen, I think the original error message is clear
> >> enough; I
> >> could try to make it clearer, but I don't think there would be much I can
> >> do.
> >> * If they should not happen, we should file another bug for them.
> >>
> >> On 2010/08/23 05:38:37, Nick Sanders wrote:
> >>
> >>> Can you download the buildbot factory image, run it on an L13 and see in
> >>> which places if fails?
> >>> These two are good, but I think there are more places where it will fail,
> >>> but grep probably can''t find them, there will also be cases where the
> >>> code
> >>> assumes H2C, or is device specific, or whatever.
> >>>
> >>
> >>  Thanks,
> >>>   -Nick
> >>>
> >>
> >>  On Sun, Aug 22, 2010 at 9:31 PM, <mailto:clchiou@google.com> wrote:
> >>>
> >>
> >>  > Hi Nick,
> >>> >
> >>> > I grep'ed source code and found two places that depends on third-party
> >>> > files.
> >>> > They are in keyboard and leds testing.  If I missed any places, please
> >>> help
> >>> > me
> >>> > indicate where it is.  Thank you very much.
> >>> >
> >>> > On 2010/08/10 07:32:35, Nick Sanders wrote:
> >>> >
> >>> >> http://codereview.chromium.org/2819079/diff/41001/42006
> >>> >> File client/site_tests/factory_Keyboard/factory_Keyboard.py (right):
> >>> >>
> >>> >
> >>> >  http://codereview.chromium.org/2819079/diff/41001/42006#newcode143
> >>> >> client/site_tests/factory_Keyboard/factory_Keyboard.py:143: kbd_image
> >>> =
> >>> >> cairo.ImageSurface.create_from_png('%s.png' % layout)
> >>> >> Can you put try/catch around this and add "keyboard img missing" and
> >>> >> "keyboard
> >>> >> binding missing" error msg
> >>> >>
> >>> >
> >>> >
> >>> >
> >>> > http://codereview.chromium.org/2819079/show
> >>> >
> >>>
> >>
> >>
> >>
> >>
> >> http://codereview.chromium.org/2819079/show
> >>
> >
> >
>

[Tom Wai-Hong Tam, 2010-09-03 04:51:26]

On Thu, Sep 2, 2010 at 8:54 PM, <clchiou@google.com> wrote:

> Hi Tom,
>
> Thanks for the suggestions.  I was not proposing add try-catch block for
> these
> exceptions, but was uncertain what should be done.
>
> On the first suggestion, thanks for updating the status of GBB.  I think it
> would be okay I just leave it there until GBB is submitted, and then we
> could
> discuss what we should do.
>
> I just submitted a workable GBB. So you can try it now. Heard that there
are 2 Mario prototypes in TW. It'd be very great if you can try the whole
factory flow on it, including firmware update, matching HWID, and replacing
the GBB.


> On the second suggestion, I tested it again with a newer TOT factory image
> including Hung-Te's patch.  It still failed as expected due to the legacy
> BIOS
> on L13, but I saw Hung-Te's patch was catching exceptions.


Maybe you can check this issue with Hung-Te.

>

On 2010/09/03 01:35:11, Tom Wai-Hong Tam wrote:
>
>> On Wed, Sep 1, 2010 at 2:45 AM, Nick Sanders <mailto:
>> nsanders@chromium.org>
>>
> wrote:
>
>  > Sounds good!
>>
>> >
>> > On Tue, Aug 31, 2010 at 9:57 PM, <mailto:clchiou@google.com> wrote:
>> >
>> >> Hi Nick,
>> >>
>> >> Many thanks for the comments and sorry for the late reply.
>> >>
>> >> Summary of test result of buildbot TOT factory image on L13:
>> >>
>> >> 1 Tests with gpio setup failure
>> >>  * factory_DeveloperRecovery and hardware_GPIOSwitches at runin test
>> >>  * Command
>> >>   /usr/sbin/gpio_setup
>> >>  * AI: add try-catch block (done)
>> >>
>> >> 2 Tests with amixer command failed
>> >>  * factory_ExtDisplay
>> >>  * Command
>> >>   amixer -c 0 cset name="IEC958 Playback Switch" on
>> >>   amixer -c 0 cset name="IEC958 Playback Switch" off
>> >>  * AI: add try-catch block (done)
>> >>
>> >> 3 Tests that have a bug
>> >>  * factory_LightSensor
>> >>  * bug: error.CmdError require 2 arguments but only 1 given
>> >>  * AI: write to original author (done)
>> >>
>> >> 4 Test error that I am not sure whether it is supposed to happen
>> >>  * factory_WriteGBB (at final stage1)
>> >>   TestError: Unable to find GBB file: gbb_*
>> >>  * hardware_Components and hardware_GPIOSwitches at runin test
>> >>   TestError: Cannot get section [FV_BSTUB] from flashrom
>> >>  * AI: ?
>> >>
>> >
>> > For this one, as long as the test prints out these errors, it should be
>> > fine.
>> > The test should not pass,  however they should give good error messages.
>> >
>>
>
>  For the first item, since there is no gbb yet, so the failure is expected.
>> We are working on it. Or you can grab one from current BIOS.
>>
>
>  For the second iterm, the main problem of hardware_Components test failed
>> is
>> you run it on legacy bios. I agree should give good error messages, like
>> catch the exception and raise another with better message. But I remember
>> Hungte had a change to ignore errors and just return an empty string,
>> please
>> sync the code and try.
>>
>
>
>  >
>> >
>> >> As for the item 4, these tests are Google Required Tests for
>> >> Google-blessed
>> >> devices.  It is not my authority to decide these error should or should
>> >> not
>> >> happen in these tests.
>> >>
>> >
>> > Since you are part of Google, it is actually your authority (since we
>> are
>> > the
>> > ones blessing the devices). =)
>> > But I agree with you that the errors reported are clear enough.
>> >
>> >
>> >> * If they should happen, I think the original error message is clear
>> >> enough; I
>> >> could try to make it clearer, but I don't think there would be much I
>> can
>> >> do.
>> >> * If they should not happen, we should file another bug for them.
>> >>
>> >> On 2010/08/23 05:38:37, Nick Sanders wrote:
>> >>
>> >>> Can you download the buildbot factory image, run it on an L13 and see
>> in
>> >>> which places if fails?
>> >>> These two are good, but I think there are more places where it will
>> fail,
>> >>> but grep probably can''t find them, there will also be cases where the
>> >>> code
>> >>> assumes H2C, or is device specific, or whatever.
>> >>>
>> >>
>> >>  Thanks,
>> >>>   -Nick
>> >>>
>> >>
>> >>  On Sun, Aug 22, 2010 at 9:31 PM, <mailto:clchiou@google.com> wrote:
>> >>>
>> >>
>> >>  > Hi Nick,
>> >>> >
>> >>> > I grep'ed source code and found two places that depends on
>> third-party
>> >>> > files.
>> >>> > They are in keyboard and leds testing.  If I missed any places,
>> please
>> >>> help
>> >>> > me
>> >>> > indicate where it is.  Thank you very much.
>> >>> >
>> >>> > On 2010/08/10 07:32:35, Nick Sanders wrote:
>> >>> >
>> >>> >> http://codereview.chromium.org/2819079/diff/41001/42006
>> >>> >> File client/site_tests/factory_Keyboard/factory_Keyboard.py
>> (right):
>> >>> >>
>> >>> >
>> >>> >  http://codereview.chromium.org/2819079/diff/41001/42006#newcode143
>> >>> >> client/site_tests/factory_Keyboard/factory_Keyboard.py:143:
>> kbd_image
>> >>> =
>> >>> >> cairo.ImageSurface.create_from_png('%s.png' % layout)
>> >>> >> Can you put try/catch around this and add "keyboard img missing"
>> and
>> >>> >> "keyboard
>> >>> >> binding missing" error msg
>> >>> >>
>> >>> >
>> >>> >
>> >>> >
>> >>> > http://codereview.chromium.org/2819079/show
>> >>> >
>> >>>
>> >>
>> >>
>> >>
>> >>
>> >> http://codereview.chromium.org/2819079/show
>> >>
>> >
>> >
>>
>
>
>
>
> http://codereview.chromium.org/2819079/show
>

[Tom Wai-Hong Tam, 2010-09-03 04:51:35]

On Thu, Sep 2, 2010 at 8:54 PM, <clchiou@google.com> wrote:

> Hi Tom,
>
> Thanks for the suggestions.  I was not proposing add try-catch block for
> these
> exceptions, but was uncertain what should be done.
>
> On the first suggestion, thanks for updating the status of GBB.  I think it
> would be okay I just leave it there until GBB is submitted, and then we
> could
> discuss what we should do.
>
> I just submitted a workable GBB. So you can try it now. Heard that there
are 2 Mario prototypes in TW. It'd be very great if you can try the whole
factory flow on it, including firmware update, matching HWID, and replacing
the GBB.


> On the second suggestion, I tested it again with a newer TOT factory image
> including Hung-Te's patch.  It still failed as expected due to the legacy
> BIOS
> on L13, but I saw Hung-Te's patch was catching exceptions.


Maybe you can check this issue with Hung-Te.

>

On 2010/09/03 01:35:11, Tom Wai-Hong Tam wrote:
>
>> On Wed, Sep 1, 2010 at 2:45 AM, Nick Sanders <mailto:
>> nsanders@chromium.org>
>>
> wrote:
>
>  > Sounds good!
>>
>> >
>> > On Tue, Aug 31, 2010 at 9:57 PM, <mailto:clchiou@google.com> wrote:
>> >
>> >> Hi Nick,
>> >>
>> >> Many thanks for the comments and sorry for the late reply.
>> >>
>> >> Summary of test result of buildbot TOT factory image on L13:
>> >>
>> >> 1 Tests with gpio setup failure
>> >>  * factory_DeveloperRecovery and hardware_GPIOSwitches at runin test
>> >>  * Command
>> >>   /usr/sbin/gpio_setup
>> >>  * AI: add try-catch block (done)
>> >>
>> >> 2 Tests with amixer command failed
>> >>  * factory_ExtDisplay
>> >>  * Command
>> >>   amixer -c 0 cset name="IEC958 Playback Switch" on
>> >>   amixer -c 0 cset name="IEC958 Playback Switch" off
>> >>  * AI: add try-catch block (done)
>> >>
>> >> 3 Tests that have a bug
>> >>  * factory_LightSensor
>> >>  * bug: error.CmdError require 2 arguments but only 1 given
>> >>  * AI: write to original author (done)
>> >>
>> >> 4 Test error that I am not sure whether it is supposed to happen
>> >>  * factory_WriteGBB (at final stage1)
>> >>   TestError: Unable to find GBB file: gbb_*
>> >>  * hardware_Components and hardware_GPIOSwitches at runin test
>> >>   TestError: Cannot get section [FV_BSTUB] from flashrom
>> >>  * AI: ?
>> >>
>> >
>> > For this one, as long as the test prints out these errors, it should be
>> > fine.
>> > The test should not pass,  however they should give good error messages.
>> >
>>
>
>  For the first item, since there is no gbb yet, so the failure is expected.
>> We are working on it. Or you can grab one from current BIOS.
>>
>
>  For the second iterm, the main problem of hardware_Components test failed
>> is
>> you run it on legacy bios. I agree should give good error messages, like
>> catch the exception and raise another with better message. But I remember
>> Hungte had a change to ignore errors and just return an empty string,
>> please
>> sync the code and try.
>>
>
>
>  >
>> >
>> >> As for the item 4, these tests are Google Required Tests for
>> >> Google-blessed
>> >> devices.  It is not my authority to decide these error should or should
>> >> not
>> >> happen in these tests.
>> >>
>> >
>> > Since you are part of Google, it is actually your authority (since we
>> are
>> > the
>> > ones blessing the devices). =)
>> > But I agree with you that the errors reported are clear enough.
>> >
>> >
>> >> * If they should happen, I think the original error message is clear
>> >> enough; I
>> >> could try to make it clearer, but I don't think there would be much I
>> can
>> >> do.
>> >> * If they should not happen, we should file another bug for them.
>> >>
>> >> On 2010/08/23 05:38:37, Nick Sanders wrote:
>> >>
>> >>> Can you download the buildbot factory image, run it on an L13 and see
>> in
>> >>> which places if fails?
>> >>> These two are good, but I think there are more places where it will
>> fail,
>> >>> but grep probably can''t find them, there will also be cases where the
>> >>> code
>> >>> assumes H2C, or is device specific, or whatever.
>> >>>
>> >>
>> >>  Thanks,
>> >>>   -Nick
>> >>>
>> >>
>> >>  On Sun, Aug 22, 2010 at 9:31 PM, <mailto:clchiou@google.com> wrote:
>> >>>
>> >>
>> >>  > Hi Nick,
>> >>> >
>> >>> > I grep'ed source code and found two places that depends on
>> third-party
>> >>> > files.
>> >>> > They are in keyboard and leds testing.  If I missed any places,
>> please
>> >>> help
>> >>> > me
>> >>> > indicate where it is.  Thank you very much.
>> >>> >
>> >>> > On 2010/08/10 07:32:35, Nick Sanders wrote:
>> >>> >
>> >>> >> http://codereview.chromium.org/2819079/diff/41001/42006
>> >>> >> File client/site_tests/factory_Keyboard/factory_Keyboard.py
>> (right):
>> >>> >>
>> >>> >
>> >>> >  http://codereview.chromium.org/2819079/diff/41001/42006#newcode143
>> >>> >> client/site_tests/factory_Keyboard/factory_Keyboard.py:143:
>> kbd_image
>> >>> =
>> >>> >> cairo.ImageSurface.create_from_png('%s.png' % layout)
>> >>> >> Can you put try/catch around this and add "keyboard img missing"
>> and
>> >>> >> "keyboard
>> >>> >> binding missing" error msg
>> >>> >>
>> >>> >
>> >>> >
>> >>> >
>> >>> > http://codereview.chromium.org/2819079/show
>> >>> >
>> >>>
>> >>
>> >>
>> >>
>> >>
>> >> http://codereview.chromium.org/2819079/show
>> >>
>> >
>> >
>>
>
>
>
>
> http://codereview.chromium.org/2819079/show
>

[hungte, 2010-09-03 05:07:46]

On Fri, Sep 3, 2010 at 12:50 PM, Tom Wai-Hong Tam (談偉航) <
waihong@chromium.org> wrote:

> On Thu, Sep 2, 2010 at 8:54 PM, <clchiou@google.com> wrote:
>
>> Hi Tom,
>>
>> Thanks for the suggestions.  I was not proposing add try-catch block for
>> these
>> exceptions, but was uncertain what should be done.
>>
>> On the first suggestion, thanks for updating the status of GBB.  I think
>> it
>> would be okay I just leave it there until GBB is submitted, and then we
>> could
>> discuss what we should do.
>>
>> I just submitted a workable GBB. So you can try it now. Heard that there
> are 2 Mario prototypes in TW. It'd be very great if you can try the whole
> factory flow on it, including firmware update, matching HWID, and replacing
> the GBB.
>
>
>> On the second suggestion, I tested it again with a newer TOT factory image
>> including Hung-Te's patch.  It still failed as expected due to the legacy
>> BIOS
>> on L13, but I saw Hung-Te's patch was catching exceptions.
>
>

> Maybe you can check this issue with Hung-Te.
>
    You just get confused as I did :-)
    Well, Che-Liang simply wanted to say the patch worked as he expected,
    so there's no issue here.


>
>
> On 2010/09/03 01:35:11, Tom Wai-Hong Tam wrote:
>>
>>> On Wed, Sep 1, 2010 at 2:45 AM, Nick Sanders <mailto:
>>> nsanders@chromium.org>
>>>
>> wrote:
>>
>>  > Sounds good!
>>>
>>> >
>>> > On Tue, Aug 31, 2010 at 9:57 PM, <mailto:clchiou@google.com> wrote:
>>> >
>>> >> Hi Nick,
>>> >>
>>> >> Many thanks for the comments and sorry for the late reply.
>>> >>
>>> >> Summary of test result of buildbot TOT factory image on L13:
>>> >>
>>> >> 1 Tests with gpio setup failure
>>> >>  * factory_DeveloperRecovery and hardware_GPIOSwitches at runin test
>>> >>  * Command
>>> >>   /usr/sbin/gpio_setup
>>> >>  * AI: add try-catch block (done)
>>> >>
>>> >> 2 Tests with amixer command failed
>>> >>  * factory_ExtDisplay
>>> >>  * Command
>>> >>   amixer -c 0 cset name="IEC958 Playback Switch" on
>>> >>   amixer -c 0 cset name="IEC958 Playback Switch" off
>>> >>  * AI: add try-catch block (done)
>>> >>
>>> >> 3 Tests that have a bug
>>> >>  * factory_LightSensor
>>> >>  * bug: error.CmdError require 2 arguments but only 1 given
>>> >>  * AI: write to original author (done)
>>> >>
>>> >> 4 Test error that I am not sure whether it is supposed to happen
>>> >>  * factory_WriteGBB (at final stage1)
>>> >>   TestError: Unable to find GBB file: gbb_*
>>> >>  * hardware_Components and hardware_GPIOSwitches at runin test
>>> >>   TestError: Cannot get section [FV_BSTUB] from flashrom
>>> >>  * AI: ?
>>> >>
>>> >
>>> > For this one, as long as the test prints out these errors, it should be
>>> > fine.
>>> > The test should not pass,  however they should give good error
>>> messages.
>>> >
>>>
>>
>>  For the first item, since there is no gbb yet, so the failure is
>>> expected.
>>> We are working on it. Or you can grab one from current BIOS.
>>>
>>
>>  For the second iterm, the main problem of hardware_Components test failed
>>> is
>>> you run it on legacy bios. I agree should give good error messages, like
>>> catch the exception and raise another with better message. But I remember
>>> Hungte had a change to ignore errors and just return an empty string,
>>> please
>>> sync the code and try.
>>>
>>
>>
>>  >
>>> >
>>> >> As for the item 4, these tests are Google Required Tests for
>>> >> Google-blessed
>>> >> devices.  It is not my authority to decide these error should or
>>> should
>>> >> not
>>> >> happen in these tests.
>>> >>
>>> >
>>> > Since you are part of Google, it is actually your authority (since we
>>> are
>>> > the
>>> > ones blessing the devices). =)
>>> > But I agree with you that the errors reported are clear enough.
>>> >
>>> >
>>> >> * If they should happen, I think the original error message is clear
>>> >> enough; I
>>> >> could try to make it clearer, but I don't think there would be much I
>>> can
>>> >> do.
>>> >> * If they should not happen, we should file another bug for them.
>>> >>
>>> >> On 2010/08/23 05:38:37, Nick Sanders wrote:
>>> >>
>>> >>> Can you download the buildbot factory image, run it on an L13 and see
>>> in
>>> >>> which places if fails?
>>> >>> These two are good, but I think there are more places where it will
>>> fail,
>>> >>> but grep probably can''t find them, there will also be cases where
>>> the
>>> >>> code
>>> >>> assumes H2C, or is device specific, or whatever.
>>> >>>
>>> >>
>>> >>  Thanks,
>>> >>>   -Nick
>>> >>>
>>> >>
>>> >>  On Sun, Aug 22, 2010 at 9:31 PM, <mailto:clchiou@google.com> wrote:
>>> >>>
>>> >>
>>> >>  > Hi Nick,
>>> >>> >
>>> >>> > I grep'ed source code and found two places that depends on
>>> third-party
>>> >>> > files.
>>> >>> > They are in keyboard and leds testing.  If I missed any places,
>>> please
>>> >>> help
>>> >>> > me
>>> >>> > indicate where it is.  Thank you very much.
>>> >>> >
>>> >>> > On 2010/08/10 07:32:35, Nick Sanders wrote:
>>> >>> >
>>> >>> >> http://codereview.chromium.org/2819079/diff/41001/42006
>>> >>> >> File client/site_tests/factory_Keyboard/factory_Keyboard.py
>>> (right):
>>> >>> >>
>>> >>> >
>>> >>> >
>>> http://codereview.chromium.org/2819079/diff/41001/42006#newcode143
>>> >>> >> client/site_tests/factory_Keyboard/factory_Keyboard.py:143:
>>> kbd_image
>>> >>> =
>>> >>> >> cairo.ImageSurface.create_from_png('%s.png' % layout)
>>> >>> >> Can you put try/catch around this and add "keyboard img missing"
>>> and
>>> >>> >> "keyboard
>>> >>> >> binding missing" error msg
>>> >>> >>
>>> >>> >
>>> >>> >
>>> >>> >
>>> >>> > http://codereview.chromium.org/2819079/show
>>> >>> >
>>> >>>
>>> >>
>>> >>
>>> >>
>>> >>
>>> >> http://codereview.chromium.org/2819079/show
>>> >>
>>> >
>>> >
>>>
>>
>>
>>
>>
>> http://codereview.chromium.org/2819079/show
>>
>
>