[viff-devel] Broken unit tests

Martin Geisler mg at lazybytes.net
Fri Jul 17 03:22:44 PDT 2009


Marcel Keller <mkeller at cs.au.dk> writes:

> Hi Martin,
>
> Martin Geisler wrote:
>> All the tests in the buildbot are currently broken:
>>
>>   http://buildbot.viff.dk/waterfall
>>
>> It is because of this change of yours:
>>
>> @@ -164,6 +166,18 @@
>>          # the Runtime, since we want everybody to wait until all
>>          # runtimes are ready.
>>          self.runtimes.append(result)
>> +        self.real_runtimes.append(runtime)
>> +        self.i = 0
>> +
>> +        # This loop call should ensure the queues of the parties are
>> +        # processed in a more or less fair manner. This is necessary
>> +        # because we have only one reactor for all parties here.
>> +        def loop_call():
>> +            for runtime in self.real_runtimes[self.i:] + self.real_runtimes[:self.i]:
>> +                runtime.process_deferred_queue()
>> +                self.i = (self.i + 1) % len(self.real_runtimes)
>> +
>> +        reactor.setLoopCall(loop_call)
>
> I think the problem is that after this change the unit tests need the
> reactor to be a ViffReactor, but it seems that the reactor is a
> SelectReactor. And this seems to be the case since your changeset
> "Make the VIFF reactor available to trial".

Okay, I see... this is a bit strange. I remember talking with Thomas
about it, and we found some sort of problem with installing the VIFF
reactor viff/test/__init__.py. I think the code was not loaded under
some circumstances, but I cannot figure out which at the moment :-(

>> I'm very confused about what exactly
>>
>>   for x in xs[i:] + xs[:i]:
>>     ...
>>     i = (i + 1) % len(xs)
>>
>> is supposed to do? After x has run through all xs (rotated i steps),
>> then i will have been incremented by len(xs). But you do it mod
>> len(xs) and so i comes out of the loop unchanged.
>
> This solution is indeed not very elegant. The problem that has to be
> solved here is the following: In a normal run every player (i.e. every
> runtime) has its own reactor and this reactor runs
> process_deferred_queue() of one runtime. In the unit tests however,
> there is one reactor for n runtimes. Therefore the reactor has to call
> process_deferred_queue() of every runtime. Now, if we just would
> iterate as usual over the runtimes this would lead to very unbalanced
> and therefore slower execution, because the loop call is called
> recursively. So this construct is to ensure that every time
> loop_call() is called, another runtime has first priority.

Is all this even necessary when the code calls self.activate_reactor
once in a while? I sort of had the impression that it was not necessary
to activate the reactor explicitly -- do things no longer work correctly
without it?

Also, the sending of messages is already delayed by a random amount, so
perhaps the unbalanced execution will be handled that way?

-- 
Martin Geisler



More information about the viff-devel mailing list