Bug 127694 – Upstream iOS old-run-webkit-tests for use with DumpRenderTree in the iOS Simulator

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

[David Farler]

As a short path to get simulator layout tests up and running, copy necessary mechanisms for running DumpRenderTree in the simulator to WebKitTestRunner.

[David Farler]

Retitling - This bug will focus on just upstreaming ORWT and getting it to work with DumpRenderTree. I'll use a separate bug to alter ORWT to work with WebKitTestRunner and WebKit2, since there will be related changes in WTR itself to open FIFOs, etc.

[David Farler]

Created attachment 223972 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 223972 [details]
Patch

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

> Tools/Scripts/old-run-webkit-tests:140
> +my $expectedResultsDir = '';  # PLATFORM_IOS

Can the variable have iOS in to remove the need for the comment?

> Tools/Scripts/old-run-webkit-tests:227
> +my $haveInstalledDumpRenderTreeAppOnce; # PLATFORM_IOS

Ditto.

[David Kilzer (:ddkilzer)]

Comment on attachment 223972 [details]
Patch

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

r=me, but consider comments before landing.

> Tools/DumpRenderTree/ios/PerlSupport/Makefile:54
> -OS_X_SDK = $(if $(shell xcrun --show-sdk-path -sdk "macosx" 2> /dev/null),macosx, /)
> -
>  $(DYLIB): DumpRenderTreeSupport.c $(WRAPPER)
> -	TOOLCHAINS= xcrun -sdk $(OS_X_SDK) cc -g -dynamiclib -o $(DYLIB) `$(PERL) -MExtUtils::Embed -eperl_inc` `$(PERL) -MExtUtils::Embed -e'my $$opts = ldopts(0); $$opts =~ s/-arch [^ ]*( |$$)//g; print $$opts, " -arch ", join(" -arch ", split(" ",$$ENV{ARCHS}))'` $^
> +	xcrun -sdk macosx clang -g -dynamiclib -o $(DYLIB) `$(PERL) -MExtUtils::Embed -eperl_inc` `$(PERL) -MExtUtils::Embed -e'my $$opts = ldopts(0); $$opts =~ s/-arch [^ ]*( |$$)//g; print $$opts, " -arch ", join(" -arch ", split(" ",$$ENV{ARCHS}))'` $^

I guess we assume a macosx SDK is always installed now?  This had a workaround for building when no macosx SDK was installed.

> Tools/DumpRenderTree/ios/PerlSupport/Makefile:65
> -	(cd IPhoneSimulatorNotification && \
> -		SDKROOT=$(OS_X_SDK) TOOLCHAINS= $(PERL) Makefile.PL INSTALL_BASE=$(OUTPUT_DIR) && \
> -		make SDKROOT=$(OS_X_SDK) TOOLCHAINS= && \
> -		make test && \
> -		make install && \
> -		make realclean)
> +	cd IPhoneSimulatorNotification && SDKROOT=iphonesimulator perl Makefile.PL INSTALL_BASE=$(OUTPUT_DIR) 
> +	make -C IPhoneSimulatorNotification test
> +	make -C IPhoneSimulatorNotification install
> +	make -C IPhoneSimulatorNotification realclean

The goal of putting the 'cd' command inside a subshell was so that the Makefile would not stay in that subdirectory after the command was done.

Do we need a "cd .." after the last make command to restore that behavior?

> Tools/Scripts/old-run-webkit-tests:230
>  my @macPlatforms = ("mac-leopard", "mac-snowleopard", "mac-lion", "mac");

I think this should be:

my @macPlatforms = ("mac-mountainlion", "mac");

> Tools/Scripts/old-run-webkit-tests:234
> +    # FIXME: No support for iphone-hardware yet.

We could remove this FIXME comment.

> Tools/Scripts/old-run-webkit-tests:314
> +  -e|--expected-results           Specify an alternate directory for expected results (PLATFORM_IOS)

The reason for the "PLATFORM_IOS" was to clearly demarcate any iOS-specific code.  Not sure if it's worth keeping these comments around any longer, or changing it to "(iOS-only)" in this case.

> Tools/Scripts/old-run-webkit-tests:365
> +    'expected-results|e=s' => \$expectedResultsDir,  # PLATFORM_IOS 

You can just remove the "# PLATFORM_IOS" comment here.

> Tools/Scripts/old-run-webkit-tests:1261
> +  # Don't use run-safari, since it tries to load iOS frameworks in desktop Safari.

Nit:  s/desktop/Mac/

> Tools/Scripts/old-run-webkit-tests:1458
> +    system "osascript", "-e", "tell application \"iPhone Simulator\" to quit";

From this date henceforth, I disavow all knowledge of the author of this line of code.

> Tools/Scripts/old-run-webkit-tests:1928
> +        push @extraPlatforms, qw(mac-snowleopard mac);

This should be:

    push @extraPlatforms, qw(mac-mountainlion mac);

Or even better:

    push @extraPlatforms, @macPlatforms;

[David Farler]

Comment on attachment 223972 [details]
Patch

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

>> Tools/DumpRenderTree/ios/PerlSupport/Makefile:54
>> +	xcrun -sdk macosx clang -g -dynamiclib -o $(DYLIB) `$(PERL) -MExtUtils::Embed -eperl_inc` `$(PERL) -MExtUtils::Embed -e'my $$opts = ldopts(0); $$opts =~ s/-arch [^ ]*( |$$)//g; print $$opts, " -arch ", join(" -arch ", split(" ",$$ENV{ARCHS}))'` $^
> 
> I guess we assume a macosx SDK is always installed now?  This had a workaround for building when no macosx SDK was installed.

Yes, I hope we can assume that, since you need Xcode to target the simulator and Xcode always comes with the OS X SDK. `xcrun -sdk / -find clang` points to /Applications/Xcode.app/Contents/Developer/Toolchains/OSX10.9.xctoolchain/usr/bin/clang.

>> Tools/DumpRenderTree/ios/PerlSupport/Makefile:65
>> +	make -C IPhoneSimulatorNotification realclean
> 
> The goal of putting the 'cd' command inside a subshell was so that the Makefile would not stay in that subdirectory after the command was done.
> 
> Do we need a "cd .." after the last make command to restore that behavior?

make -C will always return to the originating caller's cwd. Only if you specify multiple -C options in one invocation are they considered relative to each other.

>> Tools/Scripts/old-run-webkit-tests:1458
>> +    system "osascript", "-e", "tell application \"iPhone Simulator\" to quit";
> 
> From this date henceforth, I disavow all knowledge of the author of this line of code.

# David Kilzer (ddkilzer) wuz here

[David Farler]

Committed r163998: <http://trac.webkit.org/changeset/163998>