68511 2011-09-21 01:52:56 -0700 [EFL] Script for running WebKit-EFL Unit Tests 2012-07-03 21:09:30 -0700 1 1 1 Unclassified WebKit WebKit EFL 528+ (Nightly build) All Linux RESOLVED DUPLICATE 87251 P2 Enhancement --- 86092 0 k.czech webkit-unassigned gyuyoung.kim gyuyoung.kim lucas.de.marchi rakuco tmpsantos oldest_to_newest 470663 0 k.czech 2011-09-21 01:52:56 -0700 Support for running unit tests automatically. 470664 1 108124 k.czech 2011-09-21 01:54:30 -0700 Created attachment 108124 Patch 470669 2 gyuyoung.kim 2011-09-21 02:01:34 -0700 I think it is better to notify this contribution to webkit-dev mailing list first. 470931 3 108124 leandro 2011-09-21 10:53:56 -0700 Comment on attachment 108124 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=108124&action=review Informal r- for the reasons below. > Tools/Scripts/run-efl-tests:52 > +sub runTest > +{ > + my $args = @_[0]; > + my $pathToBinary = "Source/Programs/EUnitTests"; > + my $utName = ""; > + if (length($args) > 0) { > + $utName = " --gtest_filter=" . $args . ".*" . "\n"; > + } > + exit system($pathToBinary . $utName); My Perl-fu is a bit rusty, but wouldn't the following be more readable and less prone to escaping problems? sub runTest { my ($testFilter) = @_; my $pathToTestUtility = 'Source/Programs/EUnitTests'; my @testArgs = ($pathToTestUtility); push @testArgs, ('--gtest_filter', "${testFilter}.*\n") if $testFilter; return system(@testArgs); } > Tools/Scripts/run-efl-tests:57 > +sub usage > +{ > + my $programName = basename($0); Even though this is fine, the common pattern in WebKitPerl is to declare subroutine parameters as such (notice the more descriptive name as well): sub printUsage { my ($programName) = @_; ... } > Tools/Scripts/run-efl-tests:64 > + print STDERR $usageText; Why is this printed to STDERR? > Tools/Scripts/run-efl-tests:65 > + exit 1; Why does the program return 1 when successfully printing the help output? 471413 4 k.czech 2011-09-22 01:01:14 -0700 (In reply to comment #3) > (From update of attachment 108124 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=108124&action=review > > Informal r- for the reasons below. > > > Tools/Scripts/run-efl-tests:52 > > +sub runTest > > +{ > > + my $args = @_[0]; > > + my $pathToBinary = "Source/Programs/EUnitTests"; > > + my $utName = ""; > > + if (length($args) > 0) { > > + $utName = " --gtest_filter=" . $args . ".*" . "\n"; > > + } > > + exit system($pathToBinary . $utName); > > My Perl-fu is a bit rusty, but wouldn't the following be more readable and less prone to escaping problems? > > sub runTest { > my ($testFilter) = @_; > my $pathToTestUtility = 'Source/Programs/EUnitTests'; > my @testArgs = ($pathToTestUtility); > > push @testArgs, ('--gtest_filter', "${testFilter}.*\n") if $testFilter; > return system(@testArgs); > } > Your suggestion is good, I'm bit new to the perl so your Perl-fu is better than mine > > Tools/Scripts/run-efl-tests:57 > > +sub usage > > +{ > > + my $programName = basename($0); > > Even though this is fine, the common pattern in WebKitPerl is to declare subroutine parameters as such (notice the more descriptive name as well): > > sub printUsage > { > my ($programName) = @_; > ... > } > > > Tools/Scripts/run-efl-tests:64 > > + print STDERR $usageText; > > Why is this printed to STDERR? Allows stream redirection. "Help" in this context informs the user, that something did wrong. > > > Tools/Scripts/run-efl-tests:65 > > + exit 1; > > Why does the program return 1 when successfully printing the help output? Point at, this is a failure situation 473100 5 108666 k.czech 2011-09-26 07:49:00 -0700 Created attachment 108666 Patch 486844 6 108666 leandro 2011-10-19 09:31:49 -0700 Comment on attachment 108666 Patch LGTM. 528390 7 rakuco 2012-01-01 17:20:24 -0800 Coming to think of it again, I wonder if this script is really needed. Wouldn't using CMake's infrastructure for tests be enough (ie. running `make test' then runs the unit tests)? 528453 8 k.czech 2012-01-02 03:19:19 -0800 I also thought about it. On the other hand, there are already available similar scripts for other ports: run-gtk-tests, run-chromium-webkit-unit-tests, run-api-tests. I'am not sure if this is some kind of convention of adding this kind of scripts. Apart from this we could also use CMake infrastructure. 528652 9 gyuyoung.kim 2012-01-02 20:29:28 -0800 If there is a script for running EFL test, I'm please with it. But, it looks this patch can't be landed until other patches are landed. In addition, I think you need to notify this patch to webkit-dev. I think you have to get agreement from webkit community or reviewers. 659465 10 tmpsantos 2012-06-28 06:47:50 -0700 I'm not sure if this makes sense anymore. I think it can be done by using CTest and a build target. We don't have a global runner anymore. See bug 87251. 661636 11 tmpsantos 2012-07-02 11:28:52 -0700 (In reply to comment #10) > I'm not sure if this makes sense anymore. I think it can be done by using CTest and a build target. We don't have a global runner anymore. See bug 87251. Should we close this bug? 662293 12 tmpsantos 2012-07-03 09:21:57 -0700 *** This bug has been marked as a duplicate of bug 87251 *** 662658 13 108666 rakuco 2012-07-03 21:09:30 -0700 Comment on attachment 108666 Patch Clearing r? flag from the patch. 108124 2011-09-21 01:54:30 -0700 2011-09-26 07:49:00 -0700 Patch webkit-efl-unit-tests-run-script.patch text/plain 2579 k.czech ZGlmZiAtLWdpdCBhL1Rvb2xzL0NoYW5nZUxvZyBiL1Rvb2xzL0NoYW5nZUxvZwppbmRleCBmZDcw MjdiLi5jOGZjZjcwIDEwMDY0NAotLS0gYS9Ub29scy9DaGFuZ2VMb2cKKysrIGIvVG9vbHMvQ2hh bmdlTG9nCkBAIC0xLDMgKzEsMTYgQEAKKzIwMTEtMDktMjAgIEtyenlzenRvZiBDemVjaCAgPGsu Y3plY2hAc2Ftc3VuZy5jb20+CisKKyAgICAgICAgW0VGTF0gU2NyaXB0IGZvciBydW5uaW5nIFdl YktpdC1FRkwgVW5pdCBUZXN0cworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93 X2J1Zy5jZ2k/aWQ9Njg1MTEKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4K KworICAgICAgICBTdXBwb3J0IGZvciBydW5uaW5nIHVuaXQgdGVzdHMgYXV0b21hdGljYWxseS4K KworICAgICAgICAqIFNjcmlwdHMvcnVuLWVmbC10ZXN0czogQWRkZWQuCisgICAgICAgIChydW5U ZXN0KToKKyAgICAgICAgKHVzYWdlKToKKwogMjAxMS0wOS0yMCAgVG9yIEFybmUgVmVzdGLDuCAg PHRvci5hcm5lLnZlc3Rib0Bub2tpYS5jb20+CiAKICAgICAgICAgV2ViS2l0VGVzdFJ1bm5lcjog UHJvdmlkZSB1c2FnZSBpZiBydW4gd2l0aG91dCBhcmd1bWVudHMKZGlmZiAtLWdpdCBhL1Rvb2xz L1NjcmlwdHMvcnVuLWVmbC10ZXN0cyBiL1Rvb2xzL1NjcmlwdHMvcnVuLWVmbC10ZXN0cwpuZXcg ZmlsZSBtb2RlIDEwMDc1NQppbmRleCAwMDAwMDAwLi5jNWVmYmFlCi0tLSAvZGV2L251bGwKKysr IGIvVG9vbHMvU2NyaXB0cy9ydW4tZWZsLXRlc3RzCkBAIC0wLDAgKzEsNjYgQEAKKyMhL3Vzci9i aW4vcGVybAorIyAgICBDb3B5cmlnaHQgKEMpIDIwMTEgU2Ftc3VuZyBFbGVjdHJvbmljcworIwor IyAgICBUaGlzIGxpYnJhcnkgaXMgZnJlZSBzb2Z0d2FyZTsgeW91IGNhbiByZWRpc3RyaWJ1dGUg aXQgYW5kL29yCisjICAgIG1vZGlmeSBpdCB1bmRlciB0aGUgdGVybXMgb2YgdGhlIEdOVSBMZXNz ZXIgR2VuZXJhbCBQdWJsaWMKKyMgICAgTGljZW5zZSBhcyBwdWJsaXNoZWQgYnkgdGhlIEZyZWUg U29mdHdhcmUgRm91bmRhdGlvbjsgZWl0aGVyCisjICAgIHZlcnNpb24gMi4xIG9mIHRoZSBMaWNl bnNlLCBvciAoYXQgeW91ciBvcHRpb24pIGFueSBsYXRlciB2ZXJzaW9uLgorIworIyAgICBUaGlz IGxpYnJhcnkgaXMgZGlzdHJpYnV0ZWQgaW4gdGhlIGhvcGUgdGhhdCBpdCB3aWxsIGJlIHVzZWZ1 bCwKKyMgICAgYnV0IFdJVEhPVVQgQU5ZIFdBUlJBTlRZOyB3aXRob3V0IGV2ZW4gdGhlIGltcGxp ZWQgd2FycmFudHkgb2YKKyMgICAgTUVSQ0hBTlRBQklMSVRZIG9yIEZJVE5FU1MgRk9SIEEgUEFS VElDVUxBUiBQVVJQT1NFLiBTZWUgdGhlIEdOVQorIyAgICBMZXNzZXIgR2VuZXJhbCBQdWJsaWMg TGljZW5zZSBmb3IgbW9yZSBkZXRhaWxzLgorIworIyAgICBZb3Ugc2hvdWxkIGhhdmUgcmVjZWl2 ZWQgYSBjb3B5IG9mIHRoZSBHTlUgTGVzc2VyIEdlbmVyYWwgUHVibGljIExpY2Vuc2UKKyMgICAg YWxvbmcgd2l0aCB0aGlzIGxpYnJhcnk7IGlmIG5vdCwgd3JpdGUgdG8gdGhlIEZyZWUgU29mdHdh cmUgRm91bmRhdGlvbiwKKyMgICAgSW5jLiwgNTkgVGVtcGxlIFBsYWNlLCBTdWl0ZSAzMzAsIEJv c3RvbiwgTUEgMDIxMTEtMTMwNyBVU0EKK3VzZSBzdHJpY3Q7Cit1c2UgRmlsZTo6U3BlYzsKK3Vz ZSBHZXRvcHQ6Okxvbmc7Cit1c2UgRmluZEJpbjsKK3VzZSBsaWIgJEZpbmRCaW46OkJpbjsKK3Vz ZSB3ZWJraXRkaXJzOworCitteSAkc2hvd0hlbHAgPSAwOworbXkgJHRlc3ROYW1lID0gIiI7CisK K0dldE9wdGlvbnMoCisgICAgJ2hlbHAnICAgICAgICAgID0+IFwkc2hvd0hlbHAsCisgICAgJ3J8 cnVuLXRlc3Q9cycgID0+IFwkdGVzdE5hbWUsCisgICAgJzw+JyAgICAgICAgICAgID0+IFwmdXNh Z2UKKykgb3IgdXNhZ2UoKTsKKworaWYgKCRzaG93SGVscCkgeworICAgIHVzYWdlKCk7Cit9CisK K3NldENvbmZpZ3VyYXRpb24oKTsKKworbXkgJHNvdXJjZVJvb3REaXIgPSBGaWxlOjpTcGVjLT5j YXRmaWxlKCRGaW5kQmluOjpCaW4sICIuLi8uLiIpOworY2hkaXIoJHNvdXJjZVJvb3REaXIpOwor CitydW5UZXN0KCR0ZXN0TmFtZSk7CisKK3N1YiBydW5UZXN0Cit7CisgICAgbXkgJGFyZ3MgPSBA X1swXTsKKyAgICBteSAkcGF0aFRvQmluYXJ5ID0gIlNvdXJjZS9Qcm9ncmFtcy9FVW5pdFRlc3Rz IjsKKyAgICBteSAkdXROYW1lID0gIiI7CisgICAgaWYgKGxlbmd0aCgkYXJncykgPiAwKSB7Cisg ICAgICAgICR1dE5hbWUgPSAiIC0tZ3Rlc3RfZmlsdGVyPSIgLiAkYXJncyAuICIuKiIgLiAiXG4i OworICAgIH0KKyAgICBleGl0IHN5c3RlbSgkcGF0aFRvQmluYXJ5IC4gJHV0TmFtZSk7Cit9CisK K3N1YiB1c2FnZQoreworICAgIG15ICRwcm9ncmFtTmFtZSA9IGJhc2VuYW1lKCQwKTsKKyAgICBt eSAkdXNhZ2VUZXh0ID0gPDxFT0Y7CisgICAgVXNhZ2U6ICRwcm9ncmFtTmFtZSBbb3B0aW9uc10K KyAgICAtLWhlbHAgICAgICBTaG93IGhlbHAgbWVzc2FnZQorICAgIC1yfC0tcnVuLXRlc3QgIFJ1 biBwYXJ0aWN1bGFyIHRlc3QKK0VPRgorCisgICAgcHJpbnQgU1RERVJSICR1c2FnZVRleHQ7Cisg ICAgZXhpdCAxOworfQo= 108666 2011-09-26 07:49:00 -0700 2012-07-03 21:09:30 -0700 Patch webkit-efl-unit-tests-run-script.patch text/plain 2578 k.czech ZGlmZiAtLWdpdCBhL1Rvb2xzL0NoYW5nZUxvZyBiL1Rvb2xzL0NoYW5nZUxvZwppbmRleCBmZDcw MjdiLi4wMzBmOTRmIDEwMDY0NAotLS0gYS9Ub29scy9DaGFuZ2VMb2cKKysrIGIvVG9vbHMvQ2hh bmdlTG9nCkBAIC0xLDMgKzEsMTYgQEAKKzIwMTEtMDktMjAgIEtyenlzenRvZiBDemVjaCAgPGsu Y3plY2hAc2Ftc3VuZy5jb20+CisKKyAgICAgICAgW0VGTF0gU2NyaXB0IGZvciBydW5uaW5nIFdl YktpdC1FRkwgVW5pdCBUZXN0cworICAgICAgICBodHRwczovL2J1Z3Mud2Via2l0Lm9yZy9zaG93 X2J1Zy5jZ2k/aWQ9Njg1MTEKKworICAgICAgICBTdXBwb3J0IGZvciBydW5uaW5nIHVuaXQgdGVz dHMgYXV0b21hdGljYWxseS4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4K KworICAgICAgICAqIFNjcmlwdHMvcnVuLWVmbC10ZXN0czogQWRkZWQuCisgICAgICAgIChydW5U ZXN0KToKKyAgICAgICAgKHVzYWdlKToKKwogMjAxMS0wOS0yMCAgVG9yIEFybmUgVmVzdGLDuCAg PHRvci5hcm5lLnZlc3Rib0Bub2tpYS5jb20+CiAKICAgICAgICAgV2ViS2l0VGVzdFJ1bm5lcjog UHJvdmlkZSB1c2FnZSBpZiBydW4gd2l0aG91dCBhcmd1bWVudHMKZGlmZiAtLWdpdCBhL1Rvb2xz L1NjcmlwdHMvcnVuLWVmbC10ZXN0cyBiL1Rvb2xzL1NjcmlwdHMvcnVuLWVmbC10ZXN0cwpuZXcg ZmlsZSBtb2RlIDEwMDc1NQppbmRleCAwMDAwMDAwLi5iMGViZjlhCi0tLSAvZGV2L251bGwKKysr IGIvVG9vbHMvU2NyaXB0cy9ydW4tZWZsLXRlc3RzCkBAIC0wLDAgKzEsNjUgQEAKKyMhL3Vzci9i aW4vcGVybAorIyAgICBDb3B5cmlnaHQgKEMpIDIwMTEgU2Ftc3VuZyBFbGVjdHJvbmljcworIwor IyAgICBUaGlzIGxpYnJhcnkgaXMgZnJlZSBzb2Z0d2FyZTsgeW91IGNhbiByZWRpc3RyaWJ1dGUg aXQgYW5kL29yCisjICAgIG1vZGlmeSBpdCB1bmRlciB0aGUgdGVybXMgb2YgdGhlIEdOVSBMZXNz ZXIgR2VuZXJhbCBQdWJsaWMKKyMgICAgTGljZW5zZSBhcyBwdWJsaXNoZWQgYnkgdGhlIEZyZWUg U29mdHdhcmUgRm91bmRhdGlvbjsgZWl0aGVyCisjICAgIHZlcnNpb24gMi4xIG9mIHRoZSBMaWNl bnNlLCBvciAoYXQgeW91ciBvcHRpb24pIGFueSBsYXRlciB2ZXJzaW9uLgorIworIyAgICBUaGlz IGxpYnJhcnkgaXMgZGlzdHJpYnV0ZWQgaW4gdGhlIGhvcGUgdGhhdCBpdCB3aWxsIGJlIHVzZWZ1 bCwKKyMgICAgYnV0IFdJVEhPVVQgQU5ZIFdBUlJBTlRZOyB3aXRob3V0IGV2ZW4gdGhlIGltcGxp ZWQgd2FycmFudHkgb2YKKyMgICAgTUVSQ0hBTlRBQklMSVRZIG9yIEZJVE5FU1MgRk9SIEEgUEFS VElDVUxBUiBQVVJQT1NFLiBTZWUgdGhlIEdOVQorIyAgICBMZXNzZXIgR2VuZXJhbCBQdWJsaWMg TGljZW5zZSBmb3IgbW9yZSBkZXRhaWxzLgorIworIyAgICBZb3Ugc2hvdWxkIGhhdmUgcmVjZWl2 ZWQgYSBjb3B5IG9mIHRoZSBHTlUgTGVzc2VyIEdlbmVyYWwgUHVibGljIExpY2Vuc2UKKyMgICAg YWxvbmcgd2l0aCB0aGlzIGxpYnJhcnk7IGlmIG5vdCwgd3JpdGUgdG8gdGhlIEZyZWUgU29mdHdh cmUgRm91bmRhdGlvbiwKKyMgICAgSW5jLiwgNTkgVGVtcGxlIFBsYWNlLCBTdWl0ZSAzMzAsIEJv c3RvbiwgTUEgMDIxMTEtMTMwNyBVU0EKK3VzZSBzdHJpY3Q7Cit1c2UgRmlsZTo6U3BlYzsKK3Vz ZSBHZXRvcHQ6Okxvbmc7Cit1c2UgRmluZEJpbjsKK3VzZSBsaWIgJEZpbmRCaW46OkJpbjsKK3Vz ZSB3ZWJraXRkaXJzOworCitteSAkc2hvd0hlbHAgPSAwOworbXkgJHRlc3ROYW1lID0gIiI7CisK K0dldE9wdGlvbnMoCisgICAgJ2hlbHAnICAgICAgICAgID0+IFwkc2hvd0hlbHAsCisgICAgJ3J8 cnVuLXRlc3Q9cycgID0+IFwkdGVzdE5hbWUsCisgICAgJzw+JyAgICAgICAgICAgID0+IFwmdXNh Z2UKKykgb3IgdXNhZ2UoKTsKKworaWYgKCRzaG93SGVscCkgeworICAgIHVzYWdlKCk7Cit9CisK K3NldENvbmZpZ3VyYXRpb24oKTsKKworbXkgJHNvdXJjZVJvb3REaXIgPSBGaWxlOjpTcGVjLT5j YXRmaWxlKCRGaW5kQmluOjpCaW4sICIuLi8uLiIpOworY2hkaXIoJHNvdXJjZVJvb3REaXIpOwor CitydW5UZXN0KCR0ZXN0TmFtZSk7CisKK3N1YiBydW5UZXN0Cit7CisgICAgbXkgKCR0ZXN0Rmls dGVyKSA9IEBfOworICAgIG15ICRwYXRoVG9UZXN0VXRpbGl0eSA9ICJTb3VyY2UvUHJvZ3JhbXMv RVVuaXRUZXN0cyI7CisgICAgbXkgQHRlc3RBcmdzID0gKCRwYXRoVG9UZXN0VXRpbGl0eSk7Cisg ICAgcHVzaCBAdGVzdEFyZ3MsICgiIC0tZ3Rlc3RfZmlsdGVyPSR7dGVzdEZpbHRlcn0uKlxuIikg aWYgJHRlc3RGaWx0ZXI7CisKKyAgICByZXR1cm4gc3lzdGVtKCJAdGVzdEFyZ3MiKTsKK30KKwor c3ViIHVzYWdlCit7CisgICAgbXkgJHByb2dyYW1OYW1lID0gYmFzZW5hbWUoJDApOworICAgIG15 ICR1c2FnZVRleHQgPSA8PEVPRjsKKyAgICBVc2FnZTogJHByb2dyYW1OYW1lIFtvcHRpb25zXQor ICAgIC0taGVscCAgICAgIFNob3cgaGVscCBtZXNzYWdlCisgICAgLXJ8LS1ydW4tdGVzdCAgUnVu IHBhcnRpY3VsYXIgdGVzdAorRU9GCisKKyAgICBwcmludCBTVERFUlIgJHVzYWdlVGV4dDsKKyAg ICBleGl0IDE7Cit9Cg==