Bug 96369

Summary: Recognize the "--chromium-android" argument in run-webkit-tests
Product: WebKit Reporter: Peter Beverloo <peter>
Component: New BugsAssignee: Peter Beverloo <peter>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, dpranke, ojan, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 84845    
Attachments:
Description Flags
Patch
none
Patch for landing none

Description Peter Beverloo 2012-09-11 02:35:34 PDT
Recognize the "--chromium-android" argument in run-webkit-tests
Comment 1 Peter Beverloo 2012-09-11 02:36:54 PDT
Created attachment 163312 [details]
Patch
Comment 2 jochen 2012-09-11 02:46:48 PDT
Comment on attachment 163312 [details]
Patch

Can you add a test to factory_unittest.py (see FactoryTest.assert_port)
Comment 3 Peter Beverloo 2012-09-11 03:07:20 PDT
Created attachment 163320 [details]
Patch for landing
Comment 4 Peter Beverloo 2012-09-11 03:07:37 PDT
(In reply to comment #2)
> (From update of attachment 163312 [details])
> Can you add a test to factory_unittest.py (see FactoryTest.assert_port)

Done, thank you!
Comment 5 Peter Beverloo 2012-09-11 03:09:43 PDT
Committed r128164: <http://trac.webkit.org/changeset/128164>
Comment 6 Dirk Pranke 2012-09-11 10:22:46 PDT
Is this really much of an improvement over '--platform chromium-android', which already worked? The reason we have the other shortcuts (--qt, --chromium) etc. is not primarily because they're shorter, but rather to indicate "pick the appropriate variant of X for the platform I'm running on".
Comment 7 Peter Beverloo 2012-09-11 10:25:24 PDT
(In reply to comment #6)
> Is this really much of an improvement over '--platform chromium-android', which already worked? The reason we have the other shortcuts (--qt, --chromium) etc. is not primarily because they're shorter, but rather to indicate "pick the appropriate variant of X for the platform I'm running on".

Jochen asked me the same question. I chose to add the argument because it's consistent with the other flags, and with the input of the run-webkit-tests script which had to pass this along.

I'd be happy to change this.
Comment 8 Dirk Pranke 2012-09-11 10:32:22 PDT
(In reply to comment #7)
> (In reply to comment #6)
> > Is this really much of an improvement over '--platform chromium-android', which already worked? The reason we have the other shortcuts (--qt, --chromium) etc. is not primarily because they're shorter, but rather to indicate "pick the appropriate variant of X for the platform I'm running on".
> 
> Jochen asked me the same question. I chose to add the argument because it's consistent with the other flags, and with the input of the run-webkit-tests script which had to pass this along.
> 
> I'd be happy to change this.

I don't feel strongly enough about this to want to revert it, but I'm not sure where this slope stops slipping ...
Comment 9 Peter Beverloo 2012-09-11 10:36:23 PDT
(In reply to comment #8)
> (In reply to comment #7)
> > (In reply to comment #6)
> > > Is this really much of an improvement over '--platform chromium-android', which already worked? The reason we have the other shortcuts (--qt, --chromium) etc. is not primarily because they're shorter, but rather to indicate "pick the appropriate variant of X for the platform I'm running on".
> > 
> > Jochen asked me the same question. I chose to add the argument because it's consistent with the other flags, and with the input of the run-webkit-tests script which had to pass this along.
> > 
> > I'd be happy to change this.
> 
> I don't feel strongly enough about this to want to revert it, but I'm not sure where this slope stops slipping ...

If there are issues you feel we should be addressing then we should be talking about that. I value your input and concerns, so they'd be most welcome. Since you're comparing Android to a slipping slope it seems you have plenty.

I explained my rationale for making this change. I'll upload a patch to change this to --platform=chromium-android momentarily.
Comment 10 Dirk Pranke 2012-09-11 12:40:38 PDT
(In reply to comment #9)
> (In reply to comment #8)
> > (In reply to comment #7)
> > > (In reply to comment #6)
> > > > Is this really much of an improvement over '--platform chromium-android', which already worked? The reason we have the other shortcuts (--qt, --chromium) etc. is not primarily because they're shorter, but rather to indicate "pick the appropriate variant of X for the platform I'm running on".
> > > 
> > > Jochen asked me the same question. I chose to add the argument because it's consistent with the other flags, and with the input of the run-webkit-tests script which had to pass this along.
> > > 
> > > I'd be happy to change this.
> > 
> > I don't feel strongly enough about this to want to revert it, but I'm not sure where this slope stops slipping ...
> 
> If there are issues you feel we should be addressing then we should be talking about that. I value your input and concerns, so they'd be most welcome. Since you're comparing Android to a slipping slope it seems you have plenty.
> 
> I explained my rationale for making this change. I'll upload a patch to change this to --platform=chromium-android momentarily.

Don't read too much into my comment, I'm not upset about the change, and I wouldn't bother uploading another patch.

My point was, I'm not sure how we decide to add --chromium-android and not add --chromium-mac or --chromium-mac-lion, but adding a flag for every possible port seems like the wrong solution.

Generally, I only like to add command line flags either when required to support new functionality or when they are shortcuts to really commonly used sets of other options (e.g., supporting --debug rather than requiring --configuration debug) that will make things easier for a lot of people. --chromium-android doesn't fall into the first category and it's not clear to me that enough people are going to use it that it'll fall into the latter.