[webkit-reviews] review denied: [Bug 133149] REGRESSION(r169092 and r169102): Skip failing JSC tests poperly on non-x86 Darwin platforms : [Attachment 231875] Patch

[bugzilla-daemon at webkit.org]

Filip Pizlo <fpizlo at apple.com> has denied Éva Balázsfalvi
<evab.u-szeged at partner.samsung.com>'s request for review:
Bug 133149: REGRESSION(r169092 and r169102): Skip failing JSC tests poperly on
non-x86 Darwin platforms
https://bugs.webkit.org/show_bug.cgi?id=133149

Attachment 231875: Patch
https://bugs.webkit.org/attachment.cgi?id=231875&action=review

------- Additional Comments from Filip Pizlo <fpizlo at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=231875&action=review


It looks good but needs some stylistic changes.

> LayoutTests/js/script-tests/function-apply-many-args.js:1
> -//@ skip if $architecture !~ /x86/i
> +//@ skip if $architecture !~ /x86/i && $host_os == "darwin"

small nit: we typically say "and" instead of "&&" in our ruby code.

> PerformanceTests/SunSpider/profiler-test.yaml:32
> -	 if $architecture =~ /x86/
> -	     runProfiler
> -	 else
> +	 if $architecture !~ /x86/i && $host_os == "darwin"
>	     skip
> +	 else
> +	     runProfiler

ditto

> Source/JavaScriptCore/tests/mozilla/mozilla-tests.yaml:2119
> -	 if $architecture =~ /x86/i
> -	     defaultRunMozillaTest :normal, "../shell.js"
> -	 else
> +	 if $architecture !~ /x86/i && $host_os == "darwin"
>	     skip
> +	 else
> +	     defaultRunMozillaTest :normal, "../shell.js"

ditto

> Tools/Scripts/run-jsc-stress-tests:231
> +def determineOS
> +    case RbConfig::CONFIG["host_os"]
> +    when /darwin/i
> +	   "darwin"
> +    when /linux/i
> +	   "linux"
> +    when /mswin|mingw|cygwin/
> +	   "windows"
> +    else
> +	   $stderr.puts "Warning: unable to determine host operating system"
> +	   nil
> +    end
> +end
> +
>  $architecture = determineArchitecture
> +$host_os = determineOS

We usually use camelCase rather than under_scores in our Ruby code.  Please
call this $hostOS.