Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(226)

Issue 132843002: Add REPORTF test macro. (Closed)

Created:
6 years, 11 months ago by hal.canary
Modified:
6 years, 11 months ago
Reviewers:
bungeman-skia, mtklein
CC:
skia-review_googlegroups.com
Visibility:
Public.

Description

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

Patch Set 1 #

Total comments: 2

Patch Set 2 : factor #

Total comments: 8

Patch Set 3 : ERRORF, __VA_ARGS__ #

Patch Set 4 : final rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+95 lines, -120 lines) Patch
M tests/BitmapCopyTest.cpp View 1 2 5 chunks +14 lines, -27 lines 0 comments Download
M tests/BlitRowTest.cpp View 1 2 2 chunks +6 lines, -8 lines 0 comments Download
M tests/DataRefTest.cpp View 1 2 1 chunk +1 line, -3 lines 0 comments Download
M tests/EmptyPathTest.cpp View 1 2 2 chunks +7 lines, -18 lines 0 comments Download
M tests/GpuBitmapCopyTest.cpp View 1 2 1 chunk +8 lines, -11 lines 0 comments Download
M tests/ImageDecodingTest.cpp View 1 2 4 chunks +28 lines, -22 lines 0 comments Download
M tests/MathTest.cpp View 1 2 1 chunk +2 lines, -4 lines 0 comments Download
M tests/PathCoverageTest.cpp View 1 2 1 chunk +4 lines, -7 lines 0 comments Download
M tests/SortTest.cpp View 1 2 1 chunk +2 lines, -3 lines 0 comments Download
M tests/StreamTest.cpp View 1 2 1 chunk +1 line, -3 lines 0 comments Download
M tests/Test.h View 1 2 1 chunk +22 lines, -14 lines 0 comments Download

Messages

Total messages: 7 (0 generated)
hal.canary
ptal
6 years, 11 months ago (2014-01-09 23:31:56 UTC) #1
bungeman-skia
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#newcode447 tests/ImageDecodingTest.cpp:447: = ...
6 years, 11 months ago (2014-01-09 23:43:56 UTC) #2
hal.canary
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#newcode447 tests/ImageDecodingTest.cpp:447: = opts.fUseRequestedColorType On 2014/01/09 23:43:56, bungeman1 wrote: > With ...
6 years, 11 months ago (2014-01-10 13:20:33 UTC) #3
mtklein
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 ...
6 years, 11 months ago (2014-01-10 14:03:57 UTC) #4
hal.canary
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: ...
6 years, 11 months ago (2014-01-10 14:50:31 UTC) #5
mtklein
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 ...
6 years, 11 months ago (2014-01-10 14:55:57 UTC) #6
hal.canary
6 years, 11 months ago (2014-01-10 14:58:19 UTC) #7
Message was sent while issue was closed.
Committed patchset #4 manually as r13016 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698