Last Comment Bug 596132 - update talos remote testing to log to file instead of redirect to file
: update talos remote testing to log to file instead of redirect to file
Status: RESOLVED FIXED
:
Product: Testing
Classification: Components
Component: Talos (show other bugs)
: unspecified
: x86 Linux
: -- normal (vote)
: ---
Assigned To: Joel Maher ( :jmaher)
:
Mentors:
Depends on: 556530
Blocks: 579566
  Show dependency treegraph
 
Reported: 2010-09-13 21:02 PDT by Joel Maher ( :jmaher)
Modified: 2010-10-11 13:08 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
log output to a log file, not stdout (35.80 KB, patch)
2010-09-13 21:02 PDT, Joel Maher ( :jmaher)
k0scist: feedback+
Details | Diff | Splinter Review
log output to a log file, not stdout (1.1) (35.70 KB, patch)
2010-09-15 14:12 PDT, Joel Maher ( :jmaher)
anodelman: review-
Details | Diff | Splinter Review
log output to a log file, not stdout (2.0) (42.35 KB, patch)
2010-09-21 20:05 PDT, Joel Maher ( :jmaher)
no flags Details | Diff | Splinter Review
fix perfconfigurator.py to work with android and remote logging (1.0) (20.73 KB, patch)
2010-09-21 20:08 PDT, Joel Maher ( :jmaher)
anodelman: review+
Details | Diff | Splinter Review
log output to a log file, not stdout (3.0) (43.40 KB, patch)
2010-09-23 14:29 PDT, Joel Maher ( :jmaher)
no flags Details | Diff | Splinter Review
fix perfconfigurator.py to work with android and remote logging (2.0) (28.42 KB, patch)
2010-09-23 14:35 PDT, Joel Maher ( :jmaher)
anodelman: review+
Details | Diff | Splinter Review
log output to a log file, not stdout (3.1) (48.63 KB, patch)
2010-09-24 10:02 PDT, Joel Maher ( :jmaher)
anodelman: review+
Details | Diff | Splinter Review
fix perfconfigurator.py to work with android and remote logging (2.1) (28.49 KB, patch)
2010-09-24 13:52 PDT, Joel Maher ( :jmaher)
jmaher: review+
Details | Diff | Splinter Review

Description Joel Maher ( :jmaher) 2010-09-13 21:02:35 PDT
Created attachment 474977 [details] [diff] [review]
log output to a log file, not stdout

As we have found out Android devices don't have the ability to do a 'fennec > output.log'.  Currently all of talos depends on redirecting to a log and monitoring that log file for status changes and keywords to get the results we are looking for.

This patch will add MozillaFileLogging into the picture (works in IPC mode as well) for the mobile_profile which is used in fennec talos tests.

Essentially all places where there is a dump() have been changed to dumpLog() which outputs both a dump() and a file.write() to ensure full compatibility.  The log file name is set as a preference 'talos.logfile' and can be read by all code that needs to get the filename (similar to reftest)
Comment 1 Jeff Hammel 2010-09-14 09:04:07 PDT
Comment on attachment 474977 [details] [diff] [review]
log output to a log file, not stdout

>Index: bcontroller.py
>===================================================================
>RCS file: /cvsroot/mozilla/testing/performance/talos/bcontroller.py,v
>retrieving revision 1.15
>diff -u -8 -p -r1.15 bcontroller.py
>--- bcontroller.py	15 Apr 2010 18:03:58 -0000	1.15
>+++ bcontroller.py	14 Sep 2010 04:01:06 -0000

<snip/>

>+  def __init__(self, command, log, mod, remoteLog = '', deviceManager = None):
Why not remoteLog=None?


>      self.command = command
>      self.log = log
>      self.mod = mod
>      self.endTime = -1
>      self.returncode = -1
>      self.deviceManager = deviceManager
>+     if (remoteLog == None or remoteLog == ''):
>+       self.remoteLog = self.log
>+     else:
>+       self.remoteLog = remoteLog
self.remoteLog = remoteLog or self.log


>   def run(self):
>     if self.mod:
>-      if (self.deviceManager.__class__.__name__ == "WinmoProcess"):
>+      if (self.deviceManager.__class__.__name__ == "RemoteProcess"):
>         if (self.mod == "str(int(time.time()*1000))"):
>           self.command += self.deviceManager.getCurrentTime()
>       else:
>         self.command = self.command + eval(self.mod)
> 
>-    if (self.deviceManager.__class__.__name__ == "WinmoProcess"):
>-      retVal = self.deviceManager.launchProcess(self.command, timeout=600)
>+    if (self.deviceManager.__class__.__name__ == "RemoteProcess"):

I'd save the class name in a variable so that this didn't have to be typed out and then when it changes it only has to be changed in one place

>+      if (len(self.remoteLog.split('/')) <= 1):
if (self.remoteLog.count('/') <=1)
>+        self.remoteLog = self.deviceManager.getDeviceRoot() + '/' + self.remoteLog
>+
>+      retVal = self.deviceManager.launchProcess(self.command, outputFile=self.remoteLog, timeout=600)

> class BrowserController:
> 
>   def __init__(self, command, mod, name, child_process, 
>-               timeout, log, host='', port=20701, root=''):
>+               timeout, log, host='', port=20701, root='', remoteLog=''):
As above, why not None?

>@@ -172,21 +180,23 @@ def main(argv=None):
<snip/>
>-   opts, args = getopt.getopt(argv[1:], "c:t:n:p:l:m:h:r:o", ["command=", "timeout=", "name=", "child_process=", "log=", "mod=", "host=", "deviceRoot=", "port="])
>+   opts, args = getopt.getopt(argv[1:], "c:t:n:p:l:m:h:r:o:d", ["command=", "timeout=", "name=", "child_process=", 
>+                                                              "log=", "mod=", "host=", "deviceRoot=", "port=", "remoteLog="])
Ah, the horror of getopt :(  I really hope someone cleans this up one day

>Index: devicemanager.py
>===================================================================
>RCS file: /cvsroot/mozilla/testing/performance/talos/devicemanager.py,v
>retrieving revision 1.3
>diff -u -8 -p -r1.3 devicemanager.py
>--- devicemanager.py	21 Jul 2010 18:35:55 -0000	1.3
>+++ devicemanager.py	14 Sep 2010 04:01:06 -0000
>@@ -53,17 +53,17 @@ class FileError(Exception):
>     self.msg = msg
> 
>   def __str__(self):
>     return self.msg
> 
> class DeviceManager:
>   host = ''
>   port = 0
>-  debug = 3
>+  debug = 2
So why is the debug level changed?  Should that go in this patch or a separate one? I'm probably missing a piece of the puzzle.


>@@ -383,24 +383,23 @@ class DeviceManager:
> 
>     #NOTE: we sleep for 30 seconds to allow the application to startup
>     time.sleep(30)
> 
>     self.process = self.processExist(appname)
>     if (self.debug >= 4): print "got pid: " + str(self.process) + " for process: " + str(appname)
> 
>   def launchProcess(self, cmd, outputFile = "process.txt", cwd = ''):
>-    if (outputFile == "process.txt"):
>-      outputFile = self.getDeviceRoot() + '/' + "process.txt"
>-
>     cmdline = subprocess.list2cmdline(cmd)
>-    self.fireProcess(cmdline + " > " + outputFile)
>-    handle = outputFile
>+    if (outputFile == "process.txt" or outputFile == None):
>+      outputFile = self.getDeviceRoot() + '/' + "process.txt"
>+      cmdline += " > " + outputFile
I'm not sure what expected behaviour is here.  Its hard to glean intent from these lines of code

 

>@@ -711,8 +710,34 @@ class DeviceManager:
<snip/>
>+  """
>+    return the current time on the device
>+  """
>+  def getCurrentTime(self):
>+    data = self.sendCMD(['clok'])
>+    if (data == None):
>+      return None
>+    return self.stripPrompt(data).strip('\n')
I have no idea what this does but I'll take your work for it


>+  """
>+    edit the user.js in the profile (on the host machine) and 
>+    add the xpconnect priviledges for the remote server
>+  """
The docstring should go inside the function

>+  def addRemoteServerPref(self, profile_dir, server):
>+    user_js_filename = os.path.join(profile_dir, 'user.js')
>+    user_js_file = open(user_js_filename, 'a+')
>+   
>+    #TODO: p2 is hardcoded, how do we determine what prefs.js has hardcoded?
>+    remoteCode = """
>+user_pref("capability.principal.codebase.p1.granted", "UniversalPreferencesWrite UniversalXPConnect UniversalPreferencesRead");
>+user_pref("capability.principal.codebase.p1.id", "http://%(server)s");
>+user_pref("capability.principal.codebase.p1.subjectName", "");
>+""" % { "server": server }
This is fine for three lines;  if this code needs to be touched again, I'd recommend doing it a bit more generally.

<snip/>

>Index: run_tests.py
>===================================================================
>RCS file: /cvsroot/mozilla/testing/performance/talos/run_tests.py,v
>retrieving revision 1.66
>diff -u -8 -p -r1.66 run_tests.py
>--- run_tests.py	4 Aug 2010 13:22:14 -0000	1.66
>+++ run_tests.py	14 Sep 2010 04:01:07 -0000
>@@ -422,16 +422,21 @@ def test_file(filename, to_screen):
>   if 'deviceroot' in yaml_config:
>       browser_config['deviceroot'] = yaml_config['deviceroot']
>   else:
>       browser_config['deviceroot'] = ''
>   if 'remote' in yaml_config:
>       browser_config['remote'] = yaml_config['remote']
>   else:
>       browser_config['remote'] = False
>+  if 'remote_log' in yaml_config:
>+      browser_config['remote_log'] = yaml_config['remote_log']
>+  else:
>+      browser_config['remote_log'] = ''
>+
Again, why not None?

<snip/>

>Index: mobile_profile/extensions/electrolysis@mozilla.org/chrome/content/MozillaFileLogger.js
>===================================================================
<snip/>
>+  try {
>+    netscape.security.PrivilegeManager.enablePrivilege("UniversalXPConnect");
>+  } catch (ex) {} //running in ipcMode-chrome
>+
>+  MozillaFileLogger._file = Cc[LF_CID].createInstance(Ci.nsILocalFile);
>+  MozillaFileLogger._file.initWithPath(path);
>+  MozillaFileLogger._foStream = Cc[FOSTREAM_CID].createInstance(Ci.nsIFileOutputStream);
>+//  MozillaFileLogger._foStream.init(this._file, PR_WRITE_ONLY | PR_CREATE_FILE | PR_APPEND,

Any reason you kept this comment in?

Overall, looks fine.
Comment 2 Joel Maher ( :jmaher) 2010-09-15 14:12:17 PDT
Created attachment 475627 [details] [diff] [review]
log output to a log file, not stdout (1.1)

cleaned up some feedback from ctalbert about devicemanager.py and a few bits and pieces from jhammel the feedback master.  This is the patch that is running on talos staging ;)

if this passes on staging and anode gives an r+, we can put this in the queue for landing on the next talos downtime (eta Sept 23rd)
Comment 3 Joel Maher ( :jmaher) 2010-09-16 15:30:08 PDT
this passed green on talos staging.
Comment 4 alice nodelman [:alice] [:anode] 2010-09-21 15:43:42 PDT
Comment on attachment 475627 [details] [diff] [review]
log output to a log file, not stdout (1.1)

Any reason for the name change between WinmoProcess and RemoteProcess (considering that it still lives in a file called ffprocess_winmo.py

The main issue here is that you want to change totally over to dumpLog over dump.  To do that complete we'd have to update the pageloader bundle as well (http://hg.mozilla.org/build/pageloader/) which is installed per-run and allows us to cycle through pages.
Comment 5 Joel Maher ( :jmaher) 2010-09-21 18:22:18 PDT
this patch was originally written for cvs and I don't know how to remove a file in cvs, so I never changed the name from ffprocess_winmo.py to ffprocess_remote.py.  Now that we are moving to HG, this can be done.

Currently we don't support remote testing for pageloader tests.  If you want me to modify page laoder to use a similar dumpLog method, I can do that.  This patch is for the talos repository.  I have another patch which we landed for the fennecmark toolset to use the filename instead stdout.

Maybe I don't understand why this patch cannot land without support for pageloader since pageloader is a different tool?
Comment 6 Joel Maher ( :jmaher) 2010-09-21 20:05:49 PDT
Created attachment 477390 [details] [diff] [review]
log output to a log file, not stdout (2.0)

updated patch to be 'hg' patch.  Also I removed ffprocess_winmo.py and replaced it with ffprocess_remote.py!

Once I hear back regarding the direction on pageloader, I can r? this patch.
Comment 7 Joel Maher ( :jmaher) 2010-09-21 20:08:17 PDT
Created attachment 477391 [details] [diff] [review]
fix perfconfigurator.py to work with android and remote logging (1.0)

second patch that fixes PerfConfigurator.py to be smarter, work with remote_log options and support android in a cleaner method.  This patch removes winmo.config and creates remote.config.  

Here is an example commandline that I run to generate a valid config:
python PerfConfigurator.py -v -e org.mozilla.fennec -t `hostname` -b mobile-browser --activeTests tzoom --sampleConfig remote.config --browserWait 60 --noChrome --output local.config --remoteDevice 192.168.1.115 --webServer 192.168.1.102/talos
Comment 8 alice nodelman [:alice] [:anode] 2010-09-22 15:57:11 PDT
Comment on attachment 477391 [details] [diff] [review]
fix perfconfigurator.py to work with android and remote logging (1.0)

Little weird to have one log called remove_log and the other called talos.log - might want to choose a format there and stick with it.  With that said, still r+.
Comment 9 alice nodelman [:alice] [:anode] 2010-09-22 15:59:17 PDT
Oh, and you have the port set to a hardcoded default (20701) in both patches.  Could this be something that lives in the remote.config?
Comment 10 alice nodelman [:alice] [:anode] 2010-09-22 16:05:06 PDT
Re: comment #6, direction for pageloader.  I think that I'm still uncomfortable with how the code is set up as I don't believe that the dumpLog should be part of quit.js.  quit.js is for code that makes the browser stop, overloading how dump works shouldn't happen there.  I believe that this will generate confusion when people are reading code to figure out what is going on here.  I'd prefer that it would be packaged into a secondary script that could then be included by those tests that need access to it.
Comment 11 Joel Maher ( :jmaher) 2010-09-23 07:47:48 PDT
for the port being hardcoded to 20701, it is much more difficult and requires a lot more code to read it from the config file.  We need to collect information from the device (such as build id and deviceRoot) which requires the ip and port.  The solution to read it from the config file is to parse the config file and store it locally in an hash.  Then setup remote and do any variable validation cleanup.  Finally we would then write the new config out.

Currently we parse the variables (validate/cleanup) then in the write config step we read the sample.config and line by line edit on the fly as needed to create the new one.  

If we want to do this we can.  I would also change the variable input to be OptionParser or something like that so we can subclass.
Comment 12 Joel Maher ( :jmaher) 2010-09-23 14:29:11 PDT
Created attachment 478066 [details] [diff] [review]
log output to a log file, not stdout (3.0)

updated patch to use scripts/MozillaFileLogger.js instead of hacking up quit.js.  The one problem here is that we need to include MozillaFileLogger.js before quit.js to have to event generation code (or we could duplicate or create a 3rd file)

Tested this in a remote scenario as well as a hacked up scenario where we do and don't redirect firefox output to a log file.
Comment 13 Joel Maher ( :jmaher) 2010-09-23 14:35:16 PDT
Created attachment 478069 [details] [diff] [review]
fix perfconfigurator.py to work with android and remote logging (2.0)

addressed some of the concerns in previous comments.  Also pulled as much code as I could out of PerfConfigurator.py and into the existing remotePerfConfigurator.py.  Asking for another review since logic has changed.

this should help keep things separated as much as possible.

:bear, please run python remotePerfConfigurator.py now.
Comment 14 Joel Maher ( :jmaher) 2010-09-24 10:02:09 PDT
Created attachment 478305 [details] [diff] [review]
log output to a log file, not stdout (3.1)

forgot to add scripts/MozillaFileLogger.js to the patch...good catch anode!
Comment 15 alice nodelman [:alice] [:anode] 2010-09-24 10:07:17 PDT
Let's get another green staging cycle out of this and then we can consider it ready to go.
Comment 16 Joel Maher ( :jmaher) 2010-09-24 13:52:39 PDT
Created attachment 478389 [details] [diff] [review]
fix perfconfigurator.py to work with android and remote logging (2.1)

small adjustment to a text string needed for pageloader manifest mods.
Comment 17 Robert Sayre 2010-09-29 12:43:00 PDT
What needs to happen to check in these patches?
Comment 18 Joel Maher ( :jmaher) 2010-09-29 12:47:33 PDT
these patches are r+ and pass in talos staging, bug 579566 is the tracking bug for the next talos downtime.  It is my understanding that is schedule for sometime this week (usually thursday) and these two patches would land then.
Comment 19 Robert Sayre 2010-09-29 16:58:00 PDT
(In reply to comment #18)
> these patches are r+ and pass in talos staging, bug 579566 is the tracking bug
> for the next talos downtime.

That does not look like a tracking bug for downtime to me. When will this patch land?
Comment 20 alice nodelman [:alice] [:anode] 2010-09-30 15:22:37 PDT
Looks like a wrong bug number there - the downtime bug is bug 593081.  Downtimes are hard to come by these days as releng is currently blocked on completing code freeze for beta 7 - as you can see things in that bug are pretty backed up.

You'll have to talk to releng to get a better idea what their scheduling is looking like.
Comment 21 Robert Sayre 2010-09-30 15:37:46 PDT
(In reply to comment #20)
> 
> You'll have to talk to releng to get a better idea what their scheduling is
> looking like.

No, I will not. This bug is not assigned to me. What will happen is that releng and/or ateam will answer the very simple question I just asked. We are not going to wait week to land this code.
Comment 22 John O'Duinn [:joduinn] (please use "needinfo?" flag) 2010-09-30 16:21:44 PDT
(In reply to comment #19)
> (In reply to comment #18)
> > these patches are r+ and pass in talos staging, bug 579566 is the tracking bug
> > for the next talos downtime.
> 
> That does not look like a tracking bug for downtime to me. When will this patch
> land?
jmaher pointed you to the wrong bug. Bug#593081 is tracking scheduling of next downtime.




(In reply to comment #21)
> (In reply to comment #20)
> > You'll have to talk to releng to get a better idea what their scheduling is
> > looking like.
> No, I will not. This bug is not assigned to me. What will happen is that releng
> and/or ateam will answer the very simple question I just asked. We are not
> going to wait week to land this code.

To allow developers land beta7 fixes, RelEng is currently blocked from scheduling non-emergency treeclosure downtime. One possible downtime date being discussed is immediately after the "go to build" for beta7. I'm all-too-well-aware this yet-to-be-scheduled downtime is blocking 7 bugfixes from landing in production, including the one you care about. If you have additional info on why these need to be landed sooner, with a downtime interruption on other beta7 bugs, please let rel-drivers know.
Comment 23 Robert Sayre 2010-09-30 17:31:46 PDT
(In reply to comment #22)
>
> If you have
> additional info on why these need to be landed sooner, with a downtime
> interruption on other beta7 bugs, please let rel-drivers know.

There are about 20 b7 blockers left. We can tolerate a downtime. I am disturbed that I got the runaround three bug comments in a row here. That doesn't mean any one commenter was wrong to point somewhere else, but it does indicate a broken organization that passes the buck too easily. Since I am a release driver, I guess I will mail myself.
Comment 24 cmtalbert 2010-09-30 18:39:28 PDT
(In reply to comment #23)
> (In reply to comment #22)
> >
> > If you have
> > additional info on why these need to be landed sooner, with a downtime
> > interruption on other beta7 bugs, please let rel-drivers know.
> 
> There are about 20 b7 blockers left. We can tolerate a downtime. I am disturbed
> that I got the runaround three bug comments in a row here. That doesn't mean
> any one commenter was wrong to point somewhere else, but it does indicate a
> broken organization that passes the buck too easily. Since I am a release
> driver, I guess I will mail myself.

There's no run around.  The only visibility we have into the IT/RelEng downtime schedule is "every thursday morning, permitting something doesn't come up".  I can speak for the Tools team that it's not so much "passing the buck" as not knowing.  We only know when our stuff is landing the day before the down time happens.  

I think we have a critical lack of visibility into what needs downtime and why across the organization.  I'll make sure the next time we're waiting on something it's called out in that tree management section of the Tuesday meeting so it doesn't come up as a surprise.
Comment 25 Joel Maher ( :jmaher) 2010-09-30 18:47:05 PDT
just as a note, these patches are HG patches, I was under the impression we would be on HG last week, but that was pushed out to this week (although it isn't scheduled)

If we do a downtime tomorrow and it is on talos cvs, I could redo these patches to ensure they apply cleanly to a cvs checkout.
Comment 26 Robert Sayre 2010-09-30 18:51:59 PDT
(In reply to comment #24)
> 
> There's no run around.  The only visibility we have into the IT/RelEng downtime
> schedule is "every thursday morning, permitting something doesn't come up". 

It should have been today, then. But it didn't happen. Perhaps for good reason. I have mailed release-drivers to schedule time to get this patch and others in. This work is important, and we can't afford to let it sit for weeks.
Comment 27 Joel Maher ( :jmaher) 2010-10-01 10:21:06 PDT
pushed to hg repo:
http://hg.mozilla.org/build/talos/pushloghtml?changeset=f97b65124838

we will ensure that the talos.zip which releng requires is ready to go along with additional testing for a smoother process.
Comment 28 alice nodelman [:alice] [:anode] 2010-10-01 10:22:54 PDT
Removing dependency on downtime - this is dependent on talos in hg landing which will happen in the downtime.  Doing this to clear up confusion in the releng-downtime mega-bug.
Comment 29 Robert Sayre 2010-10-06 08:38:34 PDT
did this land during the downtime on monday?
Comment 30 Joel Maher ( :jmaher) 2010-10-06 08:42:01 PDT
yes, this did, I am not sure of the best way to indicate that in this bug. 

marking as resolved->fixed.
Comment 31 Robert Sayre 2010-10-08 00:24:42 PDT
(In reply to comment #23)
>
> There are about 20 b7 blockers left. We can tolerate a downtime. I am disturbed
> that I got the runaround three bug comments in a row here. That doesn't mean
> any one commenter was wrong to point somewhere else, but it does indicate a
> broken organization that passes the buck too easily. Since I am a release
> driver, I guess I will mail myself.

I gather that this message offended people. I am sorry. So, what happened? Near as I can tell, it went like this:

1) Engineering wanted Android automation
2) the team working on Android automation did the work (I don't know what the schedule looked like)
3) when the work was complete, Releng needed downtime to make the changes to accommodate Android automation
4) Engineering didn't accept any downtime, for unrelated reasons.

So, Engineering is making unreasonable and contradictory demands. However, it was not obvious that these demands were contradictory, and it took some rather impolitic prodding to figure out why everything was stopped up.

IMHO, that means there are two takeaways. The first problem is that engineering is incompetently micromanaging releng/automation logistics. The second problem is that releng/automation isn't making sure these very important projects go all the way to deployment.

These projects are critical to Mozilla's success, so we need to change. The worst result is mutual haplessness, where each group blames the other. The best outcome we can hope for is that each group has good stuff, and we need to coordinate the deployment of it all. Write good code and don't let it sit there.

The confusion in this bug never would have happened if everyone was making sure that their code was actually getting deployed properly. It's not cool to fix a bug in theory and then let some other group block you. In this case, the finger pointing actually formed a circle. That is unacceptable and unproductive.
Comment 32 Gervase Markham [:gerv] 2010-10-08 09:45:21 PDT
This comment is intended to be constructive.

sayrer: I think RelEng's actions were entirely reasonable if Engineering did not tell them that this work was of particular urgency. Surely if Engineering has given an instruction that there be no non-emergency downtime, and this fix was not labelled as being of sufficient importance to require emergency downtime, then adding it to the queue of fixes for the next release was the obvious thing to do.

The problem would then be not Engineering micromanaging RelEng, but that (as you hint) Engineering not coordinating within itself. If RelEng received what seemed to be a consistent set of instructions from Engineering (i.e. _not_ "contradictory demands"), you can't blame them for following them. It's not their job to say "hey, we wrote this code for you, and you guys aren't letting it be deployed. What gives?" They made the not unreasonable assumption that Engineering knew what they were doing.

OTOH, if RelEng were told that this issue was of utter, vital, drop-everything importance, then raising the question of whether emergency downtime was required would have been a step they should, in hindsight, have taken.

But aside from an unfortunate typo with a bug number, and joduinn not knowing you were yourself a release-driver, I don't see them giving you the run-around in this bug.

Gerv
Comment 33 cmtalbert 2010-10-11 13:08:19 PDT
Hey Rob,

Thanks for the summary here.

(In reply to comment #31)
> (In reply to comment #23)
 
> IMHO, that means there are two takeaways. The first problem is that engineering
> is incompetently micromanaging releng/automation logistics. The second problem
> is that releng/automation isn't making sure these very important projects go
> all the way to deployment.
> 
> These projects are critical to Mozilla's success, so we need to change. The
> worst result is mutual haplessness, where each group blames the other. The best
> outcome we can hope for is that each group has good stuff, and we need to
> coordinate the deployment of it all. Write good code and don't let it sit
> there.
> 
> The confusion in this bug never would have happened if everyone was making sure
> that their code was actually getting deployed properly. It's not cool to fix a
> bug in theory and then let some other group block you. In this case, the finger
> pointing actually formed a circle. That is unacceptable and unproductive.

I agree with you here.  I think that this is where we fell down - getting the stuff fixed and then waiting to see if the code would land.  I'll start encouraging my automation peeps to start thinking that the bug is not fixed until the code is landed and running, and hopefully we can work out these kinks in the system more quickly next time.

Note You need to log in before you can comment on or make changes to this bug.