Merge lp:~vila/subunit/gtk-filter-fixes into lp:~subunit/subunit/trunk

Drive-by fixes while testing subunit2gtk.

As a drive by it was fine up to here:

On Thu, 2010-01-14 at 11:25 +0000, Vincent Ladeuil wrote:
> + self.protocol.lineReceived(line)
> if not line:
> - self.protocol.lostConnection()
> self.on_finish()
> return False
> - self.protocol.lineReceived(line)
> - # schedule more IO shortly - if we say we're willing to do it
> - # immediately we starve things.
> - source_id = gobject.timeout_add(1, self.schedule_read)
> - return False
> + return True
>
> def schedule_read(self):
> - self.read_id = gobject.io_add_watch(self.stream,
> gobject.IO_IN, self.read)
> + self.read_id = gobject.io_add_watch(self.stream,
> gobject.IO_IN,
> + self.read)
>
> def hup(self, source, condition):
> - self.protocol.lostConnection()
> gobject.source_remove(self.read_id)
> self.on_finish()
> + return False

Please explain.

-Rob

>>>>> "robert" == Robert Collins <email address hidden> writes:

    robert> As a drive by it was fine up to here:
    robert> On Thu, 2010-01-14 at 11:25 +0000, Vincent Ladeuil wrote:
    >> + self.protocol.lineReceived(line)
    >> if not line:
    >> - self.protocol.lostConnection()
    >> self.on_finish()
    >> return False
    >> - self.protocol.lineReceived(line)
    >> - # schedule more IO shortly - if we say we're willing to do it
    >> - # immediately we starve things.
    >> - source_id = gobject.timeout_add(1, self.schedule_read)
    >> - return False
    >> + return True
    >>
    >> def schedule_read(self):
    >> - self.read_id = gobject.io_add_watch(self.stream,
    >> gobject.IO_IN, self.read)
    >> + self.read_id = gobject.io_add_watch(self.stream,
    >> gobject.IO_IN,
    >> + self.read)
    >>
    >> def hup(self, source, condition):
    >> - self.protocol.lostConnection()
    >> gobject.source_remove(self.read_id)
    >> self.on_finish()
    >> + return False

    robert> Please explain.

Commit message (bad launchpad didn't give it to you in mail I
suspect, sorry about that):

    Make subunit2gtk works and stop reporting the last test as an error.

    * subunit2gtk:
    (GTKTestResult.__init__, GTKTestResult.stopTest,
    GTKTestResult.progress): An attribute named like a method sounds
    like a recipe for disaster. 'progress' attribute renamed to
    'progress_model'.
    (GIOProtocolTestCase.read): Don't install additional watches, just
    return True to indicate we want to be called again. Return False
    only when we get to the end of the file. Leave hup handle the lost
    connection issues.
    (GIOProtocolTestCase.hup): We can be called only once but we need
    to return False to avoid spurious warnings.

On Thu, 2010-01-14 at 14:24 +0000, Vincent Ladeuil wrote:

So, please write better cover letters ;).

> >> - # schedule more IO shortly - if we say we're willing
> to do it
> >> - # immediately we starve things.

So, the reason I did it the way I did was that if you cat a 2MB file
through subunit2gtk with it always ready to do I/O the UI got totally
starved. Have you tested this?

I've pushed the rest of your changes, but not this one.

-Rob

>>>>> "robert" == Robert Collins <email address hidden> writes:

    robert> On Thu, 2010-01-14 at 14:24 +0000, Vincent Ladeuil wrote:
    robert> So, please write better cover letters ;).

    >> >> - # schedule more IO shortly - if we say we're willing
    >> to do it
    >> >> - # immediately we starve things.

    robert> So, the reason I did it the way I did was that if you cat a 2MB file
    robert> through subunit2gtk with it always ready to do I/O the UI got totally
    robert> starved. Have you tested this?

Oh, hmm, then the following is more appropriate IMHO:

   modified filters/subunit2gtk

=== modified file 'filters/subunit2gtk'
--- filters/subunit2gtk 2010-01-14 11:23:18 +0000
+++ filters/subunit2gtk 2010-01-15 07:24:21 +0000
@@ -225,6 +225,10 @@
         self.on_finish = on_finish

     def read(self, source, condition):
+ # We want the display to be refreshed. So we just process all pending
+ # events before handling the IO.
+ while gtk.events_pending():
+ gtk.main_iteration()
         #NB: \o/ actually blocks
         line = source.readline()
         self.protocol.lineReceived(line)

I've pushed that.

The version in trunk still makes the last test seen as an error
and I think it's due to the self.protocol.lostConnection() call.

Have you tested this ?

:)

     Vincent

On Fri, 2010-01-15 at 07:45 +0000, Vincent Ladeuil wrote:

> I've pushed that.
>
> The version in trunk still makes the last test seen as an error
> and I think it's due to the self.protocol.lostConnection() call.
>
> Have you tested this ?

Yes, it works perfectly for me.

-Rob

From irc - vila got me to try bzr selftest of log...

Its piping from a program that goes odd:

19:08 < lifeless> vila: so ./bzr selftest -s bt.test_log --subunit > foo
19:08 < lifeless> then cat foo | subunit2gtk
19:08 < lifeless> and subunit2gtk < foo
19:08 < lifeless> give different results