Merge lp:~statik/tarmac/reject-failures into lp:tarmac

[Elliot Murphy]

[Elliot Murphy (statik)]

Hi Paul,

An important thing to do if the branch fails to merge is to take it out of the approved queue. I think this branch does that, although I'm uncertain about whether setting queue_status works as I expected it to.

[Paul Hummer (rockstar)]

Hi statik-

  Strangely enough, I was planning on doing this EXACT same thing tonight.
Thanks for taking care of it for me. There's only one issue with this patch,
and I've documented it below. I'm willing to chat about this though, so don't
think it's not open for discussion.

=== modified file 'tarmac/bin.py'
--- a/tarmac/bin.py 2009-03-13 01:56:16 +0000
+++ b/tarmac/bin.py 2009-03-13 21:52:50 +0000
@@ -140,6 +145,12 @@
                     if self.dry_run:
                         print 'Branch failed test command'
                     target_tree.revert()
+ comment = '\n'.join(['Branch failed test command',
+ stdout_value, stderr_value])
+ candidate.createComment(subject="unused", content=comment,
+ vote='Needs Fixing')
+ candidate.queue_status = 'Needs Fixing'
+ candidate.lp_save()

Just a couple of things. I think the subject should be something like 'Branch
failed test command', and then the comment should just be stdout and stderr.
I'm also not sure if we should add the vote here, or just change the status.
All strings need to be unicode (although it still shouldn't affect it). The
queue_status is actually u'Needs review'. I've been trying to brainstorm a
better way to do enumerators, but for now, we just kinda gotta memorize them.

review: Needs Fixing

[Elliot Murphy (statik)]

Fixed, and pushed revision 55. Thanks for the great feedback!

[lp:~statik/tarmac/reject-failures]

55. By Elliot Murphy on 2009-03-14

Implemented review feedback.

=== modified file 'tarmac/bin.py'
--- tarmac/bin.py	2009-03-13 01:56:16 +0000
+++ tarmac/bin.py	2009-03-13 21:33:40 +0000
@@ -129,7 +129,12 @@
             if self.test_command:
                 cwd = os.getcwd()
                 os.chdir(temp_dir)
-                retcode = subprocess.call(self.test_command, shell=True)
+                proc = subprocess.Popen(self.test_command,
+                                        shell=True,
+                                        stdout=subprocess.PIPE,
+                                        stderr=subprocess.PIPE)
+                stdout_value, stderr_value = proc.communicate()
+                retcode = proc.wait()
                 os.chdir(cwd)
                 if retcode == 0:
                     if not self.dry_run:
@@ -140,6 +145,12 @@
                     if self.dry_run:
                         print 'Branch failed test command'
                     target_tree.revert()
+                    comment = '\n'.join(['Branch failed test command',
+                        stdout_value, stderr_value])
+                    candidate.createComment(subject="unused", content=comment,
+                                            vote='Needs Fixing')
+                    candidate.queue_status = 'Needs Fixing'
+                    candidate.lp_save()
             else:
                 if not self.dry_run:
                     target_tree.commit(commit_message)