Move exit-after-n-failures count into commitqueuetask in preparation for increasing it
Created attachment 89261 [details] Patch
Comment on attachment 89261 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=89261&action=review > Tools/Scripts/webkitpy/tool/bot/commitqueuetask.py:152 > + " ".join(self._run_webkit_tests_args()), That's too bad. Can use we some sort of robust serialization / deserialization instead? Maybe JSON?
(In reply to comment #2) > (From update of attachment 89261 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=89261&action=review > > > Tools/Scripts/webkitpy/tool/bot/commitqueuetask.py:152 > > + " ".join(self._run_webkit_tests_args()), > > That's too bad. Can use we some sort of robust serialization / deserialization instead? Maybe JSON? I mean, we're passing through the shell here. We could invent a way, sure. But this seems like it fits the unix model. Yes, this isnt' robust enough to handle all cases. I'm kinda against being able to pass argumetns into run-webkit-tests at all, but it seems useful in the commit-queue case. Unless we were to invent some sort of config-file system for running webkit-patch with a certain config.
Maybe we should just pass the number of tests to run? That's less general, but do we have examples of other arguments we want to pass? Maybe shlex has some quoting functions we can use?
I originally wrote that part of the patch, to handle passing ignored tests. (From the approach you didn't like.) I'm not sure if we'll have other arguments. And it's possible we dont' even need to do this. But it seemed odd for the commitqueuetask to know the behavior of runtests.py. But I can also just chuck this patch. After further though, it's not on the critical path.
Comment on attachment 89261 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=89261&action=review >>> Tools/Scripts/webkitpy/tool/bot/commitqueuetask.py:152 >>> + " ".join(self._run_webkit_tests_args()), >> >> That's too bad. Can use we some sort of robust serialization / deserialization instead? Maybe JSON? > > I mean, we're passing through the shell here. We could invent a way, sure. But this seems like it fits the unix model. Yes, this isnt' robust enough to handle all cases. > > I'm kinda against being able to pass argumetns into run-webkit-tests at all, but it seems useful in the commit-queue case. Unless we were to invent some sort of config-file system for running webkit-patch with a certain config. I'm confused. Why serialize at all? can't you just create the list in a local variable and extend it?
(In reply to comment #6) > > I'm kinda against being able to pass argumetns into run-webkit-tests at all, but it seems useful in the commit-queue case. Unless we were to invent some sort of config-file system for running webkit-patch with a certain config. > > I'm confused. Why serialize at all? can't you just create the list in a local variable and extend it? Will that survive the trip through popen? These are being passed to a separate process... I don't think it will.
Comment on attachment 89261 [details] Patch I believe that Eric took a different approach to this issue. Please re-nominate if I'm mistaken.
The fixes are orthogonal. But this one is not required for what I wanted to do. So if it's viewed as more ugly and helpful, let's just close the bug.