[webkit-reviews] review granted: [Bug 206389] It should be possible to build JavaScriptCore with LLVM Source-based Code Coverage, run the tests and see the coverage data : [Attachment 388000] proposed patch

[bugzilla-daemon at webkit.org]

Saam Barati <sbarati at apple.com> has granted Tuomas Karkkainen
<tuomas.webkit at apple.com>'s request for review:
Bug 206389: It should be possible to build JavaScriptCore with LLVM
Source-based Code Coverage, run the tests and see the coverage data
https://bugs.webkit.org/show_bug.cgi?id=206389

Attachment 388000: proposed patch

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




--- Comment #5 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 388000
  --> https://bugs.webkit.org/attachment.cgi?id=388000
proposed patch

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

r=me

Nice!

> Tools/Scripts/run-javascriptcore-tests:505
> +  $coverageDir = tempdir();
> +  $htmlDir = File::Spec->catfile($coverageDir, "html_report");
> +  $profdataPath = File::Spec->catfile($coverageDir, "jsc_tests.profdata");
> +  $envVars .=  " LLVM_PROFILE_FILE=" . File::Spec->catfile($coverageDir,
"jsc_test_%9m.profraw");

nit: 4 spaces (there might be more of these I miss)

> Tools/Scripts/run-javascriptcore-tests:669
> +    unshift @command, "xcrun", "llvm-cov", "show",
builtDylibPathForName("JavaScriptCore"), "--format=html",
"--instr-profile=".$profdataPath, "--output-dir=".$htmlDir;

nit: space on the left/right of your "." concats

> Tools/Scripts/run-javascriptcore-tests:675
> +    print("Coverage report is in ".$htmlDir."\n");

ditto

> Tools/Scripts/run-javascriptcore-tests:680
> +  convertProfrawToProfdata();
> +  generateHtmlFromProfdata();

nit: 4 spaces

> Tools/Scripts/run-javascriptcore-tests:697
> +  processCoverageData();

nit: this should be four spaces

> Tools/Scripts/webkitdirs.pm:909
> +    die "cannot enabled both ASAN and Coverage at this time\n" if
$coverageIsEnabled && $asanIsEnabled;

"enabled" => "enable"