Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(143)

Issue 569913002: fix virtual_test_suites() for browser_test ports (Closed)

Created:
6 years, 3 months ago by Dirk Pranke
Modified:
6 years, 3 months ago
Reviewers:
Lei Zhang
CC:
blink-reviews, ivandavid
Project:
blink
Visibility:
Public.

Description

Fix virtual_test_suites() for browser_test ports In r181420 we started pulling the list of virtual test suites to run from LayoutTests/VirtualTestSuites (a new file). The browser_test ports do not pull their tests from that directory, and need a different list of suites, but I forgot to override the port.virtual_test_suites() call in their implementations. The browser_test ports don't currently need or have any virtual test suites, so this patch implements returning an empty list and adds tests to ensure that we would catch this regression if it happened again. We also rework the browser_test_unittest.py tests to eliminate a bunch of duplication and make it clearer that the tests are shared and testing the same behavior. TBR=thestig@chromium.org BUG=413986 Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=181962

Patch Set 1 #

Total comments: 2

Patch Set 2 : rework implementation of test_virtual_test_suites(), browser_test_unittest #

Unified diffs Side-by-side diffs Delta from patch set Stats (+49 lines, -50 lines) Patch
M Tools/Scripts/webkitpy/layout_tests/port/browser_test.py View 1 chunk +3 lines, -0 lines 0 comments Download
M Tools/Scripts/webkitpy/layout_tests/port/browser_test_unittest.py View 1 1 chunk +30 lines, -50 lines 0 comments Download
M Tools/Scripts/webkitpy/layout_tests/port/linux_unittest.py View 1 1 chunk +1 line, -0 lines 0 comments Download
M Tools/Scripts/webkitpy/layout_tests/port/mac_unittest.py View 1 1 chunk +1 line, -0 lines 0 comments Download
M Tools/Scripts/webkitpy/layout_tests/port/mock_drt_unittest.py View 1 chunk +3 lines, -0 lines 0 comments Download
M Tools/Scripts/webkitpy/layout_tests/port/port_testcase.py View 1 4 chunks +10 lines, -0 lines 0 comments Download
M Tools/Scripts/webkitpy/layout_tests/port/win_unittest.py View 1 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 11 (4 generated)
Dirk Pranke
6 years, 3 months ago (2014-09-13 17:12:01 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patchset/569913002/1
6 years, 3 months ago (2014-09-13 17:12:39 UTC) #3
commit-bot: I haz the power
Try jobs failed on following builders: linux_blink_rel on tryserver.blink (http://build.chromium.org/p/tryserver.blink/builders/linux_blink_rel/builds/24748)
6 years, 3 months ago (2014-09-13 17:29:09 UTC) #5
Lei Zhang
lgtm https://codereview.chromium.org/569913002/diff/1/Tools/Scripts/webkitpy/layout_tests/port/win_unittest.py File Tools/Scripts/webkitpy/layout_tests/port/win_unittest.py (right): https://codereview.chromium.org/569913002/diff/1/Tools/Scripts/webkitpy/layout_tests/port/win_unittest.py#newcode33 Tools/Scripts/webkitpy/layout_tests/port/win_unittest.py:33: from webkitpy.common.system.systemhost import SystemHost nit: alphabetical order?
6 years, 3 months ago (2014-09-13 17:31:13 UTC) #6
Dirk Pranke
https://codereview.chromium.org/569913002/diff/1/Tools/Scripts/webkitpy/layout_tests/port/win_unittest.py File Tools/Scripts/webkitpy/layout_tests/port/win_unittest.py (right): https://codereview.chromium.org/569913002/diff/1/Tools/Scripts/webkitpy/layout_tests/port/win_unittest.py#newcode33 Tools/Scripts/webkitpy/layout_tests/port/win_unittest.py:33: from webkitpy.common.system.systemhost import SystemHost whoops. Yeah :).
6 years, 3 months ago (2014-09-13 22:45:56 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patchset/569913002/40001
6 years, 3 months ago (2014-09-14 22:34:22 UTC) #10
commit-bot: I haz the power
6 years, 3 months ago (2014-09-14 23:59:09 UTC) #11
Message was sent while issue was closed.
Committed patchset #2 (id:40001) as 181962

Powered by Google App Engine
This is Rietveld 408576698