Merge lp:~mikemc/ubuntuone-client/skip-test-wait-no-more-events-blame-pb into lp:ubuntuone-client

Proposed by Mike McCracken
Status: Merged
Approved by: Mike McCracken
Approved revision: 1378
Merged at revision: 1370
Proposed branch: lp:~mikemc/ubuntuone-client/skip-test-wait-no-more-events-blame-pb
Merge into: lp:ubuntuone-client
Prerequisite: lp:~mikemc/ubuntuone-client/accelerate-nirvana
Diff against target: 100 lines (+0/-73)
2 files modified
tests/platform/test_tools.py (+0/-16)
ubuntuone/platform/tools/__init__.py (+0/-57)
To merge this branch: bzr merge lp:~mikemc/ubuntuone-client/skip-test-wait-no-more-events-blame-pb
Reviewer Review Type Date Requested Status
dobey (community) Approve
Review via email: mp+142971@code.launchpad.net

Commit message

- Remove unused and potentially incorrect wait_no_more_events.

Description of the change

- Remove unused and potentially incorrect wait_no_more_events.

On windows and darwin, twisted PB's connect_signal is async, so
wait_no_more_events, which depends on connecting a handler to the
'Event' signal, will not be connected until after the
event_q.push('FILE_CREATE') call has already executed, which means
that we won't see that event.

Because wait_no_more_events internally waits for the signal to show,
it may not work as expected. Here's an incorrect sequence that happens every time in test_wait_no_more_events on darwin:

t:
0.00: call wait_no_more_events(last_event_interval=.1)
0.0?: it calls connect_signal, but doesn't wait for it to complete.
0.0?: it finishes, and registers check_last_event with the reactor to be called at 0.10.

0.0?: an event shows up sometime before the signal is connected.

0.10: check_last_event is called, and incorrectly returns True because the event
handler was not connected yet, so it saw no events.

0.1?: the event signal is potentially disconnected before it was
finished being connected (or not, it was still wrong)

So, because it's impossible to test on 2/3 of the platforms, and
doesn't appear to be used anywhere, I've removed wait_no_more_events.

To post a comment you must log in.
Revision history for this message
dobey (dobey) :
review: Approve
Revision history for this message
Ubuntu One Auto Pilot (otto-pilot) wrote :
Download full text (26.1 KiB)

The attempt to merge lp:~mikemc/ubuntuone-client/skip-test-wait-no-more-events-blame-pb into lp:ubuntuone-client failed. Below is the output from the failed tests.

/usr/bin/gnome-autogen.sh
checking for autoconf >= 2.53...
  testing autoconf2.50... not found.
  testing autoconf... found 2.69
checking for automake >= 1.10...
  testing automake-1.12... not found.
  testing automake-1.11... found 1.11.6
checking for libtool >= 1.5...
  testing libtoolize... found 2.4.2
checking for intltool >= 0.30...
  testing intltoolize... found 0.50.2
checking for pkg-config >= 0.14.0...
  testing pkg-config... found 0.26
checking for gtk-doc >= 1.0...
  testing gtkdocize... found 1.18
Checking for required M4 macros...
Checking for forbidden M4 macros...
Processing ./configure.ac
Running libtoolize...
libtoolize: putting auxiliary files in `.'.
libtoolize: copying file `./ltmain.sh'
libtoolize: putting macros in AC_CONFIG_MACRO_DIR, `m4'.
libtoolize: copying file `m4/libtool.m4'
libtoolize: copying file `m4/ltoptions.m4'
libtoolize: copying file `m4/ltsugar.m4'
libtoolize: copying file `m4/ltversion.m4'
libtoolize: copying file `m4/lt~obsolete.m4'
Running intltoolize...
Running gtkdocize...
Running aclocal-1.11...
Running autoconf...
Running autoheader...
Running automake-1.11...
Running ./configure --enable-gtk-doc --enable-debug ...
checking for a BSD-compatible install... /usr/bin/install -c
checking whether build environment is sane... yes
checking for a thread-safe mkdir -p... /bin/mkdir -p
checking for gawk... no
checking for mawk... mawk
checking whether make sets $(MAKE)... yes
checking how to create a ustar tar archive... gnutar
checking whether make supports nested variables... yes
checking for style of include used by make... GNU
checking for gcc... gcc
checking whether the C compiler works... yes
checking for C compiler default output file name... a.out
checking for suffix of executables...
checking whether we are cross compiling... no
checking for suffix of object files... o
checking whether we are using the GNU C compiler... yes
checking whether gcc accepts -g... yes
checking for gcc option to accept ISO C89... none needed
checking dependency style of gcc... gcc3
checking for library containing strerror... none required
checking for gcc... (cached) gcc
checking whether we are using the GNU C compiler... (cached) yes
checking whether gcc accepts -g... (cached) yes
checking for gcc option to accept ISO C89... (cached) none needed
checking dependency style of gcc... (cached) gcc3
checking build system type... x86_64-unknown-linux-gnu
checking host system type... x86_64-unknown-linux-gnu
checking how to print strings... printf
checking for a sed that does not truncate output... /bin/sed
checking for grep that handles long lines and -e... /bin/grep
checking for egrep... /bin/grep -E
checking for fgrep... /bin/grep -F
checking for ld used by gcc... /usr/bin/ld
checking if the linker (/usr/bin/ld) is GNU ld... yes
checking for BSD- or MS-compatible name lister (nm)... /usr/bin/nm -B
checking the name lister (/usr/bin/nm -B) interface... BSD nm
checking whether ln -s works... yes
checking the maximum length of command line arguments... 1572864
check...

1378. By Mike McCracken

remove unused import time

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'tests/platform/test_tools.py'
2--- tests/platform/test_tools.py 2013-01-08 00:05:58 +0000
3+++ tests/platform/test_tools.py 2013-01-12 00:30:31 +0000
4@@ -124,22 +124,6 @@
5 result = yield self.tool.wait_connected()
6 self.assertEqual(True, result)
7
8- def test_wait_no_more_events(self):
9- """Test wait_no_more_events."""
10- d = self.tool.wait_no_more_events(last_event_interval=.1)
11-
12- self.event_q.push("FS_FILE_CREATE", path="somepath")
13-
14- def check(result):
15- """Check result."""
16- self.assertTrue(self.handler.check_debug("new event",
17- "FS_FILE_CREATE"))
18- self.assertTrue(self.handler.check_debug("No more events"))
19- self.assertEqual(True, result)
20-
21- d.addBoth(check)
22- return d
23-
24 def test_all_downloads(self):
25 """ test wait_all_downloads """
26 d = self.tool.wait_all_downloads()
27
28=== modified file 'ubuntuone/platform/tools/__init__.py'
29--- ubuntuone/platform/tools/__init__.py 2012-10-22 13:31:02 +0000
30+++ ubuntuone/platform/tools/__init__.py 2013-01-12 00:30:31 +0000
31@@ -30,7 +30,6 @@
32
33 import logging
34 import sys
35-import time
36 import warnings
37
38 from twisted.internet import defer
39@@ -162,62 +161,6 @@
40 return d
41
42 @log_call(logger.debug)
43- def wait_no_more_events(self, last_event_interval, verbose=False):
44- """Wait until no more events are fired by the syncdaemon."""
45- from twisted.internet import reactor
46- d = defer.Deferred()
47-
48- def check_last_event():
49- """Check time!
50-
51- Check if the daemon is connected and didn't received any events
52- in the last_event_interval.
53-
54- """
55- current_time = time.time()
56- if self.last_event and \
57- current_time - self.last_event < last_event_interval:
58- # keep it running in case this is the last event
59- self.log.debug('rescheduling wait_no_more_events')
60- if not self.delayed_call.active():
61- self.delayed_call = reactor.callLater(last_event_interval,
62- check_last_event)
63- else:
64- self.delayed_call.reset(last_event_interval)
65- else:
66- self.log.debug('wait_no_more_events: No more events!')
67- d.callback(True)
68-
69- if verbose:
70- sys.stdout.write("Listening events")
71- sys.stdout.flush()
72-
73- def event_handler(event_dict):
74- """Update last_event and run checks."""
75- self.last_event = time.time()
76- self.log.debug('wait_no_more_events - new event: %s - %s',
77- event_dict['event_name'], str(self.last_event))
78- if verbose:
79- sys.stdout.write('.')
80- sys.stdout.flush()
81- if self.delayed_call.active():
82- self.delayed_call.reset(last_event_interval)
83-
84- match = self.connect_signal(signal_name='Event', handler=event_handler)
85-
86- def cleanup(result):
87- """Remove the signal handler."""
88- self.disconnect_signal(signal_name='Event', handler_or_match=match)
89- return result
90-
91- d.addBoth(cleanup)
92-
93- # in case the daemon already reached nirvana
94- self.delayed_call = reactor.callLater(last_event_interval,
95- check_last_event)
96- return d
97-
98- @log_call(logger.debug)
99 def wait_for_nirvana(self, last_event_interval=5, verbose=False):
100 """Wait until the syncdaemon reachs nirvana.
101

Subscribers

People subscribed via source and target branches