On Fri, Jan 05, 2018 at 04:47:55AM -0500, Frediano Ziglio wrote:
> >
> > On Fri, Dec 22, 2017 at 10:07:08AM +0000, Frediano Ziglio wrote:
> > > @@ -395,16 +427,32 @@ idle_next_test(void *arg)
> > > {
> > > // end previous test
> > > if (test_num >= 0) {
> > > - g_assert(encode_called);
> > > + const TestData *data = &tests_data[test_num];
> > > + if (data->success) {
> > > + g_assert(encode_called);
> > > + } else {
> > > + g_assert(mechlist_called);
> > > + g_assert(!encode_called);
> > > + }
> > > reset_test();
> > > - basic_event_loop_quit();
> > > - return FALSE;
> > > }
> > >
> > > // start next test
> > > ++test_num;
> > > alarm(4);
> > >
> > > + if (test_num >= G_N_ELEMENTS(tests_data)) {
> > > + basic_event_loop_quit();
> > > + return FALSE;
> > > + }
> > > +
> > > + TestData *data = &tests_data[test_num];
> > > + if (data->mechlen < 0) {
> > > + data->mechlen = strlen(data->mechname);
> > > + }
> > > + int len = data->mechlen;
> > > + printf("\nRunning test %d ('%*.*s' %d)\n", test_num, len, len,
> > > data->mechname, len);
> > > +
> > > int sv[2];
> > > g_assert_cmpint(socketpair(AF_LOCAL, SOCK_STREAM, 0, sv), ==, 0);
> >
> > I haven't read over the whole tests yet, but I find the way you iterate
> > over the tests through idle_next_test() quite hard to read. Adding the
> > changes below on top of this series make things easier to follow in my
> > opinion.
> >
> > diff --git a/server/tests/basic-event-loop.c
> > b/server/tests/basic-event-loop.c
> > index 6fd8a2934..12205ecef 100644
> > --- a/server/tests/basic-event-loop.c
> > +++ b/server/tests/basic-event-loop.c
> > @@ -130,7 +130,7 @@ SpiceCoreInterface *basic_event_loop_init(void)
> > {
> > ignore_sigpipe();
> > spice_assert(main_context == NULL);
> > - main_context = g_main_context_new();
> > + main_context = g_main_context_default();
> > base_core_interface = event_loop_core;
> > base_core_interface.main_context = main_context;
> >
>
> I would strongly avoid this. Qemu can use different contexts and using the default
> can lead tests works while spice-server won't work on real world cases.
> I would retain my idle_add function to deal with this. Rest of proposal make sense,
> I'll merge it.
> Would be also better to put some comments why not using the default context for
> future reference.
Yup, definitely, I did not realize you had a very strong (and good)
reason for not using the default context.
Christophe
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel