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
1=== modified file 'lib/devscripts/ec2test/remote.py'
2--- lib/devscripts/ec2test/remote.py 2010-08-24 17:51:03 +0000
3+++ lib/devscripts/ec2test/remote.py 2010-08-24 17:51:06 +0000
4@@ -213,8 +213,6 @@
5 class LaunchpadTester:
6 """Runs Launchpad tests and gathers their results in a useful way."""
7
8- # XXX: JonathanLange 2010-08-17: LaunchpadTester needs tests.
9-
10 def __init__(self, logger, test_directory, test_options=()):
11 """Construct a TestOnMergeRunner.
12
13@@ -236,9 +234,25 @@
14
15 Subclasses must provide their own implementation of this method.
16 """
17- command = ['make', 'check', 'TESTOPTS="%s"' % self._test_options]
18+ command = ['make', 'check']
19+ if self._test_options:
20+ command.append('TESTOPTS="%s"' % self._test_options)
21 return command
22
23+ def _spawn_test_process(self):
24+ """Actually run the tests.
25+
26+ :return: A `subprocess.Popen` object for the test run.
27+ """
28+ call = self.build_test_command()
29+ self._logger.write_line("Running %s" % (call,))
30+ # bufsize=0 means do not buffer any of the output. We want to
31+ # display the test output as soon as it is generated.
32+ return subprocess.Popen(
33+ call, bufsize=0,
34+ stdout=subprocess.PIPE, stderr=subprocess.STDOUT,
35+ cwd=self._test_directory)
36+
37 def test(self):
38 """Run the tests, log the results.
39
40@@ -247,20 +261,9 @@
41 the user the test results.
42 """
43 self._logger.prepare()
44-
45- call = self.build_test_command()
46-
47 try:
48- self._logger.write_line("Running %s" % (call,))
49- # XXX: JonathanLange 2010-08-18: bufsize=-1 implies "use the
50- # system buffering", when we actually want no buffering at all.
51- popen = subprocess.Popen(
52- call, bufsize=-1,
53- stdout=subprocess.PIPE, stderr=subprocess.STDOUT,
54- cwd=self._test_directory)
55-
56+ popen = self._spawn_test_process()
57 self._gather_test_output(popen.stdout, self._logger)
58-
59 exit_status = popen.wait()
60 except:
61 self._logger.error_in_testrunner(sys.exc_info())
62@@ -277,6 +280,7 @@
63 for line in input_stream:
64 subunit_server.lineReceived(line)
65 logger.got_line(line)
66+ summary_stream.flush()
67
68
69 class Request:
70
71=== modified file 'lib/devscripts/ec2test/tests/test_remote.py'
72--- lib/devscripts/ec2test/tests/test_remote.py 2010-08-24 17:51:03 +0000
73+++ lib/devscripts/ec2test/tests/test_remote.py 2010-08-24 17:51:06 +0000
74@@ -29,6 +29,7 @@
75 FlagFallStream,
76 gunzip_data,
77 gzip_data,
78+ LaunchpadTester,
79 remove_pidfile,
80 Request,
81 SummaryResult,
82@@ -37,6 +38,69 @@
83 )
84
85
86+class RequestHelpers:
87+
88+ def patch(self, obj, name, value):
89+ orig = getattr(obj, name)
90+ setattr(obj, name, value)
91+ self.addCleanup(setattr, obj, name, orig)
92+ return orig
93+
94+ def make_trunk(self, parent_url='http://example.com/bzr/trunk'):
95+ """Make a trunk branch suitable for use with `Request`.
96+
97+ `Request` expects to be given a path to a working tree that has a
98+ branch with a configured parent URL, so this helper returns such a
99+ working tree.
100+ """
101+ nick = parent_url.strip('/').split('/')[-1]
102+ tree = self.make_branch_and_tree(nick)
103+ tree.branch.set_parent(parent_url)
104+ return tree
105+
106+ def make_request(self, branch_url=None, revno=None,
107+ trunk=None, sourcecode_path=None,
108+ emails=None, pqm_message=None):
109+ """Make a request to test, specifying only things we care about.
110+
111+ Note that the returned request object will not ever send email, but
112+ will instead log "sent" emails to `request.emails_sent`.
113+ """
114+ if trunk is None:
115+ trunk = self.make_trunk()
116+ if sourcecode_path is None:
117+ sourcecode_path = self.make_sourcecode(
118+ [('a', 'http://example.com/bzr/a', 2),
119+ ('b', 'http://example.com/bzr/b', 3),
120+ ('c', 'http://example.com/bzr/c', 5)])
121+ request = Request(
122+ branch_url, revno, trunk.basedir, sourcecode_path, emails,
123+ pqm_message)
124+ request.emails_sent = []
125+ request._send_email = request.emails_sent.append
126+ return request
127+
128+ def make_sourcecode(self, branches):
129+ """Make a sourcecode directory with sample branches.
130+
131+ :param branches: A list of (name, parent_url, revno) tuples.
132+ :return: The path to the sourcecode directory.
133+ """
134+ self.build_tree(['sourcecode/'])
135+ for name, parent_url, revno in branches:
136+ tree = self.make_branch_and_tree('sourcecode/%s' % (name,))
137+ tree.branch.set_parent(parent_url)
138+ for i in range(revno):
139+ tree.commit(message=str(i))
140+ return 'sourcecode/'
141+
142+ def make_logger(self, request=None, echo_to_stdout=False):
143+ if request is None:
144+ request = self.make_request()
145+ return WebTestLogger(
146+ 'full.log', 'summary.log', 'index.html', request, echo_to_stdout)
147+
148+
149 class TestFlagFallStream(TestCase):
150 """Tests for `FlagFallStream`."""
151
152@@ -122,6 +186,104 @@
153 self.assertEqual(1, len(flush_calls))
154
155
156+class FakePopen:
157+ """Fake Popen object so we don't have to spawn processes in tests."""
158+
159+ def __init__(self, output, exit_status):
160+ self.stdout = StringIO(output)
161+ self._exit_status = exit_status
162+
163+ def wait(self):
164+ return self._exit_status
165+
166+
167+class TestLaunchpadTester(TestCaseWithTransport, RequestHelpers):
168+
169+ def make_tester(self, logger=None, test_directory=None, test_options=()):
170+ if not logger:
171+ logger = self.make_logger()
172+ if not test_directory:
173+ test_directory = 'unspecified-test-directory'
174+ return LaunchpadTester(logger, test_directory, test_options)
175+
176+ def test_build_test_command_no_options(self):
177+ # The LaunchpadTester runs "make check" if given no options.
178+ tester = self.make_tester()
179+ command = tester.build_test_command()
180+ self.assertEqual(['make', 'check'], command)
181+
182+ def test_build_test_command_options(self):
183+ # The LaunchpadTester runs 'make check TESTOPTIONS="<options>"' if
184+ # given options.
185+ tester = self.make_tester(test_options=('-vvv', '--subunit'))
186+ command = tester.build_test_command()
187+ self.assertEqual(
188+ ['make', 'check', 'TESTOPTS="-vvv --subunit"'], command)
189+
190+ def test_spawn_test_process(self):
191+ # _spawn_test_process uses subprocess.Popen to run the command
192+ # returned by build_test_command. stdout & stderr are piped together,
193+ # the cwd is the test directory specified in the constructor, and the
194+ # bufsize is zore, meaning "don't buffer".
195+ popen_calls = []
196+ self.patch(
197+ subprocess, 'Popen',
198+ lambda *args, **kwargs: popen_calls.append((args, kwargs)))
199+ tester = self.make_tester(test_directory='test-directory')
200+ tester._spawn_test_process()
201+ self.assertEqual(
202+ [((tester.build_test_command(),),
203+ {'bufsize': 0,
204+ 'stdout': subprocess.PIPE,
205+ 'stderr': subprocess.STDOUT,
206+ 'cwd': 'test-directory'})], popen_calls)
207+
208+ def test_running_test(self):
209+ # LaunchpadTester.test() runs the test command, and then calls
210+ # got_result with the result. This test is more of a smoke test to
211+ # make sure that everything integrates well.
212+ message = {'Subject': "One Crowded Hour"}
213+ request = self.make_request(pqm_message=message)
214+ logger = self.make_logger(request=request)
215+ tester = self.make_tester(logger=logger)
216+ output = "test output\n"
217+ tester._spawn_test_process = lambda: FakePopen(output, 0)
218+ tester.test()
219+ # Message being sent implies got_result thought it got a success.
220+ self.assertEqual([message], request.emails_sent)
221+
222+ def test_error_in_testrunner(self):
223+ # Any exception is raised within LaunchpadTester.test() is an error in
224+ # the testrunner. When we detect these, we do three things:
225+ # 1. Log the error to the logger using error_in_testrunner
226+ # 2. Call got_result with a False value, indicating test suite
227+ # failure.
228+ # 3. Re-raise the error. In the script, this triggers an email.
229+ message = {'Subject': "One Crowded Hour"}
230+ request = self.make_request(pqm_message=message)
231+ logger = self.make_logger(request=request)
232+ tester = self.make_tester(logger=logger)
233+ # Break the test runner deliberately. In production, this is more
234+ # likely to be a system error than a programming error.
235+ tester._spawn_test_process = lambda: 1/0
236+ self.assertRaises(ZeroDivisionError, tester.test)
237+ # Message not being sent implies got_result thought it got a failure.
238+ self.assertEqual([], request.emails_sent)
239+ self.assertIn("ERROR IN TESTRUNNER", logger.get_summary_contents())
240+ self.assertIn("ZeroDivisionError", logger.get_summary_contents())
241+
242+ def test_nonzero_exit_code(self):
243+ message = {'Subject': "One Crowded Hour"}
244+ request = self.make_request(pqm_message=message)
245+ logger = self.make_logger(request=request)
246+ tester = self.make_tester(logger=logger)
247+ output = "test output\n"
248+ tester._spawn_test_process = lambda: FakePopen(output, 10)
249+ tester.test()
250+ # Message not being sent implies got_result thought it got a failure.
251+ self.assertEqual([], request.emails_sent)
252+
253+
254 class TestPidfileHelpers(TestCase):
255 """Tests for `write_pidfile` and `remove_pidfile`."""
256
257@@ -163,63 +325,6 @@
258 self.assertEqual(data, gunzip_data(compressed))
259
260
261-class RequestHelpers:
262-
263- def make_trunk(self, parent_url='http://example.com/bzr/trunk'):
264- """Make a trunk branch suitable for use with `Request`.
265-
266- `Request` expects to be given a path to a working tree that has a
267- branch with a configured parent URL, so this helper returns such a
268- working tree.
269- """
270- nick = parent_url.strip('/').split('/')[-1]
271- tree = self.make_branch_and_tree(nick)
272- tree.branch.set_parent(parent_url)
273- return tree
274-
275- def make_request(self, branch_url=None, revno=None,
276- trunk=None, sourcecode_path=None,
277- emails=None, pqm_message=None):
278- """Make a request to test, specifying only things we care about.
279-
280- Note that the returned request object will not ever send email, but
281- will instead log "sent" emails to `request.emails_sent`.
282- """
283- if trunk is None:
284- trunk = self.make_trunk()
285- if sourcecode_path is None:
286- sourcecode_path = self.make_sourcecode(
287- [('a', 'http://example.com/bzr/a', 2),
288- ('b', 'http://example.com/bzr/b', 3),
289- ('c', 'http://example.com/bzr/c', 5)])
290- request = Request(
291- branch_url, revno, trunk.basedir, sourcecode_path, emails,
292- pqm_message)
293- request.emails_sent = []
294- request._send_email = request.emails_sent.append
295- return request
296-
297- def make_sourcecode(self, branches):
298- """Make a sourcecode directory with sample branches.
299-
300- :param branches: A list of (name, parent_url, revno) tuples.
301- :return: The path to the sourcecode directory.
302- """
303- self.build_tree(['sourcecode/'])
304- for name, parent_url, revno in branches:
305- tree = self.make_branch_and_tree('sourcecode/%s' % (name,))
306- tree.branch.set_parent(parent_url)
307- for i in range(revno):
308- tree.commit(message=str(i))
309- return 'sourcecode/'
310-
311- def make_logger(self, request=None, echo_to_stdout=False):
312- if request is None:
313- request = self.make_request()
314- return WebTestLogger(
315- 'full.log', 'summary.log', 'index.html', request, echo_to_stdout)
316-
317-
318 class TestRequest(TestCaseWithTransport, RequestHelpers):
319 """Tests for `Request`."""
320
321@@ -437,12 +542,6 @@
322
323 class TestWebTestLogger(TestCaseWithTransport, RequestHelpers):
324
325- def patch(self, obj, name, value):
326- orig = getattr(obj, name)
327- setattr(obj, name, value)
328- self.addCleanup(setattr, obj, name, orig)
329- return orig
330-
331 def test_make_in_directory(self):
332 # WebTestLogger.make_in_directory constructs a logger that writes to a
333 # bunch of specific files in a directory.