LOG(INFO) tidying in net/

LOG(INFO) tidying in net/

Some deletions, some to VLOG.

R=rsleevi@chromium.org
BUG=322805
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=237095

[scottmg, 2013-11-23 01:29:50]

Straw man, let me know if you prefer some of the test ones to stay INFO.

[Ryan Sleevi, 2013-11-25 01:08:32]

Adding rch, akalin, rtenneti, and szym as cc for FYI
rch/rtenneti: because of the quic/ shared code
akalin: because of the spdy/ shared code
szym: because of the DNS bits

This LGTM. Apologies for making requests in a clean-up CL, but there were a few
things better left as LOG(INFO), since they're related to explaining that a test
is skipped.

There's no good way in GTest to document this. Of course, perhaps I'm
over-thinking the value of saying "skipped test", since if it passed vs skipped,
no one's going to look at it. Use your own discretion; either removing the bits
or changing them back to LOG(INFO).

I'd love to see rch@/rtenneti@ look more into the QUIC/SPDY logging. It seems
that a terrible amount of the code is intended as 'printf debugging' for
bootstrapping new tests, but that seems an unnecessarily high cost. Your changes
to make them vlog are fine, I just wanted to make sure they'd own the follow-up.

https://codereview.chromium.org/82913011/diff/1/net/android/keystore_unittest.cc
File net/android/keystore_unittest.cc (left):

https://codereview.chromium.org/82913011/diff/1/net/android/keystore_unittest...
net/android/keystore_unittest.cc:579: LOG(INFO) << "This test can't run on
Android < 4.2";
Let's keep this as LOG(INFO)

https://codereview.chromium.org/82913011/diff/1/net/base/int128_unittest.cc
File net/base/int128_unittest.cc (left):

https://codereview.chromium.org/82913011/diff/1/net/base/int128_unittest.cc#o...
net/base/int128_unittest.cc:114: LOG(INFO) << big_minus_one;
just delete these?

https://codereview.chromium.org/82913011/diff/1/net/cert/cert_verify_proc_uni...
File net/cert/cert_verify_proc_unittest.cc (left):

https://codereview.chromium.org/82913011/diff/1/net/cert/cert_verify_proc_uni...
net/cert/cert_verify_proc_unittest.cc:837: LOG(INFO) << "Skipping this test in
this platform.";
Keep this as LOG(INFO)

https://codereview.chromium.org/82913011/diff/1/net/cert/nss_cert_database_un...
File net/cert/nss_cert_database_unittest.cc (left):

https://codereview.chromium.org/82913011/diff/1/net/cert/nss_cert_database_un...
net/cert/nss_cert_database_unittest.cc:932: LOG(INFO) << "test skipped on NSS <
3.13";
and these

https://codereview.chromium.org/82913011/diff/1/net/cert/test_root_certs_unit...
File net/cert/test_root_certs_unittest.cc (left):

https://codereview.chromium.org/82913011/diff/1/net/cert/test_root_certs_unit...
net/cert/test_root_certs_unittest.cc:74: LOG(INFO) << "Skipping test for NSS
3.14.2 - NSS 3.15";
and this

https://codereview.chromium.org/82913011/diff/1/net/cert/x509_util_ios.cc
File net/cert/x509_util_ios.cc (right):

https://codereview.chromium.org/82913011/diff/1/net/cert/x509_util_ios.cc#new...
net/cert/x509_util_ios.cc:104: << "Could not convert SecCertificateRef to
CERTCertificate*";
Let's delete this.

It should only come up in weird user installs, and the DLOG is not going to help
at all.

[scottmg, 2013-11-25 17:12:38]

I'm personally inclined to delete all the "skipped" since as you say, no one is
going to look at them until they care. But I put them back in any case.

If you think these are important enough to keep as LOG(INFO) then perhaps we
should have a TESTLOG << "..." or something to document and avoid presubmit
warnings and CQ failures.

https://codereview.chromium.org/82913011/diff/1/net/android/keystore_unittest.cc
File net/android/keystore_unittest.cc (left):

https://codereview.chromium.org/82913011/diff/1/net/android/keystore_unittest...
net/android/keystore_unittest.cc:579: LOG(INFO) << "This test can't run on
Android < 4.2";
On 2013/11/25 01:08:32, Ryan Sleevi wrote:
> Let's keep this as LOG(INFO)

Done.

https://codereview.chromium.org/82913011/diff/1/net/base/int128_unittest.cc
File net/base/int128_unittest.cc (left):

https://codereview.chromium.org/82913011/diff/1/net/base/int128_unittest.cc#o...
net/base/int128_unittest.cc:114: LOG(INFO) << big_minus_one;
On 2013/11/25 01:08:32, Ryan Sleevi wrote:
> just delete these?

Done.

https://codereview.chromium.org/82913011/diff/1/net/cert/cert_verify_proc_uni...
File net/cert/cert_verify_proc_unittest.cc (left):

https://codereview.chromium.org/82913011/diff/1/net/cert/cert_verify_proc_uni...
net/cert/cert_verify_proc_unittest.cc:837: LOG(INFO) << "Skipping this test in
this platform.";
On 2013/11/25 01:08:32, Ryan Sleevi wrote:
> Keep this as LOG(INFO)

Done.

https://codereview.chromium.org/82913011/diff/1/net/cert/nss_cert_database_un...
File net/cert/nss_cert_database_unittest.cc (left):

https://codereview.chromium.org/82913011/diff/1/net/cert/nss_cert_database_un...
net/cert/nss_cert_database_unittest.cc:932: LOG(INFO) << "test skipped on NSS <
3.13";
On 2013/11/25 01:08:32, Ryan Sleevi wrote:
> and these

Done.

https://codereview.chromium.org/82913011/diff/1/net/cert/test_root_certs_unit...
File net/cert/test_root_certs_unittest.cc (left):

https://codereview.chromium.org/82913011/diff/1/net/cert/test_root_certs_unit...
net/cert/test_root_certs_unittest.cc:74: LOG(INFO) << "Skipping test for NSS
3.14.2 - NSS 3.15";
On 2013/11/25 01:08:32, Ryan Sleevi wrote:
> and this

Done.

https://codereview.chromium.org/82913011/diff/1/net/cert/x509_util_ios.cc
File net/cert/x509_util_ios.cc (right):

https://codereview.chromium.org/82913011/diff/1/net/cert/x509_util_ios.cc#new...
net/cert/x509_util_ios.cc:104: << "Could not convert SecCertificateRef to
CERTCertificate*";
On 2013/11/25 01:08:32, Ryan Sleevi wrote:
> Let's delete this.
> 
> It should only come up in weird user installs, and the DLOG is not going to
help
> at all.

Done.

[scottmg, 2013-11-25 17:13:09]

(Alternatively, maybe we should exclude LOG(INFO) checking from test files in
the PRESUBMIT check.)

[scottmg, 2013-11-25 17:16:11]

Based on LGTM + adding others as cc-only, I'm going to assume this is OK to
commit. If someone doesn't like something, feel free to either fix, or ping me
and I'll do a followup.

[szym, 2013-11-25 17:19:00]

https://codereview.chromium.org/82913011/diff/60001/net/tools/dns_fuzz_stub/d...
File net/tools/dns_fuzz_stub/dns_fuzz_stub.cc (right):

https://codereview.chromium.org/82913011/diff/60001/net/tools/dns_fuzz_stub/d...
net/tools/dns_fuzz_stub/dns_fuzz_stub.cc:129: VLOG(0) << "Query: id=" << id_int
<< ", "
Don't know if the fuzzer framework parses the log output, so cc: ttuttle

[scottmg, 2013-11-25 17:20:08]

https://codereview.chromium.org/82913011/diff/60001/net/tools/dns_fuzz_stub/d...
File net/tools/dns_fuzz_stub/dns_fuzz_stub.cc (right):

https://codereview.chromium.org/82913011/diff/60001/net/tools/dns_fuzz_stub/d...
net/tools/dns_fuzz_stub/dns_fuzz_stub.cc:129: VLOG(0) << "Query: id=" << id_int
<< ", "
On 2013/11/25 17:19:00, szym wrote:
> Don't know if the fuzzer framework parses the log output, so cc: ttuttle

I'll revert the changes in this file.

[scottmg, 2013-11-25 17:23:47]

Committed patchset #3 manually as r237095 (presubmit successful).

[akalin, 2013-11-25 18:58:46]

On 2013/11/25 17:23:47, scottmg wrote:
> Committed patchset #3 manually as r237095 (presubmit successful).

uh, you guys know that VLOG(0) is exactly the same as LOG(INFO) right? Perhaps
you meant VLOG(1).

[scottmg, 2013-11-25 19:20:47]

On 2013/11/25 18:58:46, akalin wrote:
> On 2013/11/25 17:23:47, scottmg wrote:
> > Committed patchset #3 manually as r237095 (presubmit successful).
> 
> uh, you guys know that VLOG(0) is exactly the same as LOG(INFO) right? Perhaps
> you meant VLOG(1).

Sigh.

Well, I guess they're different in that the presubmit doesn't catch them. :(

[scottmg, 2013-11-25 19:21:21]

On 2013/11/25 19:20:47, scottmg wrote:
> On 2013/11/25 18:58:46, akalin wrote:
> > On 2013/11/25 17:23:47, scottmg wrote:
> > > Committed patchset #3 manually as r237095 (presubmit successful).
> > 
> > uh, you guys know that VLOG(0) is exactly the same as LOG(INFO) right?
Perhaps
> > you meant VLOG(1).
> 
> Sigh.
> 
> Well, I guess they're different in that the presubmit doesn't catch them. :(

I guess I'll just revert this and we can do something else later.