diff mbox series

[Openvpn-devel,v2] buffer_list_aggregate_separator(): add unit tests

Message ID 20171104224551.3079-1-steffan@karger.me
State Accepted
Headers show
Series [Openvpn-devel,v2] buffer_list_aggregate_separator(): add unit tests | expand

Commit Message

Steffan Karger Nov. 4, 2017, 11:45 a.m. UTC 
From: Steffan Karger <steffan.karger@fox-it.com>

Before refactoring this function, add tests to verify the new
implementation.

While writing the tests, it became clear that this function is not
behaving very well.  We'll fix that in follow-up commits.

Signed-off-by: Steffan Karger <steffan.karger@fox-it.com>
---
v2: add assert_buf_equals_str macro, and use that to compare (not zero-
    terminated) buffer contents with expected strings.

 tests/unit_tests/openvpn/test_buffer.c | 184 ++++++++++++++++++++++++++++++++-
 1 file changed, 182 insertions(+), 2 deletions(-)

Comments

Gert Doering Nov. 4, 2017, 11:12 p.m. UTC | #1 
ACK, test code is good :-)  (it passes the tests now, and does not change
anything else).  Applying to 2.4 as well, since "test good is good", even
if we might not apply the refactoring bits there.

Your patch has been applied to the master and release/2.4 branch.

commit 2ddb527abe38f5866ff01e91f8ee89d0f9700762 (master)
commit a202f6e74b32beabad764ee4226781a7f7e9c1c6 (release/2.4)
Author: Steffan Karger
Date:   Sat Nov 4 23:45:50 2017 +0100

     buffer_list_aggregate_separator(): add unit tests

     Signed-off-by: Steffan Karger <steffan.karger@fox-it.com>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <20171104224551.3079-1-steffan@karger.me>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg15748.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
diff mbox series

Patch

diff --git a/tests/unit_tests/openvpn/test_buffer.c b/tests/unit_tests/openvpn/test_buffer.c
index 69bb2e58..ba5aa670 100644
--- a/tests/unit_tests/openvpn/test_buffer.c
+++ b/tests/unit_tests/openvpn/test_buffer.c
@@ -35,7 +35,7 @@ 
 #include "buffer.h"
 
 static void
-buffer_strprefix(void **state)
+test_buffer_strprefix(void **state)
 {
     assert_true(strprefix("123456", "123456"));
     assert_true(strprefix("123456", "123"));
@@ -44,11 +44,191 @@  buffer_strprefix(void **state)
     assert_false(strprefix("12", "123"));
 }
 
+#define testsep ","
+#define testnosep ""
+#define teststr1 "one"
+#define teststr2 "two"
+#define teststr3 "three"
+#define teststr4 "four"
+
+#define assert_buf_equals_str(buf, str) \
+    assert_int_equal(BLEN(buf), strlen(str)); \
+    assert_memory_equal(BPTR(buf), str, BLEN(buf));
+
+struct test_buffer_list_aggregate_ctx {
+    struct buffer_list *empty;
+    struct buffer_list *one_two_three;
+    struct buffer_list *zero_length_strings;
+    struct buffer_list *empty_buffers;
+};
+
+static int test_buffer_list_setup(void **state)
+{
+    struct test_buffer_list_aggregate_ctx *ctx  = calloc(1, sizeof(*ctx));
+    ctx->empty = buffer_list_new(0);
+
+    ctx->one_two_three = buffer_list_new(3);
+    buffer_list_push(ctx->one_two_three, teststr1);
+    buffer_list_push(ctx->one_two_three, teststr2);
+    buffer_list_push(ctx->one_two_three, teststr3);
+
+    ctx->zero_length_strings = buffer_list_new(2);
+    buffer_list_push(ctx->zero_length_strings, "");
+    buffer_list_push(ctx->zero_length_strings, "");
+
+    ctx->empty_buffers = buffer_list_new(2);
+    uint8_t *data1 = malloc(1);
+    uint8_t *data2 = malloc(1);
+    buffer_list_push_data(ctx->empty_buffers, data1, 0);
+    buffer_list_push_data(ctx->empty_buffers, data2, 0);
+
+    *state = ctx;
+    return 0;
+}
+
+static int test_buffer_list_teardown(void **state)
+{
+    struct test_buffer_list_aggregate_ctx *ctx = *state;
+
+    buffer_list_free(ctx->empty);
+    buffer_list_free(ctx->one_two_three);
+    buffer_list_free(ctx->zero_length_strings);
+    buffer_list_free(ctx->empty_buffers);
+    free(ctx);
+    return 0;
+}
+
+static void
+test_buffer_list_full(void **state)
+{
+    struct test_buffer_list_aggregate_ctx *ctx = *state;
+
+    /* list full */
+    assert_int_equal(ctx->one_two_three->size, 3);
+    buffer_list_push(ctx->one_two_three, teststr4);
+    assert_int_equal(ctx->one_two_three->size, 3);
+}
+
+static void
+test_buffer_list_aggregate_separator_empty(void **state)
+{
+    struct test_buffer_list_aggregate_ctx *ctx = *state;
+
+    /* aggregating an empty buffer list results in an empty buffer list */
+    buffer_list_aggregate_separator(ctx->empty, 3, testsep);
+    assert_null(ctx->empty->head);
+}
+
+static void
+test_buffer_list_aggregate_separator_noop(void **state)
+{
+    struct test_buffer_list_aggregate_ctx *ctx = *state;
+
+    /* With a max length of 2, no aggregation should take place */
+    buffer_list_aggregate_separator(ctx->one_two_three, 2, testsep);
+    assert_int_equal(ctx->one_two_three->size, 3);
+    struct buffer *buf = buffer_list_peek(ctx->one_two_three);
+    assert_buf_equals_str(buf, teststr1);
+}
+
+static void
+test_buffer_list_aggregate_separator_two(void **state)
+{
+    struct test_buffer_list_aggregate_ctx *ctx = *state;
+
+    /* Aggregate the first two elements */
+    /* FIXME this exceeds the supplied max */
+    buffer_list_aggregate_separator(ctx->one_two_three, 4, testsep);
+    /* FIXME size does not get adjusted after aggregating */
+    assert_int_equal(ctx->one_two_three->size, 3);
+    struct buffer *buf = buffer_list_peek(ctx->one_two_three);
+    assert_buf_equals_str(buf, teststr1 testsep teststr2 testsep);
+}
+
+static void
+test_buffer_list_aggregate_separator_all(void **state)
+{
+    struct test_buffer_list_aggregate_ctx *ctx = *state;
+
+    /* Aggregate all */
+    buffer_list_aggregate_separator(ctx->one_two_three, 1<<16, testsep);
+    /* FIXME size does not get adjusted after aggregating */
+    assert_int_equal(ctx->one_two_three->size, 3);
+    struct buffer *buf = buffer_list_peek(ctx->one_two_three);
+    assert_buf_equals_str(buf,
+                          teststr1 testsep teststr2 testsep teststr3 testsep);
+}
+
+static void
+test_buffer_list_aggregate_separator_nosep(void **state)
+{
+    struct test_buffer_list_aggregate_ctx *ctx = *state;
+
+    /* Aggregate all */
+    buffer_list_aggregate_separator(ctx->one_two_three, 1<<16, testnosep);
+    /* FIXME size does not get adjusted after aggregating */
+    assert_int_equal(ctx->one_two_three->size, 3);
+    struct buffer *buf = buffer_list_peek(ctx->one_two_three);
+    assert_buf_equals_str(buf, teststr1 teststr2 teststr3);
+}
+
+static void
+test_buffer_list_aggregate_separator_zerolen(void **state)
+{
+    struct test_buffer_list_aggregate_ctx *ctx = *state;
+    struct buffer_list *bl_zerolen = ctx->zero_length_strings;
+
+    /* Aggregate all */
+    buffer_list_aggregate_separator(bl_zerolen, 1<<16, testnosep);
+    /* FIXME size does not get adjusted after aggregating */
+    assert_int_equal(bl_zerolen->size, 2);
+    struct buffer *buf = buffer_list_peek(bl_zerolen);
+    assert_buf_equals_str(buf, "");
+}
+
+static void
+test_buffer_list_aggregate_separator_emptybuffers(void **state)
+{
+    struct test_buffer_list_aggregate_ctx *ctx = *state;
+    struct buffer_list *bl_emptybuffers = ctx->empty_buffers;
+
+    /* Aggregate all */
+    buffer_list_aggregate_separator(bl_emptybuffers, 1<<16, testnosep);
+    /* FIXME size does not get adjusted after aggregating */
+    assert_int_equal(bl_emptybuffers->size, 2);
+    struct buffer *buf = buffer_list_peek(bl_emptybuffers);
+    assert_int_equal(BLEN(buf), 0);
+}
+
 int
 main(void)
 {
     const struct CMUnitTest tests[] = {
-        cmocka_unit_test(buffer_strprefix),
+        cmocka_unit_test(test_buffer_strprefix),
+        cmocka_unit_test_setup_teardown(test_buffer_list_full,
+                                        test_buffer_list_setup,
+                                        test_buffer_list_teardown),
+        cmocka_unit_test_setup_teardown(test_buffer_list_aggregate_separator_empty,
+                                        test_buffer_list_setup,
+                                        test_buffer_list_teardown),
+        cmocka_unit_test_setup_teardown(test_buffer_list_aggregate_separator_noop,
+                                        test_buffer_list_setup,
+                                        test_buffer_list_teardown),
+        cmocka_unit_test_setup_teardown(test_buffer_list_aggregate_separator_two,
+                                        test_buffer_list_setup,
+                                        test_buffer_list_teardown),
+        cmocka_unit_test_setup_teardown(test_buffer_list_aggregate_separator_all,
+                                        test_buffer_list_setup,
+                                        test_buffer_list_teardown),
+        cmocka_unit_test_setup_teardown(test_buffer_list_aggregate_separator_nosep,
+                                        test_buffer_list_setup,
+                                        test_buffer_list_teardown),
+        cmocka_unit_test_setup_teardown(test_buffer_list_aggregate_separator_zerolen,
+                                        test_buffer_list_setup,
+                                        test_buffer_list_teardown),
+        cmocka_unit_test_setup_teardown(test_buffer_list_aggregate_separator_emptybuffers,
+                                        test_buffer_list_setup,
+                                        test_buffer_list_teardown),
     };
 
     return cmocka_run_group_tests_name("buffer", tests, NULL, NULL);