Bug 196567 - [Win] DumpRenderTreeWatchDog.py is not checked into source
Summary: [Win] DumpRenderTreeWatchDog.py is not checked into source
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Per Arne Vollan
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-04-03 15:07 PDT by Per Arne Vollan
Modified: 2020-07-13 08:19 PDT (History)
4 users (show)

See Also:


Attachments
Patch (3.27 KB, patch)
2019-04-03 15:18 PDT, Per Arne Vollan
no flags Details | Formatted Diff | Diff
Patch (3.28 KB, patch)
2019-04-03 17:05 PDT, Per Arne Vollan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Per Arne Vollan 2019-04-03 15:07:44 PDT
This script should be checked in. Tools/EWSTools is possibly an appropriate location for it.
Comment 1 Per Arne Vollan 2019-04-03 15:08:11 PDT
rdar://problem/49525901
Comment 2 Per Arne Vollan 2019-04-03 15:18:47 PDT
Created attachment 366654 [details]
Patch
Comment 3 Lucas Forschler 2019-04-03 16:21:37 PDT
Comment on attachment 366654 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=366654&action=review

> Tools/EWSTools/dumprendertreewatchdog.py:29
> +

these should be alphabetized

> Tools/EWSTools/dumprendertreewatchdog.py:37
> +    proc_fetch = subprocess.Popen("ps -e | grep \"DumpRenderTree\" | awk '{print $1}'", shell=True, stdout=subprocess.PIPE,)

I think this will also capture the ps process itself, right? On my machine with no DumpRenderTree processes running, this command will always return one pid, itself.

> Tools/EWSTools/dumprendertreewatchdog.py:47
> +        # If the active and current PID are the and an hour has elapsed, kill DumpRenderTree and reset the active PID to 0

I think this comment is missing a word between "the" and "and"
Comment 4 Lucas Forschler 2019-04-03 16:28:57 PDT
Comment on attachment 366654 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=366654&action=review

>> Tools/EWSTools/dumprendertreewatchdog.py:37
>> +    proc_fetch = subprocess.Popen("ps -e | grep \"DumpRenderTree\" | awk '{print $1}'", shell=True, stdout=subprocess.PIPE,)
> 
> I think this will also capture the ps process itself, right? On my machine with no DumpRenderTree processes running, this command will always return one pid, itself.

Also, would this be better named pid_fetch?

(I just tested on Windows and it appears the functionality of ps does not output the ps process itself... so maybe this is ok)
Comment 5 Per Arne Vollan 2019-04-03 17:05:06 PDT
Created attachment 366673 [details]
Patch
Comment 6 Per Arne Vollan 2019-04-03 17:09:55 PDT
(In reply to Lucas Forschler from comment #4)
> Comment on attachment 366654 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=366654&action=review
> 
> >> Tools/EWSTools/dumprendertreewatchdog.py:37
> >> +    proc_fetch = subprocess.Popen("ps -e | grep \"DumpRenderTree\" | awk '{print $1}'", shell=True, stdout=subprocess.PIPE,)
> > 
> > I think this will also capture the ps process itself, right? On my machine with no DumpRenderTree processes running, this command will always return one pid, itself.
> 
> Also, would this be better named pid_fetch?
> 
> (I just tested on Windows and it appears the functionality of ps does not
> output the ps process itself... so maybe this is ok)

Thanks for reviewing! I have updated the patch.
Comment 7 WebKit Commit Bot 2019-04-04 10:39:59 PDT
Comment on attachment 366673 [details]
Patch

Clearing flags on attachment: 366673

Committed r243879: <https://trac.webkit.org/changeset/243879>
Comment 8 WebKit Commit Bot 2019-04-04 10:40:00 PDT
All reviewed patches have been landed.  Closing bug.