[webkit-reviews] review denied: [Bug 174985] [Win][JSCOnly] Support running JSC tests for win-cairo from windows command prompt : [Attachment 318643] Adding test-writer that generates ruby scripts and ruby runner for windows command prompt

[bugzilla-daemon at webkit.org]

Keith Miller <keith_miller at apple.com> has denied Stephan Szabo
<stephan.szabo at sony.com>'s request for review:
Bug 174985: [Win][JSCOnly] Support running JSC tests for win-cairo from windows
command prompt
https://bugs.webkit.org/show_bug.cgi?id=174985

Attachment 318643: Adding test-writer that generates ruby scripts and ruby
runner for windows command prompt

https://bugs.webkit.org/attachment.cgi?id=318643&action=review




--- Comment #8 from Keith Miller <keith_miller at apple.com> ---
Comment on attachment 318643
  --> https://bugs.webkit.org/attachment.cgi?id=318643
Adding test-writer that generates ruby scripts and ruby runner for windows
command prompt

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

Looks pretty good. r- since I think the runner-script api could be cleaned up a
bit though.

> Tools/Scripts/run-jsc-stress-tests:180
> +		  ['--test-writer', GetoptLong::REQUIRED_ARGUMENT],

I don't think you need this argument. It seems like you should just pick the
ruby test writer if --ruby-runner is provided.

> Tools/Scripts/run-jsc-stress-tests:454
> +require_relative "webkitruby/jsc-stress-test-writer-#{$testWriter}"

This should be decided based on the runner being used.

> Tools/Scripts/webkitruby/jsc-stress-test-writer-default.rb:355
> +def prepareRubyTestRunner
> +    File.open($runnerDir + "runscript", "w") {
> +	   | outp |
> +	   $runlist.each {
> +	       | plan |
> +	       outp.puts "print `sh test_script_#{plan.index} 2>&1`"
> +	   }
> +    }
> +end

You should delete this.

> Tools/Scripts/webkitruby/jsc-stress-test-writer-default.rb:364
> +    when :ruby
> +	   command = "ruby runscript"

Ditto.