Some tests are failed because of the handling of environment variables.
Created attachment 440516 [details] PATCH
Created attachment 440524 [details] PATCH
Created attachment 440526 [details] PATCH
Comment on attachment 440526 [details] PATCH View in context: https://bugs.webkit.org/attachment.cgi?id=440526&action=review > Tools/Scripts/webkitperl/auto-version_unittest/autoVersionTests.pl:703 > + $ENV{RC_ProjectSourceVersion} = $testCase->{'RC_ProjectSourceVersion'}; I think if you're going to set our ENV like this, you'll need to reset it between tests, otherwise values will leak from test to test (or doing something more complicated that will only set the env in the created shell). > Tools/Scripts/webkitperl/prepare-ChangeLog_unittest/generateFunctionLists.pl:508 > + plan skip_all => 'Please add `pathc` to the $PATH'; Nitpick: pathc -> patch
Comment on attachment 440526 [details] PATCH View in context: https://bugs.webkit.org/attachment.cgi?id=440526&action=review > Tools/Scripts/webkitperl/auto-version_unittest/autoVersionTests.pl:691 > +# Test can only be run if VersionStamper.exe exists This comment doesn't match with the code. Which do yo want to check VersionStamper.exe or auto-version.pl here? > Tools/Scripts/webkitperl/prepare-ChangeLog_unittest/generateFunctionLists.pl:34 > +use VCSUtils; Why do you need this change? > Tools/Scripts/webkitperl/prepare-ChangeLog_unittest/generateFunctionLists.pl:509 > + exit 0; I think the test should fail if patch isn't found.
Comment on attachment 440526 [details] PATCH View in context: https://bugs.webkit.org/attachment.cgi?id=440526&action=review >> Tools/Scripts/webkitperl/auto-version_unittest/autoVersionTests.pl:691 >> +# Test can only be run if VersionStamper.exe exists > > This comment doesn't match with the code. Which do yo want to check VersionStamper.exe or auto-version.pl here? Correct. Thanks. >> Tools/Scripts/webkitperl/auto-version_unittest/autoVersionTests.pl:703 >> + $ENV{RC_ProjectSourceVersion} = $testCase->{'RC_ProjectSourceVersion'}; > > I think if you're going to set our ENV like this, you'll need to reset it between tests, otherwise values will leak from test to test (or doing something more complicated that will only set the env in the created shell). I thought each test will be run in different process, but there seems no such assumption at all. I'll save and restore them at the end of each loop. >> Tools/Scripts/webkitperl/prepare-ChangeLog_unittest/generateFunctionLists.pl:34 >> +use VCSUtils; > > Why do you need this change? It was failed to load VCSUtils on the initial location. I bet this is related with the change on 5.26: https://metacpan.org/pod/perl5260delta#Removal-of-the-current-directory-(%22.%22)-from-@INC Anyway, it is dependent on the @INC so it is reasonable to move the usage bellow that. >> Tools/Scripts/webkitperl/prepare-ChangeLog_unittest/generateFunctionLists.pl:508 >> + plan skip_all => 'Please add `pathc` to the $PATH'; > > Nitpick: pathc -> patch Right. >> Tools/Scripts/webkitperl/prepare-ChangeLog_unittest/generateFunctionLists.pl:509 >> + exit 0; > > I think the test should fail if patch isn't found. Well, I want to hear the opinion from others.
<rdar://problem/84262724>
Created attachment 441640 [details] patch
Make `patch` required to run test. Also environment variable is recovered after getting result from process invocation.
Committed r284405 (?): <https://commits.webkit.org/r284405> All reviewed patches have been landed. Closing bug and clearing flags on attachment 441640 [details].