Message ID | 20131129145346.GA8313@otherpad.lan.raisama.net |
---|---|
State | New |
Headers | show |
On 11/29/13 15:53, Eduardo Habkost wrote: > On Thu, Nov 28, 2013 at 07:09:13PM +0100, Laszlo Ersek wrote: >> The current two GTest cases, /i440fx/defaults and /i440fx/pam can share a >> qemu process, but the next two cases will need dedicated instances. It is >> messy (and order-dependent) to dynamically configure GTest cases one by >> one to start, stop, or keep the current qtest (*); let's just have each >> GTest work with its own qtest. The performance difference should be >> negligible. >> >> (*) As g_test_run() can be invoked at most once per process startup, and >> it runs GTest cases in sequence, we'd need clumsy data structures to >> control each GTest case to start/stop/keep the qemu instance. Or, we'd >> have to code the same information into the test methods themselves, which >> would make them even more order-dependent. >> >> Signed-off-by: Laszlo Ersek <lersek@redhat.com> > [...] >> static void test_i440fx_defaults(gconstpointer opaque) >> { >> const TestData *s = opaque; >> + QPCIBus *bus; >> QPCIDevice *dev; >> uint32_t value; >> >> - dev = qpci_device_find(s->bus, QPCI_DEVFN(0, 0)); >> + bus = test_start_get_bus(s); >> + dev = qpci_device_find(bus, QPCI_DEVFN(0, 0)); > > You can use GTest setup/teardown functions to do this automatically on > all tests, but I am not sure the code really looks better when using it. > I gave it a try and it looks like this: > > diff --git a/tests/i440fx-test.c b/tests/i440fx-test.c > index 6ac46bf..c9d6f6a 100644 > --- a/tests/i440fx-test.c > +++ b/tests/i440fx-test.c > @@ -25,15 +25,35 @@ > > #define ARRAY_SIZE(array) (sizeof(array) / sizeof((array)[0])) > > -typedef struct TestData > +typedef struct TestArgs > { > int num_cpus; > +} TestArgs; > + > +typedef struct TestData > +{ > QPCIBus *bus; > } TestData; > > -static void test_i440fx_defaults(gconstpointer opaque) > +static void test_setup(TestData *s, gconstpointer user_data) > +{ > + const TestArgs *args = user_data; > + char *cmdline; > + > + cmdline = g_strdup_printf("-smp %d", args->num_cpus); > + qtest_start(cmdline); > + g_free(cmdline); > + s->bus = qpci_init_pc(); > +} > + > +static void test_teardown(TestData *s, gconstpointer user_data) > +{ > + qtest_end(); > +} > + > +static void test_i440fx_defaults(TestData *s, gconstpointer user_data) > { > - const TestData *s = opaque; > + const TestArgs *args = user_data; > QPCIDevice *dev; > uint32_t value; > > @@ -62,7 +82,7 @@ static void test_i440fx_defaults(gconstpointer opaque) > > /* 3.2.11 */ > value = qpci_config_readw(dev, 0x50); /* PMCCFG */ > - if (s->num_cpus == 1) { /* WPE */ > + if (args->num_cpus == 1) { /* WPE */ > g_assert(!(value & (1 << 15))); > } else { > g_assert((value & (1 << 15))); > @@ -176,9 +196,8 @@ static void write_area(uint32_t start, uint32_t end, uint8_t value) > g_free(data); > } > > -static void test_i440fx_pam(gconstpointer opaque) > +static void test_i440fx_pam(TestData *s, gconstpointer user_data) > { > - const TestData *s = opaque; > QPCIDevice *dev; > int i; > static struct { > @@ -258,26 +277,16 @@ static void test_i440fx_pam(gconstpointer opaque) > > int main(int argc, char **argv) > { > - TestData data; > - char *cmdline; > int ret; > + TestArgs args = { .num_cpus = 1 }; > > g_test_init(&argc, &argv, NULL); > > - data.num_cpus = 1; > - > - cmdline = g_strdup_printf("-smp %d", data.num_cpus); > - qtest_start(cmdline); > - g_free(cmdline); > - > - data.bus = qpci_init_pc(); > - > - g_test_add_data_func("/i440fx/defaults", &data, test_i440fx_defaults); > - g_test_add_data_func("/i440fx/pam", &data, test_i440fx_pam); > + g_test_add("/i440fx/defaults", TestData, &args, test_setup, > + test_i440fx_defaults, test_teardown); > + g_test_add("/i440fx/pam", TestData, &args, test_setup, > + test_i440fx_pam, test_teardown); > > ret = g_test_run(); > - > - qtest_end(); > - > return ret; > } > I agree that this specific use of the fixtures doesn't really pay off. Thank you Laszlo
diff --git a/tests/i440fx-test.c b/tests/i440fx-test.c index 6ac46bf..c9d6f6a 100644 --- a/tests/i440fx-test.c +++ b/tests/i440fx-test.c @@ -25,15 +25,35 @@ #define ARRAY_SIZE(array) (sizeof(array) / sizeof((array)[0])) -typedef struct TestData +typedef struct TestArgs { int num_cpus; +} TestArgs; + +typedef struct TestData +{ QPCIBus *bus; } TestData; -static void test_i440fx_defaults(gconstpointer opaque) +static void test_setup(TestData *s, gconstpointer user_data) +{ + const TestArgs *args = user_data; + char *cmdline; + + cmdline = g_strdup_printf("-smp %d", args->num_cpus); + qtest_start(cmdline); + g_free(cmdline); + s->bus = qpci_init_pc(); +} + +static void test_teardown(TestData *s, gconstpointer user_data) +{ + qtest_end(); +} + +static void test_i440fx_defaults(TestData *s, gconstpointer user_data) { - const TestData *s = opaque; + const TestArgs *args = user_data; QPCIDevice *dev; uint32_t value; @@ -62,7 +82,7 @@ static void test_i440fx_defaults(gconstpointer opaque) /* 3.2.11 */ value = qpci_config_readw(dev, 0x50); /* PMCCFG */ - if (s->num_cpus == 1) { /* WPE */ + if (args->num_cpus == 1) { /* WPE */ g_assert(!(value & (1 << 15))); } else { g_assert((value & (1 << 15))); @@ -176,9 +196,8 @@ static void write_area(uint32_t start, uint32_t end, uint8_t value) g_free(data); } -static void test_i440fx_pam(gconstpointer opaque) +static void test_i440fx_pam(TestData *s, gconstpointer user_data) { - const TestData *s = opaque; QPCIDevice *dev; int i; static struct { @@ -258,26 +277,16 @@ static void test_i440fx_pam(gconstpointer opaque) int main(int argc, char **argv) { - TestData data; - char *cmdline; int ret; + TestArgs args = { .num_cpus = 1 }; g_test_init(&argc, &argv, NULL); - data.num_cpus = 1; - - cmdline = g_strdup_printf("-smp %d", data.num_cpus); - qtest_start(cmdline); - g_free(cmdline); - - data.bus = qpci_init_pc(); - - g_test_add_data_func("/i440fx/defaults", &data, test_i440fx_defaults); - g_test_add_data_func("/i440fx/pam", &data, test_i440fx_pam); + g_test_add("/i440fx/defaults", TestData, &args, test_setup, + test_i440fx_defaults, test_teardown); + g_test_add("/i440fx/pam", TestData, &args, test_setup, + test_i440fx_pam, test_teardown); ret = g_test_run(); - - qtest_end(); - return ret; }