Closed Bug 542000 Opened 13 years ago Closed 12 years ago

get_pids still depends on pywin32

Categories

(Testing :: Mozbase, defect)

x86
Windows 7
defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: rain1, Assigned: avarma)

References

Details

(Whiteboard: [verified-mozmill-1.4.1])

Attachments

(1 file)

http://github.com/mikeal/mozrunner/blob/master/mozrunner/__init__.py#L107

I don't seem to hit this while running Thunderbird tests, but the problem is still there.
I believe the problem we have here is the following line:

if os.name == 'nt' or sys.platform == 'cygwin':

Seems like it only occurs when you are using an old cygwin setup. Heather, can you please enlighten us on your setup?
Running from Mozilla Build 1.4, and calling stop() on a Runner instance.
Assignee: nobody → avarma
Status: NEW → ASSIGNED
Blocks: 546678
What's up with the references to win32api starting at line 127 of the same file?

http://github.com/mikeal/mozrunner/blob/master/mozrunner/__init__.py#L127

I'm not seeing that module imported anywhere, or a variable assigned to its name, etc.  Same with win32con, which is definitely a pywin32 module.  I guess those should both be pretty easy to migrate to ctypes, but still kinda weird that they're there...
I'm looking at alternatives to porting win32pdhutil.FindPerformanceAttributesByName() to ctypes, since it looks like doing that could involve a fair bit of work. It seems like it's just being used to get the process IDs of particular programs, which will then just be used to kill the programs in question--but there's a command-line utility that does the same thing, which comes with Windows XP and Server 2003 at the very least, at %windir%\system32\taskkill.exe.

Anyone have objections to just replacing these pywin32 calls with a command-line call to taskkill? Or do we still need to use mozrunner on win2k, which taskkill may not be shipped with?
Never mind my last comment, I think we can accomplish the same thing using simpler win32 APIs. Here's a proof-of-concept that does the same thing using ctypes instead of pywin32.
I created this module on a Windows 7 VM, but need to test it out on an older version of Windows to make sure it works: MSDN docs seem to indicate that some of the API calls I found in psapi.dll may instead be found in kernel32.dll in earlier versions of Windows.
I don't think mozmill automation needs to run on windows 2000, so don't let that block you.
Cool, thanks. I just ran the proof-of-concept code on my XP machine and it works without any changes, so I think I'll start integrating this into mozmill and issue a pull request to mikeal on github later today.
It looks like mikeal accepted the pull request and my fix is in his repo. Should we mark this bug as fixed?
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Awesome!

Um, I should note, though, that we have been experiencing a few other issues with Mozrunner, or at least with mozrunner.killableprocess.  The logic for killableprocess.Popen.wait(), in particular, seems unreliable and/or incorrect, and I've been working on a "better" version in bug 554842. While the patch is against the Jetpack SDK right now, I think it might be useful to integrate it into mozrunner proper. It also comes with some simple doctests that actually fail without the "better" version.

Another mozrunner-related bug I'm looking into is mentioned in bug 554570 comment 2.

I should probably just make a tracking bug for all Jetpack SDK-related mozrunner issues.
(In reply to comment #11)
> Awesome!
> 
> Um, I should note, though, that we have been experiencing a few other issues
> with Mozrunner, or at least with mozrunner.killableprocess.  The logic for
> killableprocess.Popen.wait(), in particular, seems unreliable and/or incorrect,
> and I've been working on a "better" version in bug 554842. While the patch is
> against the Jetpack SDK right now, I think it might be useful to integrate it
> into mozrunner proper. It also comes with some simple doctests that actually
> fail without the "better" version.
> 
Awesome
> Another mozrunner-related bug I'm looking into is mentioned in bug 554570
> comment 2.
> 
cool.
> I should probably just make a tracking bug for all Jetpack SDK-related
> mozrunner issues.
That'd be nice, but also put [p1] in the whiteboard list of any bugs that you need fixed for jetpack so that they will get picked up in our queries for high priority mozmill system bugs.
(In reply to comment #12)

> That'd be nice, but also put [p1] in the whiteboard list of any bugs that you
> need fixed for jetpack so that they will get picked up in our queries for high
> priority mozmill system bugs.
Actually [jetpack-p1] would be best. Thx.
Verified using Mozrunner 2.4.2 with my regression range finder script. Thanks Atul!
Status: RESOLVED → VERIFIED
Whiteboard: [mozmill-1.4.1] → [verified-mozmill-1.4.1]
You need to log in before you can comment on or make changes to this bug.