Re: [webkit-dev] LayoutTestHelper
Could you rename all these helper methods and programs to have more descriptive names? None of this may not have happened if we had more semantically precise program/method names. On Mon, Apr 30, 2012 at 12:33 PM, Dirk Pranke dpra...@chromium.org wrote: On Sun, Apr 29, 2012 at 6:20 PM, Maciej Stachowiak m...@apple.com wrote: On Apr 29, 2012, at 5:49 PM, Maciej Stachowiak m...@apple.com wrote: Hi folks, new-run-webkit-tests seems to mess with the system color profile on Mac, even when not running pixel tests. Historically, I believe we did this only when running pixel tests. I noticed that this is because it launches the LayoutTestHelper tool unconditionally, and in addition to changing the color profile on Mac, it also changes font smoothing settings on Windows. Does anyone know whether this work is required when running non-pixel tests? If not, I'd like to change NRWT to only launch LayoutTestHelper in pixel mode, and perhaps also rename LayoutTestHelper to PixelTestHelper. I went ahead assumed that this tool was never necessary for non-pixel tests, if I'm wrong, please comment here: https://bugs.webkit.org/show_bug.cgi?id=81729 You're wrong :). I commented on the bug as well, but for the record the chromium android port,at least, requires that that method be called regardless of whether pixel tests are being run or not. I had thought at some point that the chromium layout test helper did some things that were needed on windows regardless of whether the pixel tests were being run, but my memory might be failing me. It was always the intent that the start_helper() hook be generic. However, that routine was written before we had a setup_test_run() method, so it's probable that we could move all of the non-pixel logic into that method. -- Dirk ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] LayoutTestHelper
Better naming/documentation may help. But I think the root problem here was that the Chromium Android LayoutTestHelper does not exist in the WebKit repository, as far as I can tell. So there was no reasonable way for anyone to consider it when making changes. All the LayoutTestHelpers that exist in svn.webkit.org could usefully be renamed to PixelTestHelper at the very least, or perhaps to something even more specific. Also, the mac and chromium-mac versions should likely be merged. They are almost identical and I accidentally fixed a bug in one and not the other. Regards, Maciej On May 3, 2012, at 12:31 AM, Ryosuke Niwa rn...@webkit.org wrote: Could you rename all these helper methods and programs to have more descriptive names? None of this may not have happened if we had more semantically precise program/method names. On Mon, Apr 30, 2012 at 12:33 PM, Dirk Pranke dpra...@chromium.org wrote: On Sun, Apr 29, 2012 at 6:20 PM, Maciej Stachowiak m...@apple.com wrote: On Apr 29, 2012, at 5:49 PM, Maciej Stachowiak m...@apple.com wrote: Hi folks, new-run-webkit-tests seems to mess with the system color profile on Mac, even when not running pixel tests. Historically, I believe we did this only when running pixel tests. I noticed that this is because it launches the LayoutTestHelper tool unconditionally, and in addition to changing the color profile on Mac, it also changes font smoothing settings on Windows. Does anyone know whether this work is required when running non-pixel tests? If not, I'd like to change NRWT to only launch LayoutTestHelper in pixel mode, and perhaps also rename LayoutTestHelper to PixelTestHelper. I went ahead assumed that this tool was never necessary for non-pixel tests, if I'm wrong, please comment here: https://bugs.webkit.org/show_bug.cgi?id=81729 You're wrong :). I commented on the bug as well, but for the record the chromium android port,at least, requires that that method be called regardless of whether pixel tests are being run or not. I had thought at some point that the chromium layout test helper did some things that were needed on windows regardless of whether the pixel tests were being run, but my memory might be failing me. It was always the intent that the start_helper() hook be generic. However, that routine was written before we had a setup_test_run() method, so it's probable that we could move all of the non-pixel logic into that method. -- Dirk ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] LayoutTestHelper
On Thu, May 3, 2012 at 7:35 AM, Maciej Stachowiak m...@apple.com wrote: Better naming/documentation may help. But I think the root problem here was that the Chromium Android LayoutTestHelper does not exist in the WebKit repository, as far as I can tell. So there was no reasonable way for anyone to consider it when making changes. I'm not trying to make a big deal out of this, but the python code for the android port is checked in :) http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py -- Dirk ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] LayoutTestHelper
On May 3, 2012, at 8:54 AM, Dirk Pranke dpra...@chromium.org wrote: On Thu, May 3, 2012 at 7:35 AM, Maciej Stachowiak m...@apple.com wrote: Better naming/documentation may help. But I think the root problem here was that the Chromium Android LayoutTestHelper does not exist in the WebKit repository, as far as I can tell. So there was no reasonable way for anyone to consider it when making changes. I'm not trying to make a big deal out of this, but the python code for the android port is checked in :) http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py I see, I was looking for something that would start a LayoutTestHelper tool. It seems like Android has a different interpretation of what start_helper means. I would like to fix all this stuff but I am not sure how to fix the naming. Do you have a suggestion for a better name for the port-specific pre-test setup hook? Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] LayoutTestHelper
On Thu, May 3, 2012 at 9:16 AM, Maciej Stachowiak m...@apple.com wrote: On May 3, 2012, at 8:54 AM, Dirk Pranke dpra...@chromium.org wrote: On Thu, May 3, 2012 at 7:35 AM, Maciej Stachowiak m...@apple.com wrote: Better naming/documentation may help. But I think the root problem here was that the Chromium Android LayoutTestHelper does not exist in the WebKit repository, as far as I can tell. So there was no reasonable way for anyone to consider it when making changes. I'm not trying to make a big deal out of this, but the python code for the android port is checked in :) http://trac.webkit.org/browser/trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium_android.py I see, I was looking for something that would start a LayoutTestHelper tool. It seems like Android has a different interpretation of what start_helper means. I would like to fix all this stuff but I am not sure how to fix the naming. Do you have a suggestion for a better name for the port-specific pre-test setup hook? As I mentioned earlier in the thread, there is now a setup_test_run() hook as well in the Port interface (and a clean_up_test_run() hook) -- you can see examples of use in Tools/Scripts/webkitpy/layout_tests/port/gtk.py -- so we could probably move the android code to that and then change the name or at least the doc string of start_helper to be clear that it's pixel-test-specific. -- Dirk ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] LayoutTestHelper
On Sun, Apr 29, 2012 at 6:20 PM, Maciej Stachowiak m...@apple.com wrote: On Apr 29, 2012, at 5:49 PM, Maciej Stachowiak m...@apple.com wrote: Hi folks, new-run-webkit-tests seems to mess with the system color profile on Mac, even when not running pixel tests. Historically, I believe we did this only when running pixel tests. I noticed that this is because it launches the LayoutTestHelper tool unconditionally, and in addition to changing the color profile on Mac, it also changes font smoothing settings on Windows. Does anyone know whether this work is required when running non-pixel tests? If not, I'd like to change NRWT to only launch LayoutTestHelper in pixel mode, and perhaps also rename LayoutTestHelper to PixelTestHelper. I went ahead assumed that this tool was never necessary for non-pixel tests, if I'm wrong, please comment here: https://bugs.webkit.org/show_bug.cgi?id=81729 You're wrong :). I commented on the bug as well, but for the record the chromium android port,at least, requires that that method be called regardless of whether pixel tests are being run or not. I had thought at some point that the chromium layout test helper did some things that were needed on windows regardless of whether the pixel tests were being run, but my memory might be failing me. It was always the intent that the start_helper() hook be generic. However, that routine was written before we had a setup_test_run() method, so it's probable that we could move all of the non-pixel logic into that method. -- Dirk ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] LayoutTestHelper
Hi folks, new-run-webkit-tests seems to mess with the system color profile on Mac, even when not running pixel tests. Historically, I believe we did this only when running pixel tests. I noticed that this is because it launches the LayoutTestHelper tool unconditionally, and in addition to changing the color profile on Mac, it also changes font smoothing settings on Windows. Does anyone know whether this work is required when running non-pixel tests? If not, I'd like to change NRWT to only launch LayoutTestHelper in pixel mode, and perhaps also rename LayoutTestHelper to PixelTestHelper. Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] LayoutTestHelper
On Apr 29, 2012, at 5:49 PM, Maciej Stachowiak m...@apple.com wrote: Hi folks, new-run-webkit-tests seems to mess with the system color profile on Mac, even when not running pixel tests. Historically, I believe we did this only when running pixel tests. I noticed that this is because it launches the LayoutTestHelper tool unconditionally, and in addition to changing the color profile on Mac, it also changes font smoothing settings on Windows. Does anyone know whether this work is required when running non-pixel tests? If not, I'd like to change NRWT to only launch LayoutTestHelper in pixel mode, and perhaps also rename LayoutTestHelper to PixelTestHelper. I went ahead assumed that this tool was never necessary for non-pixel tests, if I'm wrong, please comment here: https://bugs.webkit.org/show_bug.cgi?id=81729 Regards, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
