nrwt: implement simple 'virtual test suite' support
Created attachment 129173 [details] Patch
Comment on attachment 129173 [details] Patch This looks like a good approach. I wonder if we can put some fo this logic outside of base.py. That file is kind of growing out of control.
(In reply to comment #2) > (From update of attachment 129173 [details]) > This looks like a good approach. I wonder if we can put some fo this logic outside of base.py. That file is kind of growing out of control. As we discussed on IRC on Monday, I think it makes sense to move all of the logic for mapping tests to the filesystem out of base into some sort of 'finder' or 'database' class. I decided to not do that until after I had these patches reviewed, just to avoid confusing things, though. My current vision for base.py (and the port classes generally) is that they should be composite objects that either assemble the desired behavior from other components (like the http server and the test dabase, above) or just declare preferences (worker_model, default_child_processes, etc.)
That sounds like a good plan. I'll read the patch again now.
Comment on attachment 129173 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=129173&action=review Thanks! > Tools/Scripts/webkitpy/layout_tests/port/base.py:1061 > + class VirtualTestSuite(object): Can we move this declaration to the top-level? It seems like folks should just call the constructor rather than calling the virtual_suite method. > Tools/Scripts/webkitpy/layout_tests/port/base.py:1074 > + if self._populated_virtual_test_suites is None: Should we make this function @memoized?
Comment on attachment 129173 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=129173&action=review >> Tools/Scripts/webkitpy/layout_tests/port/base.py:1061 >> + class VirtualTestSuite(object): > > Can we move this declaration to the top-level? It seems like folks should just call the constructor rather than calling the virtual_suite method. I actually thought having a member function that constructed the object provided better encapsulation and saved subclasses from having to import another symbol, but I don't feel strongly about it, so I will move this to file scope. >> Tools/Scripts/webkitpy/layout_tests/port/base.py:1074 >> + if self._populated_virtual_test_suites is None: > > Should we make this function @memoized? I'm not as big a fan of using @memoized on functions that don't take any arguments, but I can add it.
Created attachment 129495 [details] update w/ review feedback
Committed r109266: <http://trac.webkit.org/changeset/109266>
Comment on attachment 129495 [details] update w/ review feedback Cleared review? from attachment 129495 [details] so that this bug does not appear in http://webkit.org/pending-review. If you would like this patch reviewed, please attach it to a new bug (or re-open this bug before marking it for review again).