testutil: return 1 on success

Require that test methods return 1 on success (not 0). This is more
customary for OpenSSL.

Reviewed-by: Rich Salz <rsalz@openssl.org>

6 files changed, 25 insertions, 41 deletions

diff --git a/test/ct_test.c b/test/ct_test.c
index 16855df5af..ce417ab9bd 100644
--- a/test/ct_test.c
+++ b/test/ct_test.c
@@ -266,7 +266,7 @@ end:
 
 static int execute_cert_test(CT_TEST_FIXTURE fixture)
 {
-    int test_failed = 0;
+    int success = 0;
     X509 *cert = NULL, *issuer = NULL;
     STACK_OF(SCT) *scts = NULL;
     SCT *sct = NULL;
@@ -282,7 +282,6 @@ static int execute_cert_test(CT_TEST_FIXTURE fixture)
                                       CT_TEST_MAX_FILE_SIZE - 1);
 
         if (sct_text_len < 0) {
-            test_failed = 1;
             fprintf(stderr, "Test data file not found: %s\n",
                 fixture.sct_text_file);
             goto end;
@@ -299,7 +298,6 @@ static int execute_cert_test(CT_TEST_FIXTURE fixture)
         cert = load_pem_cert(fixture.certs_dir, fixture.certificate_file);
 
         if (cert == NULL) {
-            test_failed = 1;
             fprintf(stderr, "Unable to load certificate: %s\n",
                 fixture.certificate_file);
             goto end;
@@ -311,7 +309,6 @@ static int execute_cert_test(CT_TEST_FIXTURE fixture)
             issuer = load_pem_cert(fixture.certs_dir, fixture.issuer_file);
 
             if (issuer == NULL) {
-                test_failed = 1;
                 fprintf(stderr, "Unable to load issuer certificate: %s\n",
                         fixture.issuer_file);
                 goto end;
@@ -325,16 +322,14 @@ static int execute_cert_test(CT_TEST_FIXTURE fixture)
         sct_extension = X509_get_ext(cert, sct_extension_index);
         if (fixture.expected_sct_count > 0) {
             if (sct_extension == NULL) {
-                test_failed = 1;
                 fprintf(stderr, "SCT extension not found in: %s\n",
                     fixture.certificate_file);
                 goto end;
             }
 
-            if (fixture.sct_text_file) {
-                test_failed = compare_extension_printout(sct_extension,
-                                                    expected_sct_text);
-                if (test_failed != 0)
+            if (fixture.sct_text_file
+                && compare_extension_printout(sct_extension,
+                                              expected_sct_text)) {
                     goto end;
             }
 
@@ -349,7 +344,6 @@ static int execute_cert_test(CT_TEST_FIXTURE fixture)
                     if (!SCT_set_source(sct_i, SCT_SOURCE_X509V3_EXTENSION)) {
                         fprintf(stderr,
                                 "Error setting SCT source to X509v3 extension\n");
-                        test_failed = 1;
                         goto end;
                     }
                 }
@@ -357,7 +351,7 @@ static int execute_cert_test(CT_TEST_FIXTURE fixture)
                 are_scts_validated = SCT_LIST_validate(scts, ct_policy_ctx);
                 if (are_scts_validated < 0) {
                     fprintf(stderr, "Error verifying SCTs\n");
-                    test_failed = 1;
+                    goto end;
                 } else if (!are_scts_validated) {
                     int invalid_sct_count = 0;
                     int valid_sct_count = 0;
@@ -390,14 +384,10 @@ static int execute_cert_test(CT_TEST_FIXTURE fixture)
                                 fixture.expected_sct_count,
                                 unverified_sct_count);
                     }
-                    test_failed = 1;
-                }
-
-                if (test_failed != 0)
                     goto end;
+                }
             }
         } else if (sct_extension != NULL) {
-            test_failed = 1;
             fprintf(stderr,
                     "Expected no SCTs, but found SCT extension in: %s\n",
                     fixture.certificate_file);
@@ -408,21 +398,18 @@ static int execute_cert_test(CT_TEST_FIXTURE fixture)
     if (fixture.tls_sct != NULL) {
         const unsigned char *p = fixture.tls_sct;
         if (o2i_SCT(&sct, &p, fixture.tls_sct_len) == NULL) {
-            test_failed = 1;
             fprintf(stderr, "Failed to decode SCT from TLS format\n");
             goto end;
         }
 
-        if (fixture.sct_text_file) {
-            test_failed = compare_sct_printout(sct, expected_sct_text);
-            if (test_failed != 0)
+        if (fixture.sct_text_file
+            && compare_sct_printout(sct, expected_sct_text)) {
                 goto end;
         }
 
         tls_sct_len = i2o_SCT(sct, &tls_sct);
         if (tls_sct_len != fixture.tls_sct_len ||
             memcmp(fixture.tls_sct, tls_sct, tls_sct_len) != 0) {
-            test_failed = 1;
             fprintf(stderr, "Failed to encode SCT into TLS format correctly\n");
             goto end;
         }
@@ -430,16 +417,15 @@ static int execute_cert_test(CT_TEST_FIXTURE fixture)
         if (fixture.test_validity && cert != NULL) {
             int is_sct_validated = SCT_validate(sct, ct_policy_ctx);
             if (is_sct_validated < 0) {
-                test_failed = 1;
                 fprintf(stderr, "Error validating SCT\n");
                 goto end;
             } else if (!is_sct_validated) {
-                test_failed = 1;
                 fprintf(stderr, "SCT failed verification\n");
                 goto end;
             }
         }
     }
+    success = 1;
 
 end:
     X509_free(cert);
@@ -448,7 +434,7 @@ end:
     SCT_free(sct);
     CT_POLICY_EVAL_CTX_free(ct_policy_ctx);
     OPENSSL_free(tls_sct);
-    return test_failed;
+    return success;
 }
 
 #define SETUP_CT_TEST_FIXTURE() SETUP_TEST_FIXTURE(CT_TEST_FIXTURE, set_up)
diff --git a/test/d2i_test.c b/test/d2i_test.c
index cf01012ef9..6ffdf5507a 100644
--- a/test/d2i_test.c
+++ b/test/d2i_test.c
@@ -40,13 +40,13 @@ static int execute_test(D2I_TEST_FIXTURE fixture)
 {
     BIO *bio = NULL;
     ASN1_VALUE *value = NULL;
-    int ret = 1;
+    int ret = 0;
     unsigned char buf[2048];
     const unsigned char *buf_ptr = buf;
     int len;
 
     if ((bio = BIO_new_file(test_file, "r")) == NULL)
-        return 1;
+        return 0;
 
     /*
      * We don't use ASN1_item_d2i_bio because it, apparently,
@@ -60,7 +60,7 @@ static int execute_test(D2I_TEST_FIXTURE fixture)
     if (value != NULL)
         goto err;
 
-    ret = 0;
+    ret = 1;
 
  err:
     BIO_free(bio);
diff --git a/test/ssl_test.c b/test/ssl_test.c
index 01ce5d5684..499afd5e23 100644
--- a/test/ssl_test.c
+++ b/test/ssl_test.c
@@ -140,8 +140,7 @@ static int check_test(HANDSHAKE_RESULT result, SSL_TEST_CTX *test_ctx)
 
 static int execute_test(SSL_TEST_FIXTURE fixture)
 {
-    /* TODO(emilia): this is confusing. Flip to return 1 on success. */
-    int ret = 1;
+    int ret = 0;
     SSL_CTX *server_ctx = NULL, *client_ctx = NULL;
     SSL_TEST_CTX *test_ctx = NULL;
     HANDSHAKE_RESULT result;
@@ -163,15 +162,14 @@ static int execute_test(SSL_TEST_FIXTURE fixture)
 
     result = do_handshake(server_ctx, client_ctx);
 
-    if (check_test(result, test_ctx))
-        ret = 0;
+    ret = check_test(result, test_ctx);
 
 err:
     CONF_modules_unload(0);
     SSL_CTX_free(server_ctx);
     SSL_CTX_free(client_ctx);
     SSL_TEST_CTX_free(test_ctx);
-    if (ret != 0)
+    if (ret != 1)
         ERR_print_errors_fp(stderr);
     return ret;
 }
diff --git a/test/ssl_test_ctx_test.c b/test/ssl_test_ctx_test.c
index d933f2bcc7..3c6fa715f2 100644
--- a/test/ssl_test_ctx_test.c
+++ b/test/ssl_test_ctx_test.c
@@ -74,7 +74,7 @@ static SSL_TEST_CTX_TEST_FIXTURE set_up(const char *const test_case_name)
 
 static int execute_test(SSL_TEST_CTX_TEST_FIXTURE fixture)
 {
-    int ret = 1;
+    int success = 0;
 
     SSL_TEST_CTX *ctx = SSL_TEST_CTX_create(conf, fixture.test_section);
 
@@ -87,10 +87,10 @@ static int execute_test(SSL_TEST_CTX_TEST_FIXTURE fixture)
     if (!SSL_TEST_CTX_equal(ctx, fixture.expected_ctx))
         goto err;
 
-    ret = 0;
+    success = 1;
  err:
     SSL_TEST_CTX_free(ctx);
-    return ret;
+    return success;
 }
 
 static int execute_failure_test(SSL_TEST_CTX_TEST_FIXTURE fixture)
@@ -101,10 +101,10 @@ static int execute_failure_test(SSL_TEST_CTX_TEST_FIXTURE fixture)
         fprintf(stderr, "Parsing bad configuration %s succeeded.\n",
                 fixture.test_section);
         SSL_TEST_CTX_free(ctx);
-        return 1;
+        return 0;
     }
 
-    return 0;
+    return 1;
 }
 
 static void tear_down(SSL_TEST_CTX_TEST_FIXTURE fixture)
diff --git a/test/testutil.c b/test/testutil.c
index ccb6248234..3f63784880 100644
--- a/test/testutil.c
+++ b/test/testutil.c
@@ -113,14 +113,14 @@ int run_tests(const char *test_prog_name)
 
     for (i = 0; i != num_tests; ++i) {
         if (all_tests[i].num == -1) {
-            if (all_tests[i].test_fn()) {
+            if (!all_tests[i].test_fn()) {
                 printf("** %s failed **\n--------\n",
                        all_tests[i].test_case_name);
                 ++num_failed;
             }
         } else {
             for (j = 0; j < all_tests[i].num; j++) {
-                if (all_tests[i].param_test_fn(j)) {
+                if (!all_tests[i].param_test_fn(j)) {
                     printf("** %s failed test %d\n--------\n",
                            all_tests[i].test_case_name, j);
                     ++num_failed;
diff --git a/test/testutil.h b/test/testutil.h
index ba54ac1503..2ae3d7d16c 100644
--- a/test/testutil.h
+++ b/test/testutil.h
@@ -68,7 +68,7 @@
  *
  * EXECUTE_TEST will pass fixture to execute_func() by value, call
  * tear_down(), and return the result of execute_func(). execute_func() should
- * take a TEST_FIXTURE_TYPE by value and return zero on success or one on
+ * take a TEST_FIXTURE_TYPE by value and return 1 on success and 0 on
  * failure.
  *
  * Unit tests can define their own SETUP_TEST_FIXTURE and EXECUTE_TEST
@@ -94,7 +94,7 @@
     int result = 0
 
 # define EXECUTE_TEST(execute_func, tear_down)\
-        if (execute_func(fixture) != 0) result = 1;\
+        result = execute_func(fixture);\
         tear_down(fixture);\
         return result