Bug 99150 – [BlackBerry] Dump DRT output to stdout if test is passed as command line argument

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Xiaobo Wang]

As we discussed earlier, we would like to have an easy way to generate DRT
output for specific test case.

Now that DRT has its own launcher, we can pass the test case name as the
argument for drt-launcher.

RIM PR #222624

[Xiaobo Wang]

Created attachment 168379 [details]
patch

[Rob Buis]

Comment on attachment 168379 [details]
patch

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

> Tools/DumpRenderTree/blackberry/DumpRenderTree.cpp:339
> +    if (testFile) {

You can combine the above two lines.

[Xiaobo Wang]

Created attachment 168614 [details]
patch

combined 2 lines into 1.

[Rob Buis]

Comment on attachment 168614 [details]
patch

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

Patch is ok, but runFromCommandLine functionality should be put into a function.

> Tools/DumpRenderTree/blackberry/DumpRenderTree.cpp:224
> +    if (runFromCommandLine) {

I talked with Ming and we think it is nicer if this is a method instead, which does the getenv internally.

[Xiaobo Wang]

(In reply to comment #4)
> (From update of attachment 168614 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=168614&action=review
> 
> Patch is ok, but runFromCommandLine functionality should be put into a function.
> 
> > Tools/DumpRenderTree/blackberry/DumpRenderTree.cpp:224
> > +    if (runFromCommandLine) {
> 
> I talked with Ming and we think it is nicer if this is a method instead, which does the getenv internally.

It's a little bit complicated to create a new function in DumpRenderTree for runFromCommandLine and call it from main.cpp directly. We need to go through libwebview and several places in webkit to call into DumpRenderTree.cpp. The setenv/getenv solution is much easier. The environment variable drtTestFile has two meanings: 1 - the test to run; 2 - it indicates the test is being run from commandline and we should dump text to stdout instead of redirecting to the dump file.
I talked with Ming about this today, and he agreed with me.

[Rob Buis]

(In reply to comment #5)
> (In reply to comment #4)
> > (From update of attachment 168614 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=168614&action=review
> > 
> > Patch is ok, but runFromCommandLine functionality should be put into a function.
> > 
> > > Tools/DumpRenderTree/blackberry/DumpRenderTree.cpp:224
> > > +    if (runFromCommandLine) {
> > 
> > I talked with Ming and we think it is nicer if this is a method instead, which does the getenv internally.
> 
> It's a little bit complicated to create a new function in DumpRenderTree for runFromCommandLine and call it from main.cpp directly. We need to go through libwebview and several places in webkit to call into DumpRenderTree.cpp. The setenv/getenv solution is much easier. The environment variable drtTestFile has two meanings: 1 - the test to run; 2 - it indicates the test is being run from commandline and we should dump text to stdout instead of redirecting to the dump file.
> I talked with Ming about this today, and he agreed with me.

You are right, the patch is fine as it is.

[Rob Buis]

Comment on attachment 168614 [details]
patch

LGTM.

[WebKit Review Bot]

Comment on attachment 168614 [details]
patch

Clearing flags on attachment: 168614

Committed r131761: <http://trac.webkit.org/changeset/131761>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.