Improve table_row_nsimage_cache_unittest.mm to get 100% code coverage.

Improve table_row_nsimage_cache_unittest.mm to get 100% code coverage.

BUG=32129
TEST=Covered by unit tests.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=49688

[Nico, 2010-06-10 18:26:40]

LG

If you need stuff to hack on, just ping me… :-)

http://codereview.chromium.org/2781004/diff/1/2
File chrome/browser/cocoa/table_row_nsimage_cache_unittest.mm (right):

http://codereview.chromium.org/2781004/diff/1/2#newcode22
chrome/browser/cocoa/table_row_nsimage_cache_unittest.mm:22: 
Nit: I'd remove this newline.

http://codereview.chromium.org/2781004/diff/1/2#newcode34
chrome/browser/cocoa/table_row_nsimage_cache_unittest.mm:34:
EXPECT_TRUE(image.allocPixels());
Why not ASSERT? Can the test produce useful results after this fails?

http://codereview.chromium.org/2781004/diff/1/2#newcode40
chrome/browser/cocoa/table_row_nsimage_cache_unittest.mm:40: // test output.
Macros: Just say no.

http://codereview.chromium.org/2781004/diff/1/2#newcode101
chrome/browser/cocoa/table_row_nsimage_cache_unittest.mm:101:
rows->insert(rows->begin() + 1, MakeImage(25, 25));
Why not `(*rows)[1] = MakeImage(25, 25);`?

http://codereview.chromium.org/2781004/diff/1/2#newcode110
chrome/browser/cocoa/table_row_nsimage_cache_unittest.mm:110:
rows->insert(rows->begin(), MakeImage(15, 15));
likewise

http://codereview.chromium.org/2781004/diff/1/2#newcode141
chrome/browser/cocoa/table_row_nsimage_cache_unittest.mm:141:
COMPARE_SK_NS_IMG_SIZES(rows->at(1), image1);
Couldn't/shouldn't you even check the pointers are equal?

[Robert Sesek, 2010-06-10 19:10:09]

http://codereview.chromium.org/2781004/diff/1/2
File chrome/browser/cocoa/table_row_nsimage_cache_unittest.mm (right):

http://codereview.chromium.org/2781004/diff/1/2#newcode34
chrome/browser/cocoa/table_row_nsimage_cache_unittest.mm:34:
EXPECT_TRUE(image.allocPixels());
On 2010/06/10 18:26:40, Nico wrote:
> Why not ASSERT? Can the test produce useful results after this fails?

That wasn't compiling for some reason. I think the compiler got confused. It
works now. *shrugs*

http://codereview.chromium.org/2781004/diff/1/2#newcode40
chrome/browser/cocoa/table_row_nsimage_cache_unittest.mm:40: // test output.
On 2010/06/10 18:26:40, Nico wrote:
> Macros: Just say no.

Going to keep this as a macro for the reason that it states: we lose variable
names if this were a function. And those names make it much easier to see what's
failing in the gtest output.

http://codereview.chromium.org/2781004/diff/1/2#newcode101
chrome/browser/cocoa/table_row_nsimage_cache_unittest.mm:101:
rows->insert(rows->begin() + 1, MakeImage(25, 25));
On 2010/06/10 18:26:40, Nico wrote:
> Why not `(*rows)[1] = MakeImage(25, 25);`?

Much better.

http://codereview.chromium.org/2781004/diff/1/2#newcode141
chrome/browser/cocoa/table_row_nsimage_cache_unittest.mm:141:
COMPARE_SK_NS_IMG_SIZES(rows->at(1), image1);
On 2010/06/10 18:26:40, Nico wrote:
> Couldn't/shouldn't you even check the pointers are equal?

Done.