Added checksums for Intel GM45.

Added checksums for Intel GM45.

This is just to get some kind of error checking on gl_Bench.  Another tool is needed to update checksums file for new platforms and convert and store images.

[piman, 2010-06-18 01:11:34]

http://codereview.chromium.org/2840006/diff/1/3
File client/site_tests/gl_Bench/gl_Bench.py (right):

http://codereview.chromium.org/2840006/diff/1/3#newcode50
client/site_tests/gl_Bench/gl_Bench.py:50: print "# no checksums found for this
board"
how about failing by default in that case, so that it doesn't become trivial for
a vendor to pass a problematic test by just tweaking the renderer string

[petkov, 2010-06-18 04:11:58]

http://codereview.chromium.org/2840006/diff/1/2
File client/deps/glbench/src/checksums (right):

http://codereview.chromium.org/2840006/diff/1/2#newcode1
client/deps/glbench/src/checksums:1: {
Chromium copyright?

http://codereview.chromium.org/2840006/diff/1/2#newcode2
client/deps/glbench/src/checksums:2: 'Tungsten Graphics, Inc / Mesa DRI Mobile
Intel® GM45 Express Chipset GEM 20091221 2009Q4' : {
Is this an automatically generated file? I assume it is. It would have been nice
to still limit the line length to 80 chars though... Also, some proper
indentation may be nice. Not critical.

http://codereview.chromium.org/2840006/diff/1/3
File client/site_tests/gl_Bench/gl_Bench.py (right):

http://codereview.chromium.org/2840006/diff/1/3#newcode11
client/site_tests/gl_Bench/gl_Bench.py:11: def md5_file(filename):
Any reason for this to be global?

http://codereview.chromium.org/2840006/diff/1/3#newcode12
client/site_tests/gl_Bench/gl_Bench.py:12: return utils.system_output('md5sum '
+ filename).split()[0]
There's some md5/hash utilities in utils.py... This is probably simple enough
though.

http://codereview.chromium.org/2840006/diff/1/3#newcode15
client/site_tests/gl_Bench/gl_Bench.py:15: def get_board_id():
Any reason for this to be global?

http://codereview.chromium.org/2840006/diff/1/3#newcode40
client/site_tests/gl_Bench/gl_Bench.py:40: checksums =
eval(open(checksums_filename).read())
There's "read_file" utility to replace the open/read part.

http://codereview.chromium.org/2840006/diff/1/3#newcode43
client/site_tests/gl_Bench/gl_Bench.py:43: print "# running on:", board_id
logging.info

http://codereview.chromium.org/2840006/diff/1/3#newcode50
client/site_tests/gl_Bench/gl_Bench.py:50: print "# no checksums found for this
board"
logging.info/warning/error

http://codereview.chromium.org/2840006/diff/1/3#newcode64
client/site_tests/gl_Bench/gl_Bench.py:64: checksum_match =
checksum_table[testname] == md5_file(
I would do a simpler

if checksum == md5_file:
  keyvals[...] = ...
else:
  keyvals[...] = ...

I find it more readable -- but readability is in the eyes of the beholder, so up
to you :-)

http://codereview.chromium.org/2840006/diff/1/3#newcode66
client/site_tests/gl_Bench/gl_Bench.py:66: keyvals[testname] = float(val) if
checksum_match else float('nan')
Shouldn't this be a failure instead?

http://codereview.chromium.org/2840006/diff/1/3#newcode68
client/site_tests/gl_Bench/gl_Bench.py:68: print '# no checksum found for test',
testname
logging

[Alexey Marinichev, 2010-06-18 19:17:52]

http://codereview.chromium.org/2840006/diff/1/2
File client/deps/glbench/src/checksums (right):

http://codereview.chromium.org/2840006/diff/1/2#newcode1
client/deps/glbench/src/checksums:1: {
On 2010/06/18 04:11:58, petkov wrote:
> Chromium copyright?
> 

I hope this will be autogenerated.  Is there an example of a copyright for the
autogenerated file?

http://codereview.chromium.org/2840006/diff/1/2#newcode2
client/deps/glbench/src/checksums:2: 'Tungsten Graphics, Inc / Mesa DRI Mobile
Intel® GM45 Express Chipset GEM 20091221 2009Q4' : {
On 2010/06/18 04:11:58, petkov wrote:
> Is this an automatically generated file? I assume it is. It would have been
nice
> to still limit the line length to 80 chars though... Also, some proper
> indentation may be nice. Not critical.
> 

I was planning to use pprint, don't want to do anything clever.  pprint of a
dictionary aligns values with colon, that makes lines really too long.  I'll
convert the dictionary to the list of tuples, it will make it more reasonable. 
I will also rename tests with excessively long names.

http://codereview.chromium.org/2840006/diff/1/3
File client/site_tests/gl_Bench/gl_Bench.py (right):

http://codereview.chromium.org/2840006/diff/1/3#newcode11
client/site_tests/gl_Bench/gl_Bench.py:11: def md5_file(filename):
On 2010/06/18 04:11:58, petkov wrote:
> Any reason for this to be global?

Any reason not to?

http://codereview.chromium.org/2840006/diff/1/3#newcode12
client/site_tests/gl_Bench/gl_Bench.py:12: return utils.system_output('md5sum '
+ filename).split()[0]
On 2010/06/18 04:11:58, petkov wrote:
> There's some md5/hash utilities in utils.py... This is probably simple enough
> though.
> 

Reading blocks from a file and updating hash objects seemed like too much work.

http://codereview.chromium.org/2840006/diff/1/3#newcode15
client/site_tests/gl_Bench/gl_Bench.py:15: def get_board_id():
On 2010/06/18 04:11:58, petkov wrote:
> Any reason for this to be global?

Any reason not to?  It doesn't take any inputs.

http://codereview.chromium.org/2840006/diff/1/3#newcode40
client/site_tests/gl_Bench/gl_Bench.py:40: checksums =
eval(open(checksums_filename).read())
On 2010/06/18 04:11:58, petkov wrote:
> There's "read_file" utility to replace the open/read part.
> 

I think this is actually more readable this way -- you don't have to wonder what
exactly "read_file" does.  But I'll change it if you think that will keep it
more consistent.

http://codereview.chromium.org/2840006/diff/1/3#newcode43
client/site_tests/gl_Bench/gl_Bench.py:43: print "# running on:", board_id
On 2010/06/18 04:11:58, petkov wrote:
> logging.info

Done.

http://codereview.chromium.org/2840006/diff/1/3#newcode50
client/site_tests/gl_Bench/gl_Bench.py:50: print "# no checksums found for this
board"
On 2010/06/18 01:11:35, piman wrote:
> how about failing by default in that case, so that it doesn't become trivial
for
> a vendor to pass a problematic test by just tweaking the renderer string

Done.

http://codereview.chromium.org/2840006/diff/1/3#newcode64
client/site_tests/gl_Bench/gl_Bench.py:64: checksum_match =
checksum_table[testname] == md5_file(
On 2010/06/18 04:11:58, petkov wrote:
> I would do a simpler
> 
> if checksum == md5_file:
>   keyvals[...] = ...
> else:
>   keyvals[...] = ...
> 
> I find it more readable -- but readability is in the eyes of the beholder, so
up
> to you :-)
> 

Heh.  This code was doing it differently at first where it made more sense.  Now
it does exactly what you said, so there's no need for extra cleverness :-)

http://codereview.chromium.org/2840006/diff/1/3#newcode66
client/site_tests/gl_Bench/gl_Bench.py:66: keyvals[testname] = float(val) if
checksum_match else float('nan')
On 2010/06/18 04:11:58, petkov wrote:
> Shouldn't this be a failure instead?
> 

Want to log all of those before I report the failure.  Is that possible somehow?

http://codereview.chromium.org/2840006/diff/1/3#newcode68
client/site_tests/gl_Bench/gl_Bench.py:68: print '# no checksum found for test',
testname
On 2010/06/18 04:11:58, petkov wrote:
> logging

Done.

[petkov, 2010-06-18 20:02:54]

http://codereview.chromium.org/2840006/diff/1/2
File client/deps/glbench/src/checksums (right):

http://codereview.chromium.org/2840006/diff/1/2#newcode1
client/deps/glbench/src/checksums:1: {
On 2010/06/18 19:17:52, Alexey Marinichev wrote:
> On 2010/06/18 04:11:58, petkov wrote:
> > Chromium copyright?
> > 
> 
> I hope this will be autogenerated.  Is there an example of a copyright for the
> autogenerated file?

Not critical but if the file is checked in, I'd say it needs the Chromium
copyright.

http://codereview.chromium.org/2840006/diff/1/3
File client/site_tests/gl_Bench/gl_Bench.py (right):

http://codereview.chromium.org/2840006/diff/1/3#newcode11
client/site_tests/gl_Bench/gl_Bench.py:11: def md5_file(filename):
On 2010/06/18 19:17:52, Alexey Marinichev wrote:
> On 2010/06/18 04:11:58, petkov wrote:
> > Any reason for this to be global?
> 
> Any reason not to?

Style in general, I guess. And who knows who loads this file and how...

http://codereview.chromium.org/2840006/diff/1/3#newcode66
client/site_tests/gl_Bench/gl_Bench.py:66: keyvals[testname] = float(val) if
checksum_match else float('nan')
On 2010/06/18 19:17:52, Alexey Marinichev wrote:
> On 2010/06/18 04:11:58, petkov wrote:
> > Shouldn't this be a failure instead?
> > 
> 
> Want to log all of those before I report the failure.  Is that possible
somehow?

Not that I know of -- other than setting a flag and checking for it at the end.

[Alexey Marinichev, 2010-06-21 23:10:37]

http://codereview.chromium.org/2840006/diff/1/3
File client/site_tests/gl_Bench/gl_Bench.py (right):

http://codereview.chromium.org/2840006/diff/1/3#newcode11
client/site_tests/gl_Bench/gl_Bench.py:11: def md5_file(filename):
import gl_Bench will leave md5_file inside gl_Bench module.  from module import
* is discouraged in python anyway.  I don't see what the problem is.

On 2010/06/18 20:02:54, petkov wrote:
> On 2010/06/18 19:17:52, Alexey Marinichev wrote:
> > On 2010/06/18 04:11:58, petkov wrote:
> > > Any reason for this to be global?
> > 
> > Any reason not to?
> 
> Style in general, I guess. And who knows who loads this file and how...

[petkov, 2010-06-21 23:33:32]

lgtm if lgt piman

http://codereview.chromium.org/2840006/diff/1/3
File client/site_tests/gl_Bench/gl_Bench.py (right):

http://codereview.chromium.org/2840006/diff/1/3#newcode11
client/site_tests/gl_Bench/gl_Bench.py:11: def md5_file(filename):
On 2010/06/21 23:10:37, Alexey Marinichev wrote:
> import gl_Bench will leave md5_file inside gl_Bench module.  from module
import
> * is discouraged in python anyway.  I don't see what the problem is.
> 
> On 2010/06/18 20:02:54, petkov wrote:
> > On 2010/06/18 19:17:52, Alexey Marinichev wrote:
> > > On 2010/06/18 04:11:58, petkov wrote:
> > > > Any reason for this to be global?
> > > 
> > > Any reason not to?
> > 
> > Style in general, I guess. And who knows who loads this file and how...
> 
> 

Feel free to ignore the comment.

[piman, 2010-06-22 00:28:14]

LGTM