[ABANDONED] Refactor CertVerifyProcWeakDigestTest.

****
ABANDONED
This has been superseded by https://codereview.chromium.org/2627523002/
****

Refactor CertVerifyProcWeakDigestTest.

Replaces the verification tests (which didn't run consistently across
platforms) with a unit-test of the underlying helper function.

BUG=649017

[eroman, 2017-01-06 02:04:53]

The CQ bit was checked by eroman@chromium.org to run a CQ dry run

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

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[eroman, 2017-01-06 02:05:50]

eroman@chromium.org changed reviewers:
+ rsleevi@chromium.org

[eroman, 2017-01-06 02:06:09]

Description was changed from

==========
Refactor the CertVerifyProcWeakDigestTest.

Replaces the verification tests (which didn't run consistently across
platoforms) with a unit-test of the underlying helper function.

BUG=649017
==========

to

==========
Refactor CertVerifyProcWeakDigestTest.

Replaces the verification tests (which didn't run consistently across
platforms) with a unit-test of the underlying helper function.

BUG=649017
==========

[Ryan Sleevi, 2017-01-06 02:27:14]

https://codereview.chromium.org/2620443002/diff/1/net/cert/cert_verify_proc_u...
File net/cert/cert_verify_proc_unittest.cc (right):

https://codereview.chromium.org/2620443002/diff/1/net/cert/cert_verify_proc_u...
net/cert/cert_verify_proc_unittest.cc:1688: TEST_P(CertVerifyProcWeakDigestTest,
VerifyDetectsAlgorithm) {
So, a few behaviour changes worth calling out:
1) Not testing Verify() is actually calling this
2) Not testing intermediates (are handled) or that roots (are ignored)

In effect, this is now just testing FillCertVerifyResultWeakSignature().

I'm wondering if we can't do something like:

(Keep all of lines 1706-1732 of the original)
ASSERT_TRUE(verify_result.verified_cert);
bool has_chain =
!verify_result.verified_cert->GetIntermediateCertificates().empty();
bool expect_has_md2 =
  (data.expected_leaf_algorithm == X509Certificate::kSignatureHashAlgorithmMd2)
||
  (has_chain && data.expected_chain_algorithm ==
X509Certificate::kSignatureHashAlgorithmMd2);
...
bool expect_has_sha1_leaf =
  (data.expected_leaf_algorithm == ...);


It may be that the above change only works if we uplift to
CertVerifyProc::Verify() rather than in the VerifyInternal()s that it is today,
because there may be short-circuits on the CertVerifyResult in the case of
failure (... probably).

If that's the case though, then I guess it's still blocked on the further
cleanup, because I'm a little nervous about any gap where we lose test coverage.

https://codereview.chromium.org/2620443002/diff/1/net/cert/cert_verify_proc_u...
net/cert/cert_verify_proc_unittest.cc:1724: //               applicable.
I'm not sure why this TODO?

[commit-bot: I haz the power, 2017-01-06 03:00:11]

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

[commit-bot: I haz the power, 2017-01-06 03:00:12]

Dry run: This issue passed the CQ dry run.

[eroman, 2017-01-10 19:14:04]

For the record, this change has been abandoned in favor of
https://codereview.chromium.org/2627523002/.

[eroman, 2017-01-10 19:14:51]

Description was changed from

==========
Refactor CertVerifyProcWeakDigestTest.

Replaces the verification tests (which didn't run consistently across
platforms) with a unit-test of the underlying helper function.

BUG=649017
==========

to

==========
****
ABANDONED
This has been superseded by https://codereview.chromium.org/2627523002/
****

Refactor CertVerifyProcWeakDigestTest.

Replaces the verification tests (which didn't run consistently across
platforms) with a unit-test of the underlying helper function.

BUG=649017
==========