Add REPORTF test macro.

Add REPORTF test macro.

This macro replaces:
    SkString str;
    str.printf("Foo test Expected %d got %d", x, y);
    reporter->reportFailed(str);
with the shorter code:
    REPORTF(reporter, ("Foo test Expected %d got %d", x, y));

The new form also appends __FILE__:__LINE__ to the message before calling reportFailed().

BUG=
R=mtklein@google.com

Committed: https://code.google.com/p/skia/source/detail?r=13016

[hal.canary, 2014-01-09 23:31:56]

ptal

[bungeman-skia, 2014-01-09 23:43:56]

One early style comment, will look more later.

https://codereview.chromium.org/132843002/diff/1/tests/ImageDecodingTest.cpp
File tests/ImageDecodingTest.cpp (right):

https://codereview.chromium.org/132843002/diff/1/tests/ImageDecodingTest.cpp#...
tests/ImageDecodingTest.cpp:447: = opts.fUseRequestedColorType
With the line breaks like this, this is really hard to follow (it was before too
though). I usually format this something like

const char* colorTypeDesc = (opts.fUseRequestedColorType)
                          ? SkColorType_to_string(opts.fRequestedColorType)
                          : "(none)";

to make it a bit more obvious. Actually, we're not even making the pointer const
anyway, so why not just

const char* colorTypeDesc = "(none)";
if (opts.fUseRequestedColorType) {
    colorTypeDesc = SkColorType_to_string(opts.fRequestedColorType);
}

dunno, just as is it is difficult to read. (Same below.)

[hal.canary, 2014-01-10 13:20:33]

https://codereview.chromium.org/132843002/diff/1/tests/ImageDecodingTest.cpp
File tests/ImageDecodingTest.cpp (right):

https://codereview.chromium.org/132843002/diff/1/tests/ImageDecodingTest.cpp#...
tests/ImageDecodingTest.cpp:447: = opts.fUseRequestedColorType
On 2014/01/09 23:43:56, bungeman1 wrote:
> With the line breaks like this, this is really hard to follow (it was before
too
> though). I usually format this something like
> 
> const char* colorTypeDesc = (opts.fUseRequestedColorType)
>                           ? SkColorType_to_string(opts.fRequestedColorType)
>                           : "(none)";
> 
> to make it a bit more obvious. Actually, we're not even making the pointer
const
> anyway, so why not just
> 
> const char* colorTypeDesc = "(none)";
> if (opts.fUseRequestedColorType) {
>     colorTypeDesc = SkColorType_to_string(opts.fRequestedColorType);
> }
> 
> dunno, just as is it is difficult to read. (Same below.)

Fixed.

[mtklein, 2014-01-10 14:03:57]

https://codereview.chromium.org/132843002/diff/110001/tests/Test.h
File tests/Test.h (right):

https://codereview.chromium.org/132843002/diff/110001/tests/Test.h#newcode110
tests/Test.h:110: #define REPORTF(REPORTER, PRINTF_ARGS)                 \
Generally, arguments to macros don't need to be capitalized themselves.  I don't
mind it though if you want to keep it.

https://codereview.chromium.org/132843002/diff/110001/tests/Test.h#newcode110
tests/Test.h:110: #define REPORTF(REPORTER, PRINTF_ARGS)                 \
If you write this as

#define REPORTF(REPORTER, ...)
  do {
    SkString message;
    message.printf(__VA_ARGS__);
    ...
  } while(0)

both the macro and the calling code look cleaner.  Despite not being standard
until C++11, VA_ARGS is supported by essentially ever compiler, including all
the ones we even vaguely care about.  (It's already used in Skia codebase.)

https://codereview.chromium.org/132843002/diff/110001/tests/Test.h#newcode114
tests/Test.h:114: message.appendf(" %s:%d", __FILE__, __LINE__); \
I'd prefer file:line to come first.  It's nice to be able to look for them at a
fixed column (0), and most other log tools put them there too.

https://codereview.chromium.org/132843002/diff/110001/tests/Test.h#newcode115
tests/Test.h:115: (REPORTER)->reportFailed(message);             \
We may want to follow up and make reportFailed more generic report-y and less
fail-y.  It currently prints FAILED: <msg> in tests.  It just doesn't seem like
all REPORTF calls will be failures, no?

On the other hand, if all REPORTF calls _will_ be some sort of failure, maybe
ERRORF?

[hal.canary, 2014-01-10 14:50:31]

https://codereview.chromium.org/132843002/diff/110001/tests/Test.h
File tests/Test.h (right):

https://codereview.chromium.org/132843002/diff/110001/tests/Test.h#newcode110
tests/Test.h:110: #define REPORTF(REPORTER, PRINTF_ARGS)                 \
On 2014/01/10 14:03:58, mtklein wrote:
> Generally, arguments to macros don't need to be capitalized themselves.  I
don't
> mind it though if you want to keep it.

Done.

https://codereview.chromium.org/132843002/diff/110001/tests/Test.h#newcode110
tests/Test.h:110: #define REPORTF(REPORTER, PRINTF_ARGS)                 \
On 2014/01/10 14:03:58, mtklein wrote:
> If you write this as
> 
> #define REPORTF(REPORTER, ...)
>   do {
>     SkString message;
>     message.printf(__VA_ARGS__);
>     ...
>   } while(0)
> 
> both the macro and the calling code look cleaner.  Despite not being standard
> until C++11, VA_ARGS is supported by essentially ever compiler, including all
> the ones we even vaguely care about.  (It's already used in Skia codebase.)

Done.

https://codereview.chromium.org/132843002/diff/110001/tests/Test.h#newcode114
tests/Test.h:114: message.appendf(" %s:%d", __FILE__, __LINE__); \
On 2014/01/10 14:03:58, mtklein wrote:
> I'd prefer file:line to come first.  It's nice to be able to look for them at
a
> fixed column (0), and most other log tools put them there too.

Done.

https://codereview.chromium.org/132843002/diff/110001/tests/Test.h#newcode115
tests/Test.h:115: (REPORTER)->reportFailed(message);             \
On 2014/01/10 14:03:58, mtklein wrote:
> We may want to follow up and make reportFailed more generic report-y and less
> fail-y.  It currently prints FAILED: <msg> in tests.  It just doesn't seem
like
> all REPORTF calls will be failures, no?
> 
> On the other hand, if all REPORTF calls _will_ be some sort of failure, maybe
> ERRORF?

Done.

[mtklein, 2014-01-10 14:55:57]

On 2014/01/10 14:50:31, Hal Canary wrote:
> https://codereview.chromium.org/132843002/diff/110001/tests/Test.h
> File tests/Test.h (right):
> 
> https://codereview.chromium.org/132843002/diff/110001/tests/Test.h#newcode110
> tests/Test.h:110: #define REPORTF(REPORTER, PRINTF_ARGS)                 \
> On 2014/01/10 14:03:58, mtklein wrote:
> > Generally, arguments to macros don't need to be capitalized themselves.  I
> don't
> > mind it though if you want to keep it.
> 
> Done.
> 
> https://codereview.chromium.org/132843002/diff/110001/tests/Test.h#newcode110
> tests/Test.h:110: #define REPORTF(REPORTER, PRINTF_ARGS)                 \
> On 2014/01/10 14:03:58, mtklein wrote:
> > If you write this as
> > 
> > #define REPORTF(REPORTER, ...)
> >   do {
> >     SkString message;
> >     message.printf(__VA_ARGS__);
> >     ...
> >   } while(0)
> > 
> > both the macro and the calling code look cleaner.  Despite not being
standard
> > until C++11, VA_ARGS is supported by essentially ever compiler, including
all
> > the ones we even vaguely care about.  (It's already used in Skia codebase.)
> 
> Done.
> 
> https://codereview.chromium.org/132843002/diff/110001/tests/Test.h#newcode114
> tests/Test.h:114: message.appendf(" %s:%d", __FILE__, __LINE__); \
> On 2014/01/10 14:03:58, mtklein wrote:
> > I'd prefer file:line to come first.  It's nice to be able to look for them
at
> a
> > fixed column (0), and most other log tools put them there too.
> 
> Done.
> 
> https://codereview.chromium.org/132843002/diff/110001/tests/Test.h#newcode115
> tests/Test.h:115: (REPORTER)->reportFailed(message);             \
> On 2014/01/10 14:03:58, mtklein wrote:
> > We may want to follow up and make reportFailed more generic report-y and
less
> > fail-y.  It currently prints FAILED: <msg> in tests.  It just doesn't seem
> like
> > all REPORTF calls will be failures, no?
> > 
> > On the other hand, if all REPORTF calls _will_ be some sort of failure,
maybe
> > ERRORF?
> 
> Done.

lgtm

[hal.canary, 2014-01-10 14:58:19]

Committed patchset #4 manually as r13016 (presubmit successful).