Added a Build Binaries Fetcher in Run WebKit Tests via a new flag `--download-binaries`
Created attachment 338656 [details] Patch
unit tests are coming - I still need to make some changes to them.
Comment on attachment 338656 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=338656&action=review > Tools/ChangeLog:3 > + Added a Build Binaries Fetcher in Run WebKit Tests via a new flag `--download-binaries` What is the rationale for having this feature? Having many ways to achieve the same result is not great, as it increases potential for confusion.
I would prefer if we just made download-built-product (or a script like it) fit for human usage and move it to scripts.
(In reply to Jonathan Bedard from comment #4) > I would prefer if we just made download-built-product (or a script like it) > fit for human usage and move it to scripts. Thanks for the feedback Jonathan. Your suggestion of moving this to a standalone script under Tools/Scripts makes sense. There is value in integrating the script with the test runner via a flag, so I assume that hook would stay there but reference the renamed standalone script. Is that also what you were thinking?
No, the suggestion is not not have the feature exposed through run-webkit-tests at all. You say that there is value in integrating support, but what is this value? Do you also intend to add such support to run-javascriptcore-tests, run-api-tests, and to other test scripts?
(In reply to Alexey Proskuryakov from comment #6) > No, the suggestion is not not have the feature exposed through > run-webkit-tests at all. You say that there is value in integrating support, > but what is this value? Do you also intend to add such support to > run-javascriptcore-tests, run-api-tests, and to other test scripts? That is indeed something that we could consider. The fact that this is encapsulated in BuildBinariesFetcher should help that. We need BuildBinariesFetcher to easily rebase WPT tests. For this specific purpose, there is no hard requirement to add it to run-webkit-tests since we need to do other stuff before and after. I believe though that this would be a handy option for users of run-webkit-tests.
Comment on attachment 338656 [details] Patch I think that I'm going to say r- at this point. It doesn't sound like there is any huge reason to create a toaster/fridge combo here.
(In reply to Alexey Proskuryakov from comment #8) > Comment on attachment 338656 [details] > Patch > > I think that I'm going to say r- at this point. It doesn't sound like there > is any huge reason to create a toaster/fridge combo here. Alexey - can you elaborate on your points of concern?
Will be moving this to a standalone script here https://bugs.webkit.org/show_bug.cgi?id=185045
> Alexey - can you elaborate on your points of concern? I think that I already did.