Do a cursory conversion of a few tests to GTest.
For now, this is the laziest conversion possible. The intent is to just
get the build setup ready so that we can get everything working in our
consumers. The intended end state is:
- The standalone build produces three test targets, one per library:
{crypto,ssl,decrepit}_tests.
- Each FOO_test is made up of:
FOO/**/*_test.cc
crypto/test/gtest_main.cc
test_support
- generate_build_files.py emits variables crypto_test_sources and
ssl_test_sources. These variables are populated with FindCFiles,
looking for *_test.cc.
- The consuming file assembles those variables into the two test targets
(plus decrepit) from there. This avoids having generate_build_files.py
emit actual build rules.
- Our standalone builders, Chromium, and Android just run the top-level
test targets using whatever GTest-based reporting story they have.
In transition, we start by converting one of two tests in each library
to populate the three test targets. Those are added to all_tests.json
and all_tests.go hacked to handle them transparently. This keeps our
standalone builder working.
generate_build_files.py, to start with, populates the new source lists
manually and subtracts them out of the old machinery. We emit both for
the time being. When this change rolls in, we'll write all the build
glue needed to build the GTest-based tests and add it to consumers'
continuous builders.
Next, we'll subsume a file-based test and get the consumers working with
that. (I.e. make sure the GTest targets can depend on a data file.)
Once that's all done, we'll be sure all this will work. At that point,
we start subsuming the remaining tests into the GTest targets and,
asynchronously, rewriting tests to use GTest properly rather than
cursory conversion here.
When all non-GTest tests are gone, the old generate_build_files.py hooks
will be removed, consumers updated to not depend on them, and standalone
builders converted to not rely on all_tests.go, which can then be
removed. (Unless bits end up being needed as a malloc test driver. I'm
thinking we'll want to do something with --gtest_filter.)
As part of this CL, I've bumped the CMake requirements (for
target_include_directories) and added a few suppressions for warnings
that GTest doesn't pass.
BUG=129
Change-Id: I881b26b07a8739cc0b52dbb51a30956908e1b71a
Reviewed-on: https://boringssl-review.googlesource.com/13232
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/BUILDING.md b/BUILDING.md
index c87e8b5..e691afd 100644
--- a/BUILDING.md
+++ b/BUILDING.md
@@ -2,7 +2,7 @@
## Build Prerequisites
- * [CMake](https://cmake.org/download/) 2.8.10 or later is required.
+ * [CMake](https://cmake.org/download/) 2.8.11 or later is required.
* Perl 5.6.1 or later is required. On Windows,
[Active State Perl](http://www.activestate.com/activeperl/) has been
diff --git a/CMakeLists.txt b/CMakeLists.txt
index b194160..fbee64a 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -1,4 +1,4 @@
-cmake_minimum_required (VERSION 2.8.10)
+cmake_minimum_required (VERSION 2.8.11)
# Defer enabling C and CXX languages.
project (BoringSSL NONE)
@@ -43,6 +43,9 @@
endif()
elseif(MSVC)
set(MSVC_DISABLED_WARNINGS_LIST
+ "C4061" # enumerator 'identifier' in switch of enum 'enumeration' is not
+ # explicitly handled by a case label
+ # Disable this because it flags even when there is a default.
"C4100" # 'exarg' : unreferenced formal parameter
"C4127" # conditional expression is constant
"C4200" # nonstandard extension used : zero-sized array in
@@ -78,12 +81,16 @@
# copy constructor is inaccessible or deleted
"C4626" # assignment operator could not be generated because a base class
# assignment operator is inaccessible or deleted
+ "C4668" # 'symbol' is not defined as a preprocessor macro, replacing with
+ # '0' for 'directives'
+ # Disable this because GTest uses it everywhere.
"C4706" # assignment within conditional expression
"C4710" # 'function': function not inlined
"C4711" # function 'function' selected for inline expansion
"C4800" # 'int' : forcing value to bool 'true' or 'false'
# (performance warning)
"C4820" # 'bytes' bytes padding added after construct 'member_name'
+ "C5026" # move constructor was implicitly defined as deleted
"C5027" # move assignment operator was implicitly defined as deleted
)
set(MSVC_LEVEL4_WARNINGS_LIST
@@ -228,6 +235,17 @@
set(ARCH "generic")
endif()
+# Add minimal googletest targets. The provided one has many side-effects, and
+# googletest has a very straightforward build.
+add_library(gtest third_party/googletest/src/gtest-all.cc)
+target_include_directories(gtest PRIVATE third_party/googletest)
+if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang")
+ # TODO(davidben): Make googletest pass -Wmissing-declarations.
+ set_target_properties(gtest PROPERTIES COMPILE_FLAGS "-Wno-missing-declarations")
+endif()
+
+include_directories(third_party/googletest/include)
+
# Declare a dummy target to build all unit tests. Test targets should inject
# themselves as dependencies next to the target definition.
add_custom_target(all_tests)
diff --git a/crypto/CMakeLists.txt b/crypto/CMakeLists.txt
index 97fea5f..36224fc 100644
--- a/crypto/CMakeLists.txt
+++ b/crypto/CMakeLists.txt
@@ -207,3 +207,17 @@
target_link_libraries(refcount_test crypto)
add_dependencies(all_tests refcount_test)
+
+# TODO(davidben): Convert the remaining tests to GTest.
+add_executable(
+ crypto_test
+
+ dh/dh_test.cc
+ dsa/dsa_test.cc
+
+ $<TARGET_OBJECTS:gtest_main>
+ $<TARGET_OBJECTS:test_support>
+)
+
+target_link_libraries(crypto_test crypto gtest)
+add_dependencies(all_tests crypto_test)
diff --git a/crypto/dh/CMakeLists.txt b/crypto/dh/CMakeLists.txt
index f1e8616..83ae6d4 100644
--- a/crypto/dh/CMakeLists.txt
+++ b/crypto/dh/CMakeLists.txt
@@ -10,14 +10,3 @@
check.c
dh_asn1.c
)
-
-add_executable(
- dh_test
-
- dh_test.cc
-
- $<TARGET_OBJECTS:test_support>
-)
-
-target_link_libraries(dh_test crypto)
-add_dependencies(all_tests dh_test)
diff --git a/crypto/dh/dh_test.cc b/crypto/dh/dh_test.cc
index 8165c1a..9cde679 100644
--- a/crypto/dh/dh_test.cc
+++ b/crypto/dh/dh_test.cc
@@ -61,6 +61,8 @@
#include <vector>
+#include <gtest/gtest.h>
+
#include <openssl/bn.h>
#include <openssl/bytestring.h>
#include <openssl/crypto.h>
@@ -77,20 +79,16 @@
static bool TestASN1();
static bool TestRFC3526();
-int main() {
- CRYPTO_library_init();
-
+// TODO(davidben): Convert this file to GTest properly.
+TEST(DHTest, AllTests) {
if (!RunBasicTests() ||
!RunRFC5114Tests() ||
!TestBadY() ||
!TestASN1() ||
!TestRFC3526()) {
ERR_print_errors_fp(stderr);
- return 1;
+ ADD_FAILURE() << "Tests failed.";
}
-
- printf("PASS\n");
- return 0;
}
static int GenerateCallback(int p, int n, BN_GENCB *arg) {
diff --git a/crypto/dsa/CMakeLists.txt b/crypto/dsa/CMakeLists.txt
index 4d66136..d3c12f5 100644
--- a/crypto/dsa/CMakeLists.txt
+++ b/crypto/dsa/CMakeLists.txt
@@ -8,14 +8,3 @@
dsa.c
dsa_asn1.c
)
-
-add_executable(
- dsa_test
-
- dsa_test.cc
-
- $<TARGET_OBJECTS:test_support>
-)
-
-target_link_libraries(dsa_test crypto)
-add_dependencies(all_tests dsa_test)
diff --git a/crypto/dsa/dsa_test.cc b/crypto/dsa/dsa_test.cc
index 5fee6aa..d2cd33e 100644
--- a/crypto/dsa/dsa_test.cc
+++ b/crypto/dsa/dsa_test.cc
@@ -62,6 +62,8 @@
#include <stdio.h>
#include <string.h>
+#include <gtest/gtest.h>
+
#include <openssl/bn.h>
#include <openssl/crypto.h>
#include <openssl/err.h>
@@ -302,9 +304,8 @@
return true;
}
-int main(int argc, char **argv) {
- CRYPTO_library_init();
-
+// TODO(davidben): Convert this file to GTest properly.
+TEST(DSATest, AllTests) {
if (!TestGenerate(stdout) ||
!TestVerify(fips_sig, sizeof(fips_sig), 1) ||
!TestVerify(fips_sig_negative, sizeof(fips_sig_negative), -1) ||
@@ -312,9 +313,6 @@
!TestVerify(fips_sig_bad_length, sizeof(fips_sig_bad_length), -1) ||
!TestVerify(fips_sig_bad_r, sizeof(fips_sig_bad_r), 0)) {
ERR_print_errors_fp(stderr);
- return 1;
+ ADD_FAILURE() << "Tests failed";
}
-
- printf("PASS\n");
- return 0;
}
diff --git a/crypto/test/CMakeLists.txt b/crypto/test/CMakeLists.txt
index 8c75314..8857913 100644
--- a/crypto/test/CMakeLists.txt
+++ b/crypto/test/CMakeLists.txt
@@ -7,3 +7,11 @@
malloc.cc
test_util.cc
)
+
+add_library(
+ gtest_main
+
+ OBJECT
+
+ gtest_main.cc
+)
diff --git a/crypto/test/gtest_main.cc b/crypto/test/gtest_main.cc
new file mode 100644
index 0000000..50147bc
--- /dev/null
+++ b/crypto/test/gtest_main.cc
@@ -0,0 +1,23 @@
+/* Copyright (c) 2016, Google Inc.
+ *
+ * Permission to use, copy, modify, and/or distribute this software for any
+ * purpose with or without fee is hereby granted, provided that the above
+ * copyright notice and this permission notice appear in all copies.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
+ * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
+ * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR ANY
+ * SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
+ * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN ACTION
+ * OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR IN
+ * CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */
+
+#include <gtest/gtest.h>
+
+#include <openssl/crypto.h>
+
+int main(int argc, char **argv) {
+ CRYPTO_library_init();
+ testing::InitGoogleTest(&argc, argv);
+ return RUN_ALL_TESTS();
+}
diff --git a/decrepit/CMakeLists.txt b/decrepit/CMakeLists.txt
index 1e2386a..ee49bc8 100644
--- a/decrepit/CMakeLists.txt
+++ b/decrepit/CMakeLists.txt
@@ -1,3 +1,5 @@
+include_directories(../include)
+
add_subdirectory(bio)
add_subdirectory(biossl)
add_subdirectory(blowfish)
@@ -35,3 +37,15 @@
)
target_link_libraries(decrepit crypto ssl)
+
+add_executable(
+ decrepit_test
+
+ ripemd/ripemd_test.cc
+
+ $<TARGET_OBJECTS:gtest_main>
+ $<TARGET_OBJECTS:test_support>
+)
+
+target_link_libraries(decrepit_test crypto decrepit gtest)
+add_dependencies(all_tests decrepit_test)
diff --git a/decrepit/ripemd/CMakeLists.txt b/decrepit/ripemd/CMakeLists.txt
index 0714a8f..d3dd284 100644
--- a/decrepit/ripemd/CMakeLists.txt
+++ b/decrepit/ripemd/CMakeLists.txt
@@ -7,15 +7,3 @@
ripemd.c
)
-
-add_executable(
- ripemd_test
-
- ripemd_test.cc
-
- $<TARGET_OBJECTS:test_support>
-)
-
-target_link_libraries(ripemd_test crypto)
-target_link_libraries(ripemd_test decrepit)
-add_dependencies(all_tests ripemd_test)
diff --git a/decrepit/ripemd/ripemd_test.cc b/decrepit/ripemd/ripemd_test.cc
index e39c893..5c54196 100644
--- a/decrepit/ripemd/ripemd_test.cc
+++ b/decrepit/ripemd/ripemd_test.cc
@@ -19,6 +19,8 @@
#include <stdio.h>
#include <string.h>
+#include <gtest/gtest.h>
+
#include "../../crypto/internal.h"
#include "../../crypto/test/test_util.h"
@@ -53,7 +55,8 @@
0xd3, 0x32, 0x3c, 0xab, 0x82, 0xbf, 0x63, 0x32, 0x6b, 0xfb}},
};
-int main(void) {
+// TODO(davidben): Convert this file to GTest properly.
+TEST(RIPEMDTest, RunTest) {
unsigned test_num = 0;
int ok = 1;
@@ -111,10 +114,5 @@
ok = 0;
}
- if (!ok) {
- return 1;
- }
-
- printf("PASS\n");
- return 0;
+ EXPECT_EQ(1, ok);
}
diff --git a/ssl/CMakeLists.txt b/ssl/CMakeLists.txt
index fae17f1..a2d4c81 100644
--- a/ssl/CMakeLists.txt
+++ b/ssl/CMakeLists.txt
@@ -45,8 +45,9 @@
ssl_test.cc
+ $<TARGET_OBJECTS:gtest_main>
$<TARGET_OBJECTS:test_support>
)
-target_link_libraries(ssl_test ssl crypto)
+target_link_libraries(ssl_test ssl crypto gtest)
add_dependencies(all_tests ssl_test)
diff --git a/ssl/ssl_test.cc b/ssl/ssl_test.cc
index b27673b..33ef025 100644
--- a/ssl/ssl_test.cc
+++ b/ssl/ssl_test.cc
@@ -21,6 +21,8 @@
#include <utility>
#include <vector>
+#include <gtest/gtest.h>
+
#include <openssl/base64.h>
#include <openssl/bio.h>
#include <openssl/cipher.h>
@@ -3128,9 +3130,8 @@
return true;
}
-int main() {
- CRYPTO_library_init();
-
+// TODO(davidben): Convert this file to GTest properly.
+TEST(SSLTest, AllTests) {
if (!TestCipherRules() ||
!TestCurveRules() ||
!TestSSL_SESSIONEncoding(kOpenSSLSession) ||
@@ -3179,9 +3180,6 @@
!ForEachVersion(TestAutoChain) ||
!ForEachVersion(TestSSLWriteRetry)) {
ERR_print_errors_fp(stderr);
- return 1;
+ ADD_FAILURE() << "Tests failed";
}
-
- printf("PASS\n");
- return 0;
}
diff --git a/util/all_tests.go b/util/all_tests.go
index a937c8c..6cf8d13 100644
--- a/util/all_tests.go
+++ b/util/all_tests.go
@@ -185,6 +185,14 @@
(len(stdout) == 5 || stdout[len(stdout)-6] == '\n') {
return true, nil
}
+
+ // Also accept a googletest-style pass line. This is left here in
+ // transition until the tests are all converted and this script made
+ // unnecessary.
+ if bytes.Contains(stdout, []byte("\n[ PASSED ]")) {
+ return true, nil
+ }
+
return false, nil
}
diff --git a/util/all_tests.json b/util/all_tests.json
index 2382893..4589ee7 100644
--- a/util/all_tests.json
+++ b/util/all_tests.json
@@ -28,12 +28,11 @@
["crypto/cipher/cipher_test", "crypto/cipher/test/cipher_tests.txt"],
["crypto/cmac/cmac_test"],
["crypto/constant_time_test"],
+ ["crypto/crypto_test"],
["crypto/curve25519/ed25519_test", "crypto/curve25519/ed25519_tests.txt"],
["crypto/curve25519/x25519_test"],
["crypto/curve25519/spake25519_test"],
- ["crypto/dh/dh_test"],
["crypto/digest/digest_test"],
- ["crypto/dsa/dsa_test"],
["crypto/ec/ec_test"],
["crypto/ec/example_mul"],
["crypto/ec/p256-x86_64_test", "crypto/ec/p256-x86_64_tests.txt"],
@@ -61,6 +60,6 @@
["crypto/x509/x509_test"],
["crypto/x509v3/tab_test"],
["crypto/x509v3/v3name_test"],
- ["decrepit/ripemd/ripemd_test"],
+ ["decrepit/decrepit_test"],
["ssl/ssl_test"]
]
diff --git a/util/generate_build_files.py b/util/generate_build_files.py
index 67e5e37..d9f6522 100644
--- a/util/generate_build_files.py
+++ b/util/generate_build_files.py
@@ -50,6 +50,20 @@
],
}
+# For now, GTest-based tests are specified manually. Once everything has updated
+# to support GTest, these will be determined automatically by looking for files
+# ending with _test.cc.
+CRYPTO_TEST_SOURCES = [
+ 'crypto/dh/dh_test.cc',
+ 'crypto/dsa/dsa_test.cc',
+]
+DECREPIT_TEST_SOURCES = [
+ 'decrepit/decrepit_test.cc',
+]
+SSL_TEST_SOURCES = [
+ 'ssl/ssl_test.cc',
+]
+
PREFIX = None
@@ -147,6 +161,22 @@
blueprint.write('}\n\n')
blueprint.write('cc_defaults {\n')
+ blueprint.write(' name: "boringssl_crypto_test_sources",\n')
+ blueprint.write(' srcs: [\n')
+ for f in sorted(files['crypto_test']):
+ blueprint.write(' "%s",\n' % f)
+ blueprint.write(' ],\n')
+ blueprint.write('}\n\n')
+
+ blueprint.write('cc_defaults {\n')
+ blueprint.write(' name: "boringssl_ssl_test_sources",\n')
+ blueprint.write(' srcs: [\n')
+ for f in sorted(files['ssl_test']):
+ blueprint.write(' "%s",\n' % f)
+ blueprint.write(' ],\n')
+ blueprint.write('}\n\n')
+
+ blueprint.write('cc_defaults {\n')
blueprint.write(' name: "boringssl_tests_sources",\n')
blueprint.write(' srcs: [\n')
for f in sorted(files['test']):
@@ -220,6 +250,10 @@
out.write(']\n\n')
+ self.PrintVariableSection(out, 'crypto_test_sources',
+ files['crypto_test'])
+ self.PrintVariableSection(out, 'ssl_test_sources', files['ssl_test'])
+
out.write('def create_tests(copts, crypto, ssl):\n')
name_counts = {}
for test in files['tests']:
@@ -330,9 +364,12 @@
self.firstSection = True
out.write(self.header)
- self.PrintVariableSection(out, '_test_support_sources',
+ self.PrintVariableSection(out, 'test_support_sources',
files['test_support'] +
files['test_support_headers'])
+ self.PrintVariableSection(out, 'crypto_test_sources',
+ files['crypto_test'])
+ self.PrintVariableSection(out, 'ssl_test_sources', files['ssl_test'])
out.write('\n')
out.write('template("create_tests") {\n')
@@ -346,7 +383,7 @@
out.write(' sources = [\n')
out.write(' "%s",\n' % test)
out.write(' ]\n')
- out.write(' sources += _test_support_sources\n')
+ out.write(' sources += test_support_sources\n')
out.write(' if (defined(invoker.configs_exclude)) {\n')
out.write(' configs -= invoker.configs_exclude\n')
out.write(' }\n')
@@ -425,6 +462,13 @@
non-test sources."""
if is_dir:
return dent != 'test'
+ # For now, GTest-based tests are specified manually.
+ if dent in [os.path.basename(p) for p in CRYPTO_TEST_SOURCES]:
+ return False
+ if dent in [os.path.basename(p) for p in DECREPIT_TEST_SOURCES]:
+ return False
+ if dent in [os.path.basename(p) for p in SSL_TEST_SOURCES]:
+ return False
return '_test.' in dent or dent.startswith('example_')
@@ -434,6 +478,10 @@
return True
+def NotGTestMain(dent, is_dir):
+ return dent != 'gtest_main.cc'
+
+
def SSLHeaderFiles(dent, is_dir):
return dent in ['ssl.h', 'tls1.h', 'ssl23.h', 'ssl3.h', 'dtls1.h']
@@ -588,7 +636,7 @@
crypto_c_files.append('err_data.c')
test_support_c_files = FindCFiles(os.path.join('src', 'crypto', 'test'),
- AllFiles)
+ NotGTestMain)
test_support_h_files = (
FindHeaderFiles(os.path.join('src', 'crypto', 'test'), AllFiles) +
FindHeaderFiles(os.path.join('src', 'ssl', 'test'), AllFiles))
@@ -616,8 +664,10 @@
with open('src/util/all_tests.json', 'r') as f:
tests = json.load(f)
- # Skip tests for libdecrepit. Consumers import that manually.
- tests = [test for test in tests if not test[0].startswith("decrepit/")]
+ # For now, GTest-based tests are specified manually.
+ tests = [test for test in tests if test[0] not in ['crypto/crypto_test',
+ 'decrepit/decrepit_test',
+ 'ssl/ssl_test']]
test_binaries = set([test[0] for test in tests])
test_sources = set([
test.replace('.cc', '').replace('.c', '').replace(
@@ -637,10 +687,13 @@
'crypto': crypto_c_files,
'crypto_headers': crypto_h_files,
'crypto_internal_headers': crypto_internal_h_files,
+ 'crypto_test': sorted(CRYPTO_TEST_SOURCES +
+ ['crypto/test/gtest_main.cc']),
'fuzz': fuzz_c_files,
'ssl': ssl_c_files,
'ssl_headers': ssl_h_files,
'ssl_internal_headers': ssl_internal_h_files,
+ 'ssl_test': sorted(SSL_TEST_SOURCES + ['crypto/test/gtest_main.cc']),
'tool': tool_c_files,
'tool_headers': tool_h_files,
'test': test_c_files,
