Unittest fixes.

Unittest fixes.

Adjusts the expected unittest output.

BUG= None
R=kschimpf@google.com, stichnot@chromium.org

Committed: https://gerrit.chromium.org/gerrit/gitweb?p=native_client/pnacl-subzero.git;a=commit;h=8eefffade3f1a44af97ad0824328ff0910dcee5e

[John, 2015-06-11 18:38:53]

jpp@chromium.org changed reviewers:
+ kschimpf@chromium.org, stichnot@chromium.org

[Jim Stichnoth, 2015-06-11 19:07:14]

https://codereview.chromium.org/1173353003/diff/1/unittest/IceParseInstsTest.cpp
File unittest/IceParseInstsTest.cpp (right):

https://codereview.chromium.org/1173353003/diff/1/unittest/IceParseInstsTest....
unittest/IceParseInstsTest.cpp:60:
EXPECT_EQ("\t.type\t__Sz_block_profile_info,@object\n"
Can you factor this string into a const char *, or a macro if necessary?

[Karl, 2015-06-11 19:58:54]

kschimpf@google.com changed reviewers:
+ kschimpf@google.com

[Karl, 2015-06-11 19:58:54]

https://codereview.chromium.org/1173353003/diff/1/unittest/IceParseInstsTest.cpp
File unittest/IceParseInstsTest.cpp (right):

https://codereview.chromium.org/1173353003/diff/1/unittest/IceParseInstsTest....
unittest/IceParseInstsTest.cpp:59: EXPECT_FALSE(Munger.runTest());
Why not add an (optional) flag to runTest() in (unittest/BitcodeMunge.h) to
disable translation. Then, for cases with parse errors, pass in the disable flag
as an argument. This should keep from having to add this (unconditional) code to
results returned by getTestResults().

[John, 2015-06-11 20:42:09]

https://codereview.chromium.org/1173353003/diff/1/unittest/IceParseInstsTest.cpp
File unittest/IceParseInstsTest.cpp (right):

https://codereview.chromium.org/1173353003/diff/1/unittest/IceParseInstsTest....
unittest/IceParseInstsTest.cpp:59: EXPECT_FALSE(Munger.runTest());
On 2015/06/11 19:58:54, Karl wrote:
> Why not add an (optional) flag to runTest() in (unittest/BitcodeMunge.h) to
> disable translation. Then, for cases with parse errors, pass in the disable
flag
> as an argument. This should keep from having to add this (unconditional) code
to
> results returned by getTestResults().

There was precedent to doing this (e.g., lines 52-55.) Thanks for the
suggestion.

https://codereview.chromium.org/1173353003/diff/1/unittest/IceParseInstsTest....
unittest/IceParseInstsTest.cpp:60:
EXPECT_EQ("\t.type\t__Sz_block_profile_info,@object\n"
On 2015/06/11 19:07:14, stichnot wrote:
> Can you factor this string into a const char *, or a macro if necessary?

Not easily. EXPECT_EQ expands to another another macro, and macro substitution
is limited to two (?)... Anyway, I added the ParseError parameter, so this no
longer applies.

[Karl, 2015-06-11 20:52:59]

lgtm

[Jim Stichnoth, 2015-06-11 20:57:27]

LGTM, except I'd prefer different var names than ParseError.

[John, 2015-06-11 22:14:41]

Committed patchset #2 (id:20001) manually as
8eefffade3f1a44af97ad0824328ff0910dcee5e (presubmit successful).