Bug 190537 – [Tools][webkitpy] fix handling of JSCTESTS_OPTIONS

• Component: Tools / Tests
• Status: RESOLVED FIXED

[jsc-armv7 EWS]

In DeprecatedPort.run_javascriptcore_tests_command() JSCTESTS_OPTIONS needs to be split before adding it to the command since it is treated as a list of args.

[Guillaume Emont]

Created attachment 352204 [details]
Patch

The patch.

[jsc-armv7 EWS]

(In reply to Guillaume Emont from comment #1)
> Created attachment 352204 [details]
> Patch
> 
> The patch.

I think (hope) the jsc-armv7 EWS should get green if this works.

[jsc-armv7 EWS]

(In reply to Guillaume Emont (jsc-armv7-ews) from comment #2)
> (In reply to Guillaume Emont from comment #1)
> > Created attachment 352204 [details]
> > Patch
> > 
> > The patch.
> 
> I think (hope) the jsc-armv7 EWS should get green if this works.

...Except webkitpy changes are skipped.

[Alexey Proskuryakov]

Comment on attachment 352204 [details]
Patch

The patch looks good.

I’m not so sure about the idea of passing options via environment. The port object is supposed to implement the knowledge of how to run tools, and passing arguments verbatim violates incapsulation. 

Why was this approach chosen?

[jsc-armv7 EWS]

(In reply to Alexey Proskuryakov from comment #4)
> Comment on attachment 352204 [details]
> Patch
> 
> The patch looks good.
> 
> I’m not so sure about the idea of passing options via environment. The port
> object is supposed to implement the knowledge of how to run tools, and
> passing arguments verbatim violates incapsulation. 
> 
> Why was this approach chosen?

The goal is to be able to pass a --remote-config-file option so that run-javascriptcore-tests runs the test on the correct devices, which are dependent on the machine where it is run and not on the port.

[jsc-armv7 EWS]

(In reply to Guillaume Emont (jsc-armv7-ews) from comment #5)
> (In reply to Alexey Proskuryakov from comment #4)
> > Comment on attachment 352204 [details]
> > Patch
> > 
> > The patch looks good.
> > 
> > I’m not so sure about the idea of passing options via environment. The port
> > object is supposed to implement the knowledge of how to run tools, and
> > passing arguments verbatim violates incapsulation. 
> > 
> > Why was this approach chosen?
> 
> The goal is to be able to pass a --remote-config-file option so that
> run-javascriptcore-tests runs the test on the correct devices, which are
> dependent on the machine where it is run and not on the port.

Also, I'd love if that could be a parameter to webkit-patch <name of ews queue>, but I could not find a simple way to do that, and saw that there was a precedent with MAKEFLAGS.

[WebKit Commit Bot]

Comment on attachment 352204 [details]
Patch

Clearing flags on attachment: 352204

Committed r237089: <https://trac.webkit.org/changeset/237089>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/45241227>