| Message ID | 1396840915-10384-7-git-send-email-quintela@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Mon, Apr 07, 2014 at 05:20:24AM +0200, Juan Quintela wrote: > This commit refactor the simple tests to test all integer types. We > move to hex because it is easier to read values of different types. > > Signed-off-by: Juan Quintela <quintela@redhat.com> > --- > tests/test-vmstate.c | 195 +++++++++++++++++++++++++++++++++++++++------------ > 1 file changed, 150 insertions(+), 45 deletions(-) > > diff --git a/tests/test-vmstate.c b/tests/test-vmstate.c > index 8b242c4..39a769e 100644 > --- a/tests/test-vmstate.c > +++ b/tests/test-vmstate.c > @@ -54,80 +54,186 @@ static QEMUFile *open_test_file(bool write) > return qemu_fdopen(fd, write ? "wb" : "rb"); > } > > -typedef struct TestSruct { > - uint32_t a, b, c, e; > - uint64_t d, f; > - bool skip_c_e; > -} TestStruct; > +/* Test struct that we are going to use for our tests */ > + > +typedef struct TestSimple { > + bool b_1, b_2; > + uint8_t u8_1, u8_2; > + uint16_t u16_1, u16_2; > + uint32_t u32_1, u32_2; > + uint64_t u64_1, u64_2; > + int8_t i8_1, i8_2; > + int16_t i16_1, i16_2; > + int32_t i32_1, i32_2; > + int64_t i64_1, i64_2; > +} TestSimple; > + > +/* Object instantiation, we are going to use it in more than one test */ > + > +TestSimple obj_simple = { > + .b_1 = true, > + .b_2 = false, > + .u8_1 = 129, > + .u8_1 = 130, > + .u16_1 = 512, > + .u16_2 = 45000, > + .u32_1 = 70000, > + .u32_2 = 100000, > + .u64_1 = 12121212, > + .u64_2 = 23232323, > + .i8_1 = 65, > + .i8_2 = -65, > + .i16_1 = 512, > + .i16_2 = -512, > + .i32_1 = 70000, > + .i32_2 = -70000, > + .i64_1 = 12121212, > + .i64_2 = -12121212, > +}; > > +/* Description of the values. If you add a primitive type > + you are expected to add a test here */ > > -static const VMStateDescription vmstate_simple = { > - .name = "test", > +static const VMStateDescription vmstate_simple_primitive = { > + .name = "simple/primitive", > .version_id = 1, > .minimum_version_id = 1, > .fields = (VMStateField[]) { > - VMSTATE_UINT32(a, TestStruct), > - VMSTATE_UINT32(b, TestStruct), > - VMSTATE_UINT32(c, TestStruct), > - VMSTATE_UINT64(d, TestStruct), > + VMSTATE_BOOL(b_1, TestSimple), > + VMSTATE_BOOL(b_2, TestSimple), > + VMSTATE_UINT8(u8_1, TestSimple), > + VMSTATE_UINT16(u16_1, TestSimple), > + VMSTATE_UINT32(u32_1, TestSimple), > + VMSTATE_UINT64(u64_1, TestSimple), > + VMSTATE_INT8(i8_1, TestSimple), > + VMSTATE_INT8(i8_2, TestSimple), > + VMSTATE_INT16(i16_1, TestSimple), > + VMSTATE_INT16(i16_2, TestSimple), > + VMSTATE_INT32(i32_1, TestSimple), > + VMSTATE_INT32(i32_2, TestSimple), > + VMSTATE_INT64(i64_1, TestSimple), > + VMSTATE_INT64(i64_2, TestSimple), > VMSTATE_END_OF_LIST() > } > }; > > -static void test_simple_save(void) > +/* It describes what goes through the wire. Our tests are basically: > + > + * save test > + - save a struct a vmstate to a file > + - read that file back (binary read, no vmstate) > + - compare it with what we expect to be on the wire > + * load test > + - save to the file what we expect to be on the wire > + - read struct back with vmstate in a different > + - compare back with the original struct > +*/ > + > +uint8_t wire_simple_primitive[] = { > + /* b_1 */ 0x01, > + /* b_2 */ 0x00, > + /* u8_1 */ 0x82, > + /* u16_1 */ 0x02, 0x00, > + /* u32_1 */ 0x00, 0x01, 0x11, 0x70, > + /* u64_1 */ 0x00, 0x00, 0x00, 0x00, 0x00, 0xb8, 0xf4, 0x7c, > + /* i8_1 */ 0x41, > + /* i8_2 */ 0xbf, > + /* i16_1 */ 0x02, 0x00, > + /* i16_2 */ 0xfe, 0x0, > + /* i32_1 */ 0x00, 0x01, 0x11, 0x70, > + /* i32_2 */ 0xff, 0xfe, 0xee, 0x90, > + /* i64_1 */ 0x00, 0x00, 0x00, 0x00, 0x00, 0xb8, 0xf4, 0x7c, > + /* i64_2 */ 0xff, 0xff, 0xff, 0xff, 0xff, 0x47, 0x0b, 0x84, > + QEMU_VM_EOF, /* just to ensure we won't get EOF reported prematurely */ > +}; Nice job. I was too lazy to test every single type. :-) > + > +#define SUCCESS(val) \ > + g_assert_cmpint((val), ==, 0) Isn't this the same as g_assert(!val)? > + > +#define FAILURE(val) \ > + g_assert_cmpint((val), !=, 0) Isn't this the same as g_assert(val)? I know, it is my fault as I have written the g_assert_cmpint(..., 0) code in the first place. :) > + > +static void test_simple_primitive(void) > { > QEMUFile *fsave = open_test_file(true); > - TestStruct obj = { .a = 1, .b = 2, .c = 3, .d = 4 }; > - vmstate_save_state(fsave, &vmstate_simple, &obj); > + > + /* Save file with vmstate */ > + vmstate_save_state(fsave, &vmstate_simple_primitive, &obj_simple); > g_assert(!qemu_file_get_error(fsave)); > qemu_fclose(fsave); > > QEMUFile *loading = open_test_file(false); > - uint8_t expected[] = { > - 0, 0, 0, 1, /* a */ > - 0, 0, 0, 2, /* b */ > - 0, 0, 0, 3, /* c */ > - 0, 0, 0, 0, 0, 0, 0, 4, /* d */ > - }; > - uint8_t result[sizeof(expected)]; > + /* we don't need QEMU_VM_EOF */ > + uint8_t result[sizeof(wire_simple_primitive)-1]; > + > + /* read back as binary */ > + > g_assert_cmpint(qemu_get_buffer(loading, result, sizeof(result)), ==, > sizeof(result)); > g_assert(!qemu_file_get_error(loading)); > - g_assert_cmpint(memcmp(result, expected, sizeof(result)), ==, 0); > + > + /* Compare that what is on the file is the same that what we > + expected to be there */ > + SUCCESS(memcmp(result, wire_simple_primitive, sizeof(result))); > > /* Must reach EOF */ > qemu_get_byte(loading); > g_assert_cmpint(qemu_file_get_error(loading), ==, -EIO); > > qemu_fclose(loading); > -} > > -static void test_simple_load(void) > -{ > - QEMUFile *fsave = open_test_file(true); > - uint8_t buf[] = { > - 0, 0, 0, 10, /* a */ > - 0, 0, 0, 20, /* b */ > - 0, 0, 0, 30, /* c */ > - 0, 0, 0, 0, 0, 0, 0, 40, /* d */ > - QEMU_VM_EOF, /* just to ensure we won't get EOF reported prematurely */ > - }; > - qemu_put_buffer(fsave, buf, sizeof(buf)); > + /* We save the file again. We want the EOF this time */ > + > + fsave = open_test_file(true); > + qemu_put_buffer(fsave, wire_simple_primitive, > + sizeof(wire_simple_primitive)); > qemu_fclose(fsave); > > - QEMUFile *loading = open_test_file(false); > - TestStruct obj; > - vmstate_load_state(loading, &vmstate_simple, &obj, 1); > + loading = open_test_file(false); > + TestSimple obj; > + SUCCESS(vmstate_load_state(loading, &vmstate_simple_primitive, &obj, 1)); > g_assert(!qemu_file_get_error(loading)); > - g_assert_cmpint(obj.a, ==, 10); > - g_assert_cmpint(obj.b, ==, 20); > - g_assert_cmpint(obj.c, ==, 30); > - g_assert_cmpint(obj.d, ==, 40); > + > +#define FIELD_ASSERT(name) g_assert_cmpint(obj.name, ==, obj_simple.name) > + FIELD_ASSERT(b_1); > + FIELD_ASSERT(b_2); > + FIELD_ASSERT(u8_1); > + FIELD_ASSERT(u16_1); > + FIELD_ASSERT(u32_1); > + FIELD_ASSERT(u64_1); > + FIELD_ASSERT(i8_1); > + FIELD_ASSERT(i8_2); > + FIELD_ASSERT(i16_1); > + FIELD_ASSERT(i16_2); > + FIELD_ASSERT(i32_1); > + FIELD_ASSERT(i32_2); > + FIELD_ASSERT(i64_1); > + FIELD_ASSERT(i64_2); > +#undef FIELD_ASSERT I wonder if we could do something to allow us to safely simplify this to use memcmp(). Looks good, anyway. > + > + /* We save the file again. We want the EOF this time */ > + > + fsave = open_test_file(true); > + /* this time we don't write the whole file */ > + qemu_put_buffer(fsave, wire_simple_primitive, 10); > + qemu_fclose(fsave); What about not just testing if it fails with 10 bytes, but checking all possible lengths from 0 to N-1 bytes? With this check we are only testing if the u64 loader handles EOF as expected. If subtle bugs are going to be introduced in EOF checking, I find them more likely in corner cases such as length==0, length==1 and lenght==N-1. > + > + loading = open_test_file(false); > + FAILURE(vmstate_load_state(loading, &vmstate_simple_primitive, &obj, 1)); > + > + /* Now it has to have an error */ > + g_assert(qemu_file_get_error(loading)); > qemu_fclose(loading); > } > > +typedef struct TestStruct { > + uint32_t a, b, c, e; > + uint64_t d, f; > + bool skip_c_e; > +} TestStruct; > + > static const VMStateDescription vmstate_versioned = { > - .name = "test", > + .name = "test/versioned", > .version_id = 2, > .minimum_version_id = 1, > .fields = (VMStateField[]) { > @@ -202,7 +308,7 @@ static bool test_skip(void *opaque, int version_id) > } > > static const VMStateDescription vmstate_skipping = { > - .name = "test", > + .name = "test/skip", > .version_id = 2, > .minimum_version_id = 1, > .fields = (VMStateField[]) { > @@ -337,8 +443,7 @@ int main(int argc, char **argv) > temp_fd = mkstemp(temp_file); > > g_test_init(&argc, &argv, NULL); > - g_test_add_func("/vmstate/simple/save", test_simple_save); > - g_test_add_func("/vmstate/simple/load", test_simple_load); > + g_test_add_func("/vmstate/simple/primitive", test_simple_primitive); > g_test_add_func("/vmstate/versioned/load/v1", test_load_v1); > g_test_add_func("/vmstate/versioned/load/v2", test_load_v2); > g_test_add_func("/vmstate/field_exists/load/noskip", test_load_noskip); > -- > 1.9.0 > >
diff --git a/tests/test-vmstate.c b/tests/test-vmstate.c index 8b242c4..39a769e 100644 --- a/tests/test-vmstate.c +++ b/tests/test-vmstate.c @@ -54,80 +54,186 @@ static QEMUFile *open_test_file(bool write) return qemu_fdopen(fd, write ? "wb" : "rb"); } -typedef struct TestSruct { - uint32_t a, b, c, e; - uint64_t d, f; - bool skip_c_e; -} TestStruct; +/* Test struct that we are going to use for our tests */ + +typedef struct TestSimple { + bool b_1, b_2; + uint8_t u8_1, u8_2; + uint16_t u16_1, u16_2; + uint32_t u32_1, u32_2; + uint64_t u64_1, u64_2; + int8_t i8_1, i8_2; + int16_t i16_1, i16_2; + int32_t i32_1, i32_2; + int64_t i64_1, i64_2; +} TestSimple; + +/* Object instantiation, we are going to use it in more than one test */ + +TestSimple obj_simple = { + .b_1 = true, + .b_2 = false, + .u8_1 = 129, + .u8_1 = 130, + .u16_1 = 512, + .u16_2 = 45000, + .u32_1 = 70000, + .u32_2 = 100000, + .u64_1 = 12121212, + .u64_2 = 23232323, + .i8_1 = 65, + .i8_2 = -65, + .i16_1 = 512, + .i16_2 = -512, + .i32_1 = 70000, + .i32_2 = -70000, + .i64_1 = 12121212, + .i64_2 = -12121212, +}; +/* Description of the values. If you add a primitive type + you are expected to add a test here */ -static const VMStateDescription vmstate_simple = { - .name = "test", +static const VMStateDescription vmstate_simple_primitive = { + .name = "simple/primitive", .version_id = 1, .minimum_version_id = 1, .fields = (VMStateField[]) { - VMSTATE_UINT32(a, TestStruct), - VMSTATE_UINT32(b, TestStruct), - VMSTATE_UINT32(c, TestStruct), - VMSTATE_UINT64(d, TestStruct), + VMSTATE_BOOL(b_1, TestSimple), + VMSTATE_BOOL(b_2, TestSimple), + VMSTATE_UINT8(u8_1, TestSimple), + VMSTATE_UINT16(u16_1, TestSimple), + VMSTATE_UINT32(u32_1, TestSimple), + VMSTATE_UINT64(u64_1, TestSimple), + VMSTATE_INT8(i8_1, TestSimple), + VMSTATE_INT8(i8_2, TestSimple), + VMSTATE_INT16(i16_1, TestSimple), + VMSTATE_INT16(i16_2, TestSimple), + VMSTATE_INT32(i32_1, TestSimple), + VMSTATE_INT32(i32_2, TestSimple), + VMSTATE_INT64(i64_1, TestSimple), + VMSTATE_INT64(i64_2, TestSimple), VMSTATE_END_OF_LIST() } }; -static void test_simple_save(void) +/* It describes what goes through the wire. Our tests are basically: + + * save test + - save a struct a vmstate to a file + - read that file back (binary read, no vmstate) + - compare it with what we expect to be on the wire + * load test + - save to the file what we expect to be on the wire + - read struct back with vmstate in a different + - compare back with the original struct +*/ + +uint8_t wire_simple_primitive[] = { + /* b_1 */ 0x01, + /* b_2 */ 0x00, + /* u8_1 */ 0x82, + /* u16_1 */ 0x02, 0x00, + /* u32_1 */ 0x00, 0x01, 0x11, 0x70, + /* u64_1 */ 0x00, 0x00, 0x00, 0x00, 0x00, 0xb8, 0xf4, 0x7c, + /* i8_1 */ 0x41, + /* i8_2 */ 0xbf, + /* i16_1 */ 0x02, 0x00, + /* i16_2 */ 0xfe, 0x0, + /* i32_1 */ 0x00, 0x01, 0x11, 0x70, + /* i32_2 */ 0xff, 0xfe, 0xee, 0x90, + /* i64_1 */ 0x00, 0x00, 0x00, 0x00, 0x00, 0xb8, 0xf4, 0x7c, + /* i64_2 */ 0xff, 0xff, 0xff, 0xff, 0xff, 0x47, 0x0b, 0x84, + QEMU_VM_EOF, /* just to ensure we won't get EOF reported prematurely */ +}; + +#define SUCCESS(val) \ + g_assert_cmpint((val), ==, 0) + +#define FAILURE(val) \ + g_assert_cmpint((val), !=, 0) + +static void test_simple_primitive(void) { QEMUFile *fsave = open_test_file(true); - TestStruct obj = { .a = 1, .b = 2, .c = 3, .d = 4 }; - vmstate_save_state(fsave, &vmstate_simple, &obj); + + /* Save file with vmstate */ + vmstate_save_state(fsave, &vmstate_simple_primitive, &obj_simple); g_assert(!qemu_file_get_error(fsave)); qemu_fclose(fsave); QEMUFile *loading = open_test_file(false); - uint8_t expected[] = { - 0, 0, 0, 1, /* a */ - 0, 0, 0, 2, /* b */ - 0, 0, 0, 3, /* c */ - 0, 0, 0, 0, 0, 0, 0, 4, /* d */ - }; - uint8_t result[sizeof(expected)]; + /* we don't need QEMU_VM_EOF */ + uint8_t result[sizeof(wire_simple_primitive)-1]; + + /* read back as binary */ + g_assert_cmpint(qemu_get_buffer(loading, result, sizeof(result)), ==, sizeof(result)); g_assert(!qemu_file_get_error(loading)); - g_assert_cmpint(memcmp(result, expected, sizeof(result)), ==, 0); + + /* Compare that what is on the file is the same that what we + expected to be there */ + SUCCESS(memcmp(result, wire_simple_primitive, sizeof(result))); /* Must reach EOF */ qemu_get_byte(loading); g_assert_cmpint(qemu_file_get_error(loading), ==, -EIO); qemu_fclose(loading); -} -static void test_simple_load(void) -{ - QEMUFile *fsave = open_test_file(true); - uint8_t buf[] = { - 0, 0, 0, 10, /* a */ - 0, 0, 0, 20, /* b */ - 0, 0, 0, 30, /* c */ - 0, 0, 0, 0, 0, 0, 0, 40, /* d */ - QEMU_VM_EOF, /* just to ensure we won't get EOF reported prematurely */ - }; - qemu_put_buffer(fsave, buf, sizeof(buf)); + /* We save the file again. We want the EOF this time */ + + fsave = open_test_file(true); + qemu_put_buffer(fsave, wire_simple_primitive, + sizeof(wire_simple_primitive)); qemu_fclose(fsave); - QEMUFile *loading = open_test_file(false); - TestStruct obj; - vmstate_load_state(loading, &vmstate_simple, &obj, 1); + loading = open_test_file(false); + TestSimple obj; + SUCCESS(vmstate_load_state(loading, &vmstate_simple_primitive, &obj, 1)); g_assert(!qemu_file_get_error(loading)); - g_assert_cmpint(obj.a, ==, 10); - g_assert_cmpint(obj.b, ==, 20); - g_assert_cmpint(obj.c, ==, 30); - g_assert_cmpint(obj.d, ==, 40); + +#define FIELD_ASSERT(name) g_assert_cmpint(obj.name, ==, obj_simple.name) + FIELD_ASSERT(b_1); + FIELD_ASSERT(b_2); + FIELD_ASSERT(u8_1); + FIELD_ASSERT(u16_1); + FIELD_ASSERT(u32_1); + FIELD_ASSERT(u64_1); + FIELD_ASSERT(i8_1); + FIELD_ASSERT(i8_2); + FIELD_ASSERT(i16_1); + FIELD_ASSERT(i16_2); + FIELD_ASSERT(i32_1); + FIELD_ASSERT(i32_2); + FIELD_ASSERT(i64_1); + FIELD_ASSERT(i64_2); +#undef FIELD_ASSERT + + /* We save the file again. We want the EOF this time */ + + fsave = open_test_file(true); + /* this time we don't write the whole file */ + qemu_put_buffer(fsave, wire_simple_primitive, 10); + qemu_fclose(fsave); + + loading = open_test_file(false); + FAILURE(vmstate_load_state(loading, &vmstate_simple_primitive, &obj, 1)); + + /* Now it has to have an error */ + g_assert(qemu_file_get_error(loading)); qemu_fclose(loading); } +typedef struct TestStruct { + uint32_t a, b, c, e; + uint64_t d, f; + bool skip_c_e; +} TestStruct; + static const VMStateDescription vmstate_versioned = { - .name = "test", + .name = "test/versioned", .version_id = 2, .minimum_version_id = 1, .fields = (VMStateField[]) { @@ -202,7 +308,7 @@ static bool test_skip(void *opaque, int version_id) } static const VMStateDescription vmstate_skipping = { - .name = "test", + .name = "test/skip", .version_id = 2, .minimum_version_id = 1, .fields = (VMStateField[]) { @@ -337,8 +443,7 @@ int main(int argc, char **argv) temp_fd = mkstemp(temp_file); g_test_init(&argc, &argv, NULL); - g_test_add_func("/vmstate/simple/save", test_simple_save); - g_test_add_func("/vmstate/simple/load", test_simple_load); + g_test_add_func("/vmstate/simple/primitive", test_simple_primitive); g_test_add_func("/vmstate/versioned/load/v1", test_load_v1); g_test_add_func("/vmstate/versioned/load/v2", test_load_v2); g_test_add_func("/vmstate/field_exists/load/noskip", test_load_noskip);
This commit refactor the simple tests to test all integer types. We move to hex because it is easier to read values of different types. Signed-off-by: Juan Quintela <quintela@redhat.com> --- tests/test-vmstate.c | 195 +++++++++++++++++++++++++++++++++++++++------------ 1 file changed, 150 insertions(+), 45 deletions(-)