Launchpad itself

Merge lp:~jml/launchpad/launchpad-tester into lp:launchpad

  1. launchpad-tester
  2. Merge into devel
Proposed by Jonathan Lange
Status: Merged
Approved by: Robert Collins
Approved revision: no longer in the source branch.
Merged at revision: 11437
Proposed branch: lp:~jml/launchpad/launchpad-tester
Merge into: lp:launchpad
Prerequisite: lp:~jml/launchpad/subject-and-attachment
Diff against target: 333 lines (+181/-78)
2 files modified
lib/devscripts/ec2test/remote.py (+19/-15)
lib/devscripts/ec2test/tests/test_remote.py (+162/-63)
To merge this branch: bzr merge lp:~jml/launchpad/launchpad-tester
Reviewer Review Type Date Requested Status
Robert Collins (community) Approve
Review via email: mp+33564@code.launchpad.net

Commit message

Add tests for devscripts.ec2test.remote.LaunchpadTester.

Description of the change

This branch only makes one small behaviour change -- it flushes the summary
log more regularly so that the "Summary" on an ec2 test web server is always
up-to-date.

More interestingly, it adds tests for LaunchpadTester.

I was going to change the behaviour more significantly, but every time I
did I felt uncomfortable and uncertain.

To post a comment you must log in.
Revision history for this message
Robert Collins (lifeless) wrote :

It might be nice to:
 - use fixtures rather than pushing those helpers around
 - flush the detailed log too, so that tribunal stays up to date more.

review: Approve
Revision history for this message
Jonathan Lange (jml) wrote :

On Tue, Aug 24, 2010 at 8:41 PM, Robert Collins
<email address hidden> wrote:
> Review: Approve
> It might be nice to:
>  - use fixtures rather than pushing those helpers around

You mean "prefer composition to inheritance", right? Does bzrlib give
me a way to do make_branch_and_tree without subclassing
TestCaseWithRepository?

>  - flush the detailed log too, so that tribunal stays up to date more.

The detailed log is flushed elsewhere.

jml

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'lib/devscripts/ec2test/remote.py'
--- lib/devscripts/ec2test/remote.py 2010-08-24 17:51:03 +0000
+++ lib/devscripts/ec2test/remote.py 2010-08-24 17:51:06 +0000
@@ -213,8 +213,6 @@
213class LaunchpadTester:213class LaunchpadTester:
214 """Runs Launchpad tests and gathers their results in a useful way."""214 """Runs Launchpad tests and gathers their results in a useful way."""
215215
216 # XXX: JonathanLange 2010-08-17: LaunchpadTester needs tests.
217
218 def __init__(self, logger, test_directory, test_options=()):216 def __init__(self, logger, test_directory, test_options=()):
219 """Construct a TestOnMergeRunner.217 """Construct a TestOnMergeRunner.
220218
@@ -236,9 +234,25 @@
236234
237 Subclasses must provide their own implementation of this method.235 Subclasses must provide their own implementation of this method.
238 """236 """
239 command = ['make', 'check', 'TESTOPTS="%s"' % self._test_options]237 command = ['make', 'check']
238 if self._test_options:
239 command.append('TESTOPTS="%s"' % self._test_options)
240 return command240 return command
241241
242 def _spawn_test_process(self):
243 """Actually run the tests.
244
245 :return: A `subprocess.Popen` object for the test run.
246 """
247 call = self.build_test_command()
248 self._logger.write_line("Running %s" % (call,))
249 # bufsize=0 means do not buffer any of the output. We want to
250 # display the test output as soon as it is generated.
251 return subprocess.Popen(
252 call, bufsize=0,
253 stdout=subprocess.PIPE, stderr=subprocess.STDOUT,
254 cwd=self._test_directory)
255
242 def test(self):256 def test(self):
243 """Run the tests, log the results.257 """Run the tests, log the results.
244258
@@ -247,20 +261,9 @@
247 the user the test results.261 the user the test results.
248 """262 """
249 self._logger.prepare()263 self._logger.prepare()
250
251 call = self.build_test_command()
252
253 try:264 try:
254 self._logger.write_line("Running %s" % (call,))265 popen = self._spawn_test_process()
255 # XXX: JonathanLange 2010-08-18: bufsize=-1 implies "use the
256 # system buffering", when we actually want no buffering at all.
257 popen = subprocess.Popen(
258 call, bufsize=-1,
259 stdout=subprocess.PIPE, stderr=subprocess.STDOUT,
260 cwd=self._test_directory)
261
262 self._gather_test_output(popen.stdout, self._logger)266 self._gather_test_output(popen.stdout, self._logger)
263
264 exit_status = popen.wait()267 exit_status = popen.wait()
265 except:268 except:
266 self._logger.error_in_testrunner(sys.exc_info())269 self._logger.error_in_testrunner(sys.exc_info())
@@ -277,6 +280,7 @@
277 for line in input_stream:280 for line in input_stream:
278 subunit_server.lineReceived(line)281 subunit_server.lineReceived(line)
279 logger.got_line(line)282 logger.got_line(line)
283 summary_stream.flush()
280284
281285
282class Request:286class Request:
283287
=== modified file 'lib/devscripts/ec2test/tests/test_remote.py'
--- lib/devscripts/ec2test/tests/test_remote.py 2010-08-24 17:51:03 +0000
+++ lib/devscripts/ec2test/tests/test_remote.py 2010-08-24 17:51:06 +0000
@@ -29,6 +29,7 @@
29 FlagFallStream,29 FlagFallStream,
30 gunzip_data,30 gunzip_data,
31 gzip_data,31 gzip_data,
32 LaunchpadTester,
32 remove_pidfile,33 remove_pidfile,
33 Request,34 Request,
34 SummaryResult,35 SummaryResult,
@@ -37,6 +38,69 @@
37 )38 )
3839
3940
41class RequestHelpers:
42
43 def patch(self, obj, name, value):
44 orig = getattr(obj, name)
45 setattr(obj, name, value)
46 self.addCleanup(setattr, obj, name, orig)
47 return orig
48
49 def make_trunk(self, parent_url='http://example.com/bzr/trunk'):
50 """Make a trunk branch suitable for use with `Request`.
51
52 `Request` expects to be given a path to a working tree that has a
53 branch with a configured parent URL, so this helper returns such a
54 working tree.
55 """
56 nick = parent_url.strip('/').split('/')[-1]
57 tree = self.make_branch_and_tree(nick)
58 tree.branch.set_parent(parent_url)
59 return tree
60
61 def make_request(self, branch_url=None, revno=None,
62 trunk=None, sourcecode_path=None,
63 emails=None, pqm_message=None):
64 """Make a request to test, specifying only things we care about.
65
66 Note that the returned request object will not ever send email, but
67 will instead log "sent" emails to `request.emails_sent`.
68 """
69 if trunk is None:
70 trunk = self.make_trunk()
71 if sourcecode_path is None:
72 sourcecode_path = self.make_sourcecode(
73 [('a', 'http://example.com/bzr/a', 2),
74 ('b', 'http://example.com/bzr/b', 3),
75 ('c', 'http://example.com/bzr/c', 5)])
76 request = Request(
77 branch_url, revno, trunk.basedir, sourcecode_path, emails,
78 pqm_message)
79 request.emails_sent = []
80 request._send_email = request.emails_sent.append
81 return request
82
83 def make_sourcecode(self, branches):
84 """Make a sourcecode directory with sample branches.
85
86 :param branches: A list of (name, parent_url, revno) tuples.
87 :return: The path to the sourcecode directory.
88 """
89 self.build_tree(['sourcecode/'])
90 for name, parent_url, revno in branches:
91 tree = self.make_branch_and_tree('sourcecode/%s' % (name,))
92 tree.branch.set_parent(parent_url)
93 for i in range(revno):
94 tree.commit(message=str(i))
95 return 'sourcecode/'
96
97 def make_logger(self, request=None, echo_to_stdout=False):
98 if request is None:
99 request = self.make_request()
100 return WebTestLogger(
101 'full.log', 'summary.log', 'index.html', request, echo_to_stdout)
102
103
40class TestFlagFallStream(TestCase):104class TestFlagFallStream(TestCase):
41 """Tests for `FlagFallStream`."""105 """Tests for `FlagFallStream`."""
42106
@@ -122,6 +186,104 @@
122 self.assertEqual(1, len(flush_calls))186 self.assertEqual(1, len(flush_calls))
123187
124188
189class FakePopen:
190 """Fake Popen object so we don't have to spawn processes in tests."""
191
192 def __init__(self, output, exit_status):
193 self.stdout = StringIO(output)
194 self._exit_status = exit_status
195
196 def wait(self):
197 return self._exit_status
198
199
200class TestLaunchpadTester(TestCaseWithTransport, RequestHelpers):
201
202 def make_tester(self, logger=None, test_directory=None, test_options=()):
203 if not logger:
204 logger = self.make_logger()
205 if not test_directory:
206 test_directory = 'unspecified-test-directory'
207 return LaunchpadTester(logger, test_directory, test_options)
208
209 def test_build_test_command_no_options(self):
210 # The LaunchpadTester runs "make check" if given no options.
211 tester = self.make_tester()
212 command = tester.build_test_command()
213 self.assertEqual(['make', 'check'], command)
214
215 def test_build_test_command_options(self):
216 # The LaunchpadTester runs 'make check TESTOPTIONS="<options>"' if
217 # given options.
218 tester = self.make_tester(test_options=('-vvv', '--subunit'))
219 command = tester.build_test_command()
220 self.assertEqual(
221 ['make', 'check', 'TESTOPTS="-vvv --subunit"'], command)
222
223 def test_spawn_test_process(self):
224 # _spawn_test_process uses subprocess.Popen to run the command
225 # returned by build_test_command. stdout & stderr are piped together,
226 # the cwd is the test directory specified in the constructor, and the
227 # bufsize is zore, meaning "don't buffer".
228 popen_calls = []
229 self.patch(
230 subprocess, 'Popen',
231 lambda *args, **kwargs: popen_calls.append((args, kwargs)))
232 tester = self.make_tester(test_directory='test-directory')
233 tester._spawn_test_process()
234 self.assertEqual(
235 [((tester.build_test_command(),),
236 {'bufsize': 0,
237 'stdout': subprocess.PIPE,
238 'stderr': subprocess.STDOUT,
239 'cwd': 'test-directory'})], popen_calls)
240
241 def test_running_test(self):
242 # LaunchpadTester.test() runs the test command, and then calls
243 # got_result with the result. This test is more of a smoke test to
244 # make sure that everything integrates well.
245 message = {'Subject': "One Crowded Hour"}
246 request = self.make_request(pqm_message=message)
247 logger = self.make_logger(request=request)
248 tester = self.make_tester(logger=logger)
249 output = "test output\n"
250 tester._spawn_test_process = lambda: FakePopen(output, 0)
251 tester.test()
252 # Message being sent implies got_result thought it got a success.
253 self.assertEqual([message], request.emails_sent)
254
255 def test_error_in_testrunner(self):
256 # Any exception is raised within LaunchpadTester.test() is an error in
257 # the testrunner. When we detect these, we do three things:
258 # 1. Log the error to the logger using error_in_testrunner
259 # 2. Call got_result with a False value, indicating test suite
260 # failure.
261 # 3. Re-raise the error. In the script, this triggers an email.
262 message = {'Subject': "One Crowded Hour"}
263 request = self.make_request(pqm_message=message)
264 logger = self.make_logger(request=request)
265 tester = self.make_tester(logger=logger)
266 # Break the test runner deliberately. In production, this is more
267 # likely to be a system error than a programming error.
268 tester._spawn_test_process = lambda: 1/0
269 self.assertRaises(ZeroDivisionError, tester.test)
270 # Message not being sent implies got_result thought it got a failure.
271 self.assertEqual([], request.emails_sent)
272 self.assertIn("ERROR IN TESTRUNNER", logger.get_summary_contents())
273 self.assertIn("ZeroDivisionError", logger.get_summary_contents())
274
275 def test_nonzero_exit_code(self):
276 message = {'Subject': "One Crowded Hour"}
277 request = self.make_request(pqm_message=message)
278 logger = self.make_logger(request=request)
279 tester = self.make_tester(logger=logger)
280 output = "test output\n"
281 tester._spawn_test_process = lambda: FakePopen(output, 10)
282 tester.test()
283 # Message not being sent implies got_result thought it got a failure.
284 self.assertEqual([], request.emails_sent)
285
286
125class TestPidfileHelpers(TestCase):287class TestPidfileHelpers(TestCase):
126 """Tests for `write_pidfile` and `remove_pidfile`."""288 """Tests for `write_pidfile` and `remove_pidfile`."""
127289
@@ -163,63 +325,6 @@
163 self.assertEqual(data, gunzip_data(compressed))325 self.assertEqual(data, gunzip_data(compressed))
164326
165327
166class RequestHelpers:
167
168 def make_trunk(self, parent_url='http://example.com/bzr/trunk'):
169 """Make a trunk branch suitable for use with `Request`.
170
171 `Request` expects to be given a path to a working tree that has a
172 branch with a configured parent URL, so this helper returns such a
173 working tree.
174 """
175 nick = parent_url.strip('/').split('/')[-1]
176 tree = self.make_branch_and_tree(nick)
177 tree.branch.set_parent(parent_url)
178 return tree
179
180 def make_request(self, branch_url=None, revno=None,
181 trunk=None, sourcecode_path=None,
182 emails=None, pqm_message=None):
183 """Make a request to test, specifying only things we care about.
184
185 Note that the returned request object will not ever send email, but
186 will instead log "sent" emails to `request.emails_sent`.
187 """
188 if trunk is None:
189 trunk = self.make_trunk()
190 if sourcecode_path is None:
191 sourcecode_path = self.make_sourcecode(
192 [('a', 'http://example.com/bzr/a', 2),
193 ('b', 'http://example.com/bzr/b', 3),
194 ('c', 'http://example.com/bzr/c', 5)])
195 request = Request(
196 branch_url, revno, trunk.basedir, sourcecode_path, emails,
197 pqm_message)
198 request.emails_sent = []
199 request._send_email = request.emails_sent.append
200 return request
201
202 def make_sourcecode(self, branches):
203 """Make a sourcecode directory with sample branches.
204
205 :param branches: A list of (name, parent_url, revno) tuples.
206 :return: The path to the sourcecode directory.
207 """
208 self.build_tree(['sourcecode/'])
209 for name, parent_url, revno in branches:
210 tree = self.make_branch_and_tree('sourcecode/%s' % (name,))
211 tree.branch.set_parent(parent_url)
212 for i in range(revno):
213 tree.commit(message=str(i))
214 return 'sourcecode/'
215
216 def make_logger(self, request=None, echo_to_stdout=False):
217 if request is None:
218 request = self.make_request()
219 return WebTestLogger(
220 'full.log', 'summary.log', 'index.html', request, echo_to_stdout)
221
222
223class TestRequest(TestCaseWithTransport, RequestHelpers):328class TestRequest(TestCaseWithTransport, RequestHelpers):
224 """Tests for `Request`."""329 """Tests for `Request`."""
225330
@@ -437,12 +542,6 @@
437542
438class TestWebTestLogger(TestCaseWithTransport, RequestHelpers):543class TestWebTestLogger(TestCaseWithTransport, RequestHelpers):
439544
440 def patch(self, obj, name, value):
441 orig = getattr(obj, name)
442 setattr(obj, name, value)
443 self.addCleanup(setattr, obj, name, orig)
444 return orig
445
446 def test_make_in_directory(self):545 def test_make_in_directory(self):
447 # WebTestLogger.make_in_directory constructs a logger that writes to a546 # WebTestLogger.make_in_directory constructs a logger that writes to a
448 # bunch of specific files in a directory.547 # bunch of specific files in a directory.