Set limit for number of failed EXPECTs per test in test launcher output.

Set limit for number of failed EXPECTs per test in test launcher output.

Some tests may contain broken EXPECT with a large message inside a loop
with several thousands iterations and produce huge output (several
gigabytes). Limit number of serialized test result parts to prevent it.

R=phajdan.jr
BUG=

Review-Url: https://codereview.chromium.org/2846673004
Cr-Commit-Position: refs/heads/master@{#471766}
Committed: https://chromium.googlesource.com/chromium/src/+/452ecc0e7cf545a650047227a7e9ce4702ed227d

[alex-ac, 2017-04-27 10:58:44]

Description was changed from

==========
Set limit for number of failed EXPECTs per test in test launcher output.

Some tests may contain broken EXPECT with a large message inside a loop
with several thousands iterations and produce huge output (several
gigabytes). Limit number of serialized test result parts to prevent it.

R=phajdan.jr
BUG=
==========

to

==========
Set limit for number of failed EXPECTs per test in test launcher output.

Some tests may contain broken EXPECT with a large message inside a loop
with several thousands iterations and produce huge output (several
gigabytes). Limit number of serialized test result parts to prevent it.

R=phajdan.jr
BUG=
==========

[alex-ac, 2017-04-27 10:58:44]

alex-ac@yandex-team.ru changed reviewers:
+ phajdan.jr@chromium.org

[alex-ac, 2017-05-01 09:25:39]

Pawel Hajdan Jr, PTAL.

[Paweł Hajdan Jr., 2017-05-05 17:33:03]

https://codereview.chromium.org/2846673004/diff/1/base/test/gtest_xml_unittes...
File base/test/gtest_xml_unittest_result_printer.cc (right):

https://codereview.chromium.org/2846673004/diff/1/base/test/gtest_xml_unittes...
base/test/gtest_xml_unittest_result_printer.cc:86: for (int i = 0; i < limit;
++i) {
We need to add something in case the limit is exceeded, to avoid a
silent-failure-like condition.

https://codereview.chromium.org/2846673004/diff/1/base/test/gtest_xml_unittes...
File base/test/gtest_xml_unittest_result_printer.h (right):

https://codereview.chromium.org/2846673004/diff/1/base/test/gtest_xml_unittes...
base/test/gtest_xml_unittest_result_printer.h:30: void
set_test_part_results_limit(int limit) {
Instead of a setter, can we make this class read corresponding command-line flag
directly?

https://codereview.chromium.org/2846673004/diff/1/base/test/test_suite.cc
File base/test/test_suite.cc (right):

https://codereview.chromium.org/2846673004/diff/1/base/test/test_suite.cc#new...
base/test/test_suite.cc:243: if (CommandLine::ForCurrentProcess()->HasSwitch(
Can we just move this code to the printer?

[alex-ac, 2017-05-07 12:07:24]

https://codereview.chromium.org/2846673004/diff/1/base/test/gtest_xml_unittes...
File base/test/gtest_xml_unittest_result_printer.cc (right):

https://codereview.chromium.org/2846673004/diff/1/base/test/gtest_xml_unittes...
base/test/gtest_xml_unittest_result_printer.cc:86: for (int i = 0; i < limit;
++i) {
On 2017/05/05 17:33:03, Paweł Hajdan Jr. wrote:
> We need to add something in case the limit is exceeded, to avoid a
> silent-failure-like condition.

Done.

https://codereview.chromium.org/2846673004/diff/1/base/test/gtest_xml_unittes...
File base/test/gtest_xml_unittest_result_printer.h (right):

https://codereview.chromium.org/2846673004/diff/1/base/test/gtest_xml_unittes...
base/test/gtest_xml_unittest_result_printer.h:30: void
set_test_part_results_limit(int limit) {
On 2017/05/05 17:33:03, Paweł Hajdan Jr. wrote:
> Instead of a setter, can we make this class read corresponding command-line
flag
> directly?

Done.

https://codereview.chromium.org/2846673004/diff/1/base/test/test_suite.cc
File base/test/test_suite.cc (right):

https://codereview.chromium.org/2846673004/diff/1/base/test/test_suite.cc#new...
base/test/test_suite.cc:243: if (CommandLine::ForCurrentProcess()->HasSwitch(
On 2017/05/05 17:33:03, Paweł Hajdan Jr. wrote:
> Can we just move this code to the printer?

Done.

[Paweł Hajdan Jr., 2017-05-08 17:38:03]

Thanks. Just one tiny comment.

https://codereview.chromium.org/2846673004/diff/20001/base/test/gtest_xml_uni...
File base/test/gtest_xml_unittest_result_printer.h (right):

https://codereview.chromium.org/2846673004/diff/20001/base/test/gtest_xml_uni...
base/test/gtest_xml_unittest_result_printer.h:54: int test_part_results_limit_;
Is it worth to add a member variable just for this?

It seems to me any gains from "caching" it are negligible.

[alex-ac, 2017-05-12 12:07:44]

https://codereview.chromium.org/2846673004/diff/20001/base/test/gtest_xml_uni...
File base/test/gtest_xml_unittest_result_printer.h (right):

https://codereview.chromium.org/2846673004/diff/20001/base/test/gtest_xml_uni...
base/test/gtest_xml_unittest_result_printer.h:54: int test_part_results_limit_;
On 2017/05/08 17:38:03, Paweł Hajdan Jr. wrote:
> Is it worth to add a member variable just for this?
> 
> It seems to me any gains from "caching" it are negligible.

Done.

[Paweł Hajdan Jr., 2017-05-15 12:39:00]

LGTM w/nit

Thanks!

https://codereview.chromium.org/2846673004/diff/40001/base/test/gtest_xml_uni...
File base/test/gtest_xml_unittest_result_printer.cc (right):

https://codereview.chromium.org/2846673004/diff/40001/base/test/gtest_xml_uni...
base/test/gtest_xml_unittest_result_printer.cc:22: }
nit: // namespace

[alex-ac, 2017-05-15 12:53:16]

https://codereview.chromium.org/2846673004/diff/40001/base/test/gtest_xml_uni...
File base/test/gtest_xml_unittest_result_printer.cc (right):

https://codereview.chromium.org/2846673004/diff/40001/base/test/gtest_xml_uni...
base/test/gtest_xml_unittest_result_printer.cc:22: }
On 2017/05/15 12:39:00, Paweł Hajdan Jr. wrote:
> nit: // namespace

Done.

[alex-ac, 2017-05-15 12:53:23]

The CQ bit was checked by alex-ac@yandex-team.ru

[alex-ac, 2017-05-15 12:53:23]

The patchset sent to the CQ was uploaded after l-g-t-m from
phajdan.jr@chromium.org
Link to the patchset: https://codereview.chromium.org/2846673004/#ps60001
(title: "Fix nit.")

[commit-bot: I haz the power, 2017-05-15 12:53:35]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-15 13:01:04]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-05-15 13:01:05]

Try jobs failed on following builders:
  ios-device-xcode-clang on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device-xcode-...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[alex-ac, 2017-05-15 13:04:24]

The CQ bit was checked by alex-ac@yandex-team.ru

[alex-ac, 2017-05-15 13:04:24]

The patchset sent to the CQ was uploaded after l-g-t-m from
phajdan.jr@chromium.org
Link to the patchset: https://codereview.chromium.org/2846673004/#ps80001
(title: "Fix missed constant usage.")

[commit-bot: I haz the power, 2017-05-15 13:04:37]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-05-15 15:08:00]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1494853464089290,
"parent_rev": "7c66cb42c5d8bb5f5b80a6456ab1d1b0dd259604", "commit_rev":
"452ecc0e7cf545a650047227a7e9ce4702ed227d"}

[commit-bot: I haz the power, 2017-05-15 15:08:20]

Description was changed from

==========
Set limit for number of failed EXPECTs per test in test launcher output.

Some tests may contain broken EXPECT with a large message inside a loop
with several thousands iterations and produce huge output (several
gigabytes). Limit number of serialized test result parts to prevent it.

R=phajdan.jr
BUG=
==========

to

==========
Set limit for number of failed EXPECTs per test in test launcher output.

Some tests may contain broken EXPECT with a large message inside a loop
with several thousands iterations and produce huge output (several
gigabytes). Limit number of serialized test result parts to prevent it.

R=phajdan.jr
BUG=

Review-Url: https://codereview.chromium.org/2846673004
Cr-Commit-Position: refs/heads/master@{#471766}
Committed:
https://chromium.googlesource.com/chromium/src/+/452ecc0e7cf545a650047227a7e9...
==========

[commit-bot: I haz the power, 2017-05-15 15:08:23]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/452ecc0e7cf545a650047227a7e9...