Merge lp:~mars/tarmac/fix-test-command-call into lp:~launchpad/tarmac/lp-tarmac

Proposed by Māris Fogels on 2011-01-10
Status: Merged
Approved by: Māris Fogels on 2011-01-10
Approved revision: 394
Merged at revision: 393
Proposed branch: lp:~mars/tarmac/fix-test-command-call
Merge into: lp:~launchpad/tarmac/lp-tarmac
Diff against target: 90 lines (+29/-28)
1 file modified
tarmac/plugins/bundlecommand.py (+29/-28)
To merge this branch: bzr merge lp:~mars/tarmac/fix-test-command-call
Reviewer Review Type Date Requested Status
Gary Poster (community) 2011-01-10 Approve on 2011-01-10
Review via email: mp+45767@code.launchpad.net

Commit message

Fixed calling the program defined by bundle_verify_command.

Description of the change

Hi,

This branch fixes calls to the program defined by bundle_verify_command. The subprocess call for the command must use a file that is opened in 'w' mode, not 'rw'.

I also took a moment to clean up how the current working directory for the subprocess is set. I added comments and whitespace to make the function body more readable.

Maris

To post a comment you must log in.
Gary Poster (gary) wrote :

Looks good, thank you.

As the smallest of niggles, line 37 of the diff feels like a bit too much whitespace within the function to me, but that's one of those "sprinkle to taste" things. Just a thought.

Thanks again

Gary

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'tarmac/plugins/bundlecommand.py'
2--- tarmac/plugins/bundlecommand.py 2011-01-05 17:09:54 +0000
3+++ tarmac/plugins/bundlecommand.py 2011-01-10 22:15:30 +0000
4@@ -136,6 +136,7 @@
5 proposals merged into target and then runs the verify command.
6 Updating the list of proposals in case of failure.
7 """
8+ # Try and find the command that runs the test suite.
9 try:
10 self.verify_command = target.config.bundle_verify_command
11 except AttributeError:
12@@ -149,11 +150,7 @@
13 except AttributeError:
14 return
15
16- start_time = datetime.datetime.utcnow()
17- self.logger.debug('Running test command: %s' % self.verify_command)
18- cwd = os.getcwd()
19- os.chdir(target.tree.abspath(''))
20- # Output must be logged and kept in the filesystem to help debugging.
21+ # Rotate the test output logs.
22 current_log = os.path.join(
23 target.config.debug_log_location, 'current.log')
24 previous_log = os.path.join(
25@@ -161,39 +158,43 @@
26 if os.path.exists(current_log):
27 # Move it out of the way. Trashes previous.log.
28 os.rename(current_log, previous_log)
29- # Create a new empty file to log the output.
30- open(current_log, 'w').close()
31- # File must be opened for read and write.
32- output = open(current_log, 'rw')
33+
34+ # Run our test command.
35+ start_time = datetime.datetime.utcnow()
36+ self.logger.debug('Running test command: %s' % self.verify_command)
37+
38+ output = open(current_log, 'w')
39 try:
40 proc = subprocess.Popen(
41 self.verify_command,
42 shell=True,
43+ cwd=target.tree.abspath(''),
44 stdout=output,
45 stderr=subprocess.STDOUT)
46 return_code = proc.wait()
47- output.seek(0)
48- os.chdir(cwd)
49- self.logger.debug('Completed test command: %s' % self.verify_command)
50- end_time = datetime.datetime.utcnow()
51-
52- if return_code != 0:
53- for source, proposal in collected_proposals:
54- self.do_failed(
55- target, start_time, end_time, proposal, output)
56- # XXX matsubara 2010-12-17: The command code should be
57- # handling this, but due to the way we want emails formatted
58- # and sent, this is done here for now.
59- # See bug 691563 for more details.
60- # Uncommit and revert changes up to the point where target
61- # was before the merge.
62- target.uncommit_and_revert(restore_revno)
63- else:
64- self.logger.info('==========')
65- self.logger.info(output.read())
66 finally:
67 output.close()
68
69+ self.logger.debug('Completed test command: %s' % self.verify_command)
70+ end_time = datetime.datetime.utcnow()
71+
72+ # Handle the test command output.
73+ testlog = open(current_log)
74+ if return_code != 0:
75+ for source, proposal in collected_proposals:
76+ self.do_failed(
77+ target, start_time, end_time, proposal, testlog)
78+ # XXX matsubara 2010-12-17: The command code should be
79+ # handling this, but due to the way we want emails formatted
80+ # and sent, this is done here for now.
81+ # See bug 691563 for more details.
82+ # Uncommit and revert changes up to the point where target
83+ # was before the merge.
84+ target.uncommit_and_revert(restore_revno)
85+ else:
86+ self.logger.info('==========')
87+ self.logger.info(testlog.read())
88+
89 def do_failed(self, target, start_time, end_time, proposal, output):
90 '''Perform failure tests.
91

Subscribers

People subscribed via source and target branches

to all changes: