Disable SpdyCredentialBuilder tests with OpenSSL.

net/spdy/spdy_credential_builder_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/spdy/spdy_credential_builder.h"
 
 #include "base/threading/sequenced_worker_pool.h"
 #include "crypto/ec_signature_creator.h"
 #include "crypto/ec_private_key.h"
 #include "net/base/asn1_util.h"
@@ skipping 57 matching lines @@
     return SpdyCredentialBuilder::GetCredentialSecret(
         MockClientSocket::kTlsUnique);
   }
 
   SpdyTestStateHelper helper_;  // Provides deterministic EC signatures.
   std::string cert_;
   std::string key_;
   SpdyCredential credential_;
 };
 
-TEST_F(SpdyCredentialBuilderTest, GetCredentialSecret) {
+// http://crbug.com/142833. The following tests fail on Linux redux which uses
+// OpenSSL due to the unimplemented ec_private_key_openssl.cc.

[Ryan Sleevi, 2012/08/16 17:34:05]

Is there a reason you're using OS_LINUX here? Android also uses OpenSSL, and
also is unimplemented there.

Also, the Chromium style guide recommends MAYBE_ as the prefix here -
http://www.chromium.org/developers/tree-sheriffs/sheriff-details-chromium/han...

Yes, this means duplicating the #ifdef between the code, but this is the
existing pattern in Chromium code.

[Philippe, 2012/08/17 09:10:34]

Android had its own way of disabling this test through an entry in
build/android/gtest_filter/net_unittests_disabled. I removed it from there and
made MAYBE_ depend only on USE_OPENSSL here.

I also updated the code by following the link you pointed out.

Also, would it be reasonable to keep the old approach by renaming
DISABLED_ON_LINUX_REDUX() to MAYBE() (although this could lead to name clashes)
to avoid duplicating all this code?

[Ryan Sleevi, 2012/08/17 17:17:25]

The use of macro functions tends to harm readability - because it adds an extra
layer of indirection that has to be followed by the caller. This is especially
true with stringizing macros, since it can be hard to find the final string.

I think how you've written it is fine, and makes it clear at each test site
which tests are disabled on which platforms. This is also important because it
still leaves the possibility that individual tests can be disabled on other
platforms as necessary.

+#if defined(OS_LINUX) && defined(USE_OPENSSL)
+#define DISABLED_ON_LINUX_REDUX(Test) DISABLED_##Test
+#else
+#define DISABLED_ON_LINUX_REDUX(Test) Test
+#endif
+
+TEST_F(SpdyCredentialBuilderTest,
+       DISABLED_ON_LINUX_REDUX(GetCredentialSecret)) {
   std::string secret_str(kSecretPrefix, arraysize(kSecretPrefix));
   secret_str.append(MockClientSocket::kTlsUnique);
 
   EXPECT_EQ(secret_str, GetCredentialSecret());
 }
 
-TEST_F(SpdyCredentialBuilderTest, SucceedsWithECDSACert) {
+TEST_F(SpdyCredentialBuilderTest,
+       DISABLED_ON_LINUX_REDUX(SucceedsWithECDSACert)) {
   EXPECT_EQ(OK, BuildWithType(CLIENT_CERT_ECDSA_SIGN));
 }
 
-TEST_F(SpdyCredentialBuilderTest, FailsWithRSACert) {
+TEST_F(SpdyCredentialBuilderTest, DISABLED_ON_LINUX_REDUX(FailsWithRSACert)) {
   EXPECT_EQ(ERR_BAD_SSL_CLIENT_AUTH_CERT,
             BuildWithType(CLIENT_CERT_RSA_SIGN));
 }
 
-TEST_F(SpdyCredentialBuilderTest, SetsSlotCorrectly) {
+TEST_F(SpdyCredentialBuilderTest, DISABLED_ON_LINUX_REDUX(SetsSlotCorrectly)) {
   ASSERT_EQ(OK, Build());
   EXPECT_EQ(kSlot, credential_.slot);
 }
 
-TEST_F(SpdyCredentialBuilderTest, SetsCertCorrectly) {
+TEST_F(SpdyCredentialBuilderTest, DISABLED_ON_LINUX_REDUX(SetsCertCorrectly)) {
   ASSERT_EQ(OK, Build());
   base::StringPiece spki;
   ASSERT_TRUE(asn1::ExtractSPKIFromDERCert(cert_, &spki));
   base::StringPiece spk;
   ASSERT_TRUE(asn1::ExtractSubjectPublicKeyFromSPKI(spki, &spk));
   EXPECT_EQ(1u, credential_.certs.size());
   EXPECT_EQ(0, (int)spk[0]);
   EXPECT_EQ(4, (int)spk[1]);
   EXPECT_EQ(spk.substr(2, spk.length()).as_string(), credential_.certs[0]);
 }
 
-TEST_F(SpdyCredentialBuilderTest, SetsProofCorrectly) {
+TEST_F(SpdyCredentialBuilderTest, DISABLED_ON_LINUX_REDUX(SetsProofCorrectly)) {
   ASSERT_EQ(OK, Build());
   base::StringPiece spki;
   ASSERT_TRUE(asn1::ExtractSPKIFromDERCert(cert_, &spki));
   std::vector<uint8> spki_data(spki.data(),
                                spki.data() + spki.size());
   std::vector<uint8> key_data(key_.data(),
                               key_.data() + key_.length());
   std::vector<uint8> proof_data;
   scoped_ptr<crypto::ECPrivateKey> private_key(
       crypto::ECPrivateKey::CreateFromEncryptedPrivateKeyInfo(
           ServerBoundCertService::kEPKIPassword, key_data, spki_data));
   scoped_ptr<crypto::ECSignatureCreator> creator(
       crypto::ECSignatureCreator::Create(private_key.get()));
   std::string secret = GetCredentialSecret();
   creator->Sign(reinterpret_cast<const unsigned char *>(secret.data()),
                 secret.length(), &proof_data);
 
   std::string proof(proof_data.begin(), proof_data.end());
   EXPECT_EQ(proof, credential_.proof);
 }
 
+#undef DISABLED_ON_LINUX_REDUX
+
 }  // namespace net