Closed Bug 454999 Opened 16 years ago Closed 12 years ago

Standalone talos kills all running Firefox processes

Categories

(Testing :: Talos, defect, P5)

defect

Tracking

(Not tracked)

RESOLVED WONTFIX

People

(Reporter: bzbarsky, Unassigned)

References

Details

(Whiteboard: [talos][needs staging])

Attachments

(1 file)

I just tried running standalone talos to test something, and it killed every single Firefox-like process I had running (including several things I was in the middle of debugging, etc). I don't see why it's killing anything other than instances of the binary it was told to run, honestly....
And the problem with this, of course, is it means I have to completely stop all work on everything else if I want to run this thing, which means I'll try hard as I can to never run it.
Severity: normal → major
While I can see that it should probably not go around killing stuff without a warning - any data you collect while actively working on a machine is not going to be useful. Talos is meant to be run on a set up where nothing else is happening so as to reduce variance, if you attempt to run it while on a busy machine you'll mostly know whether the tests complete but the numbers collected won't be meaningful.
Component: Testing → Release Engineering: Talos
Product: Core → mozilla.org
QA Contact: testing → release
Version: Trunk → other
Oh, agreed. I was trying to run talos because it was going orange on Tinderbox, so I didn't really care about the performance data: I just wanted to find the crash or hang that it was running into....
We intentionally kill off other firefox processes, as part of cleaning up before running performance tests. One option would be to throw warnings and not allow talos to run?
Component: Release Engineering: Talos → Release Engineering: Future
OS: Mac OS X → All
Priority: -- → P3
Hardware: PC → All
I understand why it's done. I just don't think it necessarily makes sense for _standalone_ (as opposed to tinderbox) talos.
As a stop gap I've updated the standalone talos code to V1.3. In this version the user is warned that all open firefox processes must be closed before testing can start. This stops talos from unexpectedly killing off browser windows and allows the user to shut them down cleanly. Still not as nice as being able to leave existing firefox processes open - but I think that it is a good half way point.
Alice, that sounds much better, yes. I do have one question: when I ran talos, it killed all my browsers every so often even in mid-test (that is, I restarted some browsers after the test started, and at some point they got killed again). Will that now just prompt the user again?
After each individual test is run talos goes through a clean up phase where it kills any existing firefox/crashreporter/etc processes lingering around. I've only added protection at the beginning of the test run so that a given user can cleanly shut down their browsers before starting. If you start a browser window while a test is being run it will end up being killed off during clean up. You might be happier playing around with the sample.config and running each test individually (just comment out the other tests in the list) so that there is less time that you need to be browser free.
Mass move of bugs from Release Engineering:Future -> Release Engineering. See http://coop.deadsquid.com/2010/02/kiss-the-future-goodbye/ for more details.
Component: Release Engineering: Future → Release Engineering
Found in triage. After comment#8, is there anything left to do here?
Priority: P3 → P5
Yes. There's still the fact that I can't run talos locally without it killing my browsers, which is what the bug was originally filed on. In particular it means that I can't run it without it making my computed unusable while it runs. That sems rather unnecessary.
Hm, maybe an option that says killAllProcs=True, that bz can set to False?
Attached patch run_dirty optionSplinter Review
This patch lets you run standalone without killing your running browser. The run_dirty option disables some checks and tries to kill by pid rather than process name. Tested on my macbook, running Ts only. Also removed a lot of trailing whitespace.
Attachment #444046 - Flags: feedback?(anodelman)
This looks like a good approach - if it passes staging tests I'm totally on board.
Just noticed I had an extra |if self.runDirty: return| in TerminateAllProcesses on the mac but not on win32 and linux. Not sure if that matters.
Whiteboard: [talos] → [talos][needs staging]
This is about standalone talos only. There is no RelEng automation involved here, so nothing for RelEng to do here. Moving to Testing:General as requested by bmoss.
Component: Release Engineering → General
Product: mozilla.org → Testing
QA Contact: release → general
Version: other → unspecified
Moving to Talos component.
Component: General → Talos
Comment on attachment 444046 [details] [diff] [review] run_dirty option This has most certainly bitrotted since it was posted. I'm also unsure if having multiple firefox processes running at the same time will confuse the metric collection code in talos - that should be taken into account when attempting to run dirty.
Attachment #444046 - Flags: feedback?(anodelman) → feedback-
See also bug 700722 for a more surgical way of getting processes
Comment on attachment 444046 [details] [diff] [review] run_dirty option While I like returning the pid, I'd prefer taking runDirty -- that is, not cleaning up Firefox -- as part of --develop, and would in general like to ensure that, when possible/convenient, that running Talos locally and running in production follow the same pattern. I largely recommend the fix from bug 700722
See Also: → 700722
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: